fix: prevent fatal error when mail is not configured in Career Portal#820
Open
ocjorge wants to merge 3 commits into
Open
fix: prevent fatal error when mail is not configured in Career Portal#820ocjorge wants to merge 3 commits into
ocjorge 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.
Comment on lines
+461
to
+476
| 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; | ||
| } |
Author
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
Contributor
|
Can you please address Codacy's warning? |
Author
|
Addressed in commit e668f2f — removed the unused $mailerStatus variable. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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: