Problem/Motivation
Steps to reproduce
Sendgrid cannot control what other models try to do with emails. Something might send an email with an empty body. #3593810: Fatal error submitting a webform when email handler has an empty custom body.
If this happens, we get an exception:
SendGrid\Exception\TypeException: "$value" must have at least 1 characters. Got: 0 in SendGrid\Helper\Assert::minLength() (line 354 of /code/vendor/fastglass/sendgrid/src/Helper/Assert.php).
Proposed resolution
Fail gracefully, log an error, and allow execution to continue.
Remaining tasks
1. Make the code change ✓
2. Review
* confirmation that automated tests run successfully (I don't think this Drupal project is configured to run them on every MR).
* manual testing
* maintainer review
3. Merge in
User interface changes
none
API changes
don't throw an exception if something sends an empty body.
Data model changes
| Comment | File | Size | Author |
|---|
Issue fork sendgrid-3593813
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mukeshaddweb commentedTwo fixes in SendgridHandler::sendMail():
1. Added an explicit empty body guard before addContent() calls.
When both text and html are empty, the SendGrid library throws
TypeException: "$value" must have at least 1 characters. Got: 0
The fix logs a clear error and returns FALSE gracefully instead
of letting the exception propagate as a fatal error.
2. Widened catch (\Exception $e) to catch (\Throwable $e) to also
handle PHP Error objects (not just Exception subclasses), making
the handler robust against any unexpected throwable from the
SendGrid library.
Comment #3
dalin@mukeshaddweb
Same thing here, can you open a merge request rather than a patch file?
Comment #5
mukeshaddweb commentedComment #6
dalinLooks good to me, though I didn't do any functional tests, so I won't set this to RTBC.
I think this needs:
* confirmation that automated tests run successfully (I don't think this Drupal project is configured to run them on every MR).
* manual testing
* maintainer review