fix: prevent fatal error when mail is not configured in Career Portal - #820
fix: prevent fatal error when mail is not configured in Career Portal#820ocjorge wants to merge 3 commits into
Conversation
Without this change, CareerPortal::sendEmail() throws an uncaught PHPMailer exception when no mail server is configured, causing a fatal 500 error that prevents candidates from completing their application through the Career Portal. Wrapping the mail send in try/catch allows the application flow to continue normally when mail is unavailable, while still logging the error class and code for debugging without exposing sensitive SMTP configuration details in logs. Tested on PHP 8.4.21 + Debian 12 without a configured mail server.
There was a problem hiding this comment.
Pull request overview
This PR prevents candidate-facing HTTP 500 errors in the Career Portal when outgoing email is not configured by handling PHPMailer exceptions inside CareerPortal::sendEmail() so the application flow can complete and still show the confirmation page.
Changes:
- Wrap
Mailer::sendToOne()intry/catchto prevent uncaught mail exceptions from aborting the request. - Log minimal exception metadata (class + code) and force a deterministic
$mailerStatus = falseon failure.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| $mailer = new Mailer($this->_siteID, $userID); | ||
| $mailerStatus = $mailer->sendToOne( | ||
| array($destination, ''), | ||
| $subject, | ||
| $body, | ||
| true | ||
| ); | ||
| } catch (Exception $e) { | ||
| // Mail not configured or unavailable - log and continue | ||
| error_log( | ||
| 'OpenCATS CareerPortal mail error (site=' . $this->_siteID . '): ' | ||
| . get_class($e) . ' [' . $e->getCode() . ']' | ||
| ); | ||
| $mailerStatus = false; | ||
| } |
There was a problem hiding this comment.
Addressed in commit 9de0586:
- Changed catch to \PHPMailer\PHPMailer\Exception specifically to avoid
swallowing unrelated application errors - Updated brace style to Allman to match existing code conventions
- Catch \PHPMailer\PHPMailer\Exception instead of base Exception to avoid swallowing unrelated application errors - Use Allman brace style to match existing code conventions
|
Can you please address Codacy's warning? |
|
Addressed in commit e668f2f — removed the unused $mailerStatus variable. |
RussH
left a comment
RussH
left a comment
There was a problem hiding this comment.
Hi @ocjorge , as siteID has now been removed from the current codebase, can you rebase this against master and remove the remaining siteID references from the mailer call and error handling?
Everything else looks good. Once this is done, good to merge!
Problem
CareerPortal::sendEmail() throws an uncaught PHPMailer exception when
no mail server is configured. This causes a fatal HTTP 500 error that
prevents candidates from completing their application through the
Career Portal — the application is saved to the database but the
candidate sees an error page instead of a confirmation.
Solution
Wrap the mail send call in try/catch to allow the application flow to
continue normally when mail is unavailable.
The catch block:
configuration details in logs
Testing
Tested on PHP 8.4.21 + MariaDB 10.11 + Debian 12 without a configured
mail server. The candidate application flow completes successfully and
the confirmation page is shown. The error is logged to the PHP error log.
Context
This fix was suggested in the review of PR #792 by @anonymoususer72041: