Handle error sending confirmation email#1580
Conversation
stephencdaly
force-pushed
the
handle-error-sending-confirmation-email
branch
3 times, most recently
from
ec35b35 to
7fb2e4c
Compare
stephencdaly
force-pushed
the
handle-error-sending-confirmation-email
branch
3 times, most recently
from
c492301 to
9addd31
Compare
stephencdaly
force-pushed
the
handle-error-sending-confirmation-email
branch
from
9addd31 to
d099c61
Compare
theseanything
left a comment
theseanything
left a comment
There was a problem hiding this comment.
Thanks! Ultimately we don't want to be logging these validation errors (as they should just be displayed to the user, but useful to know what the problem is atm).
Some optional changes suggested.
We want to send the error message we get when ActionMailer is not able to parse the "To" email address for a Mail message to Sentry so we can try to figure out why certain invalid email addresses are getting through our validation. However, we do not want to sent PII to Sentry. This helper method attempts to obfuscate email addresses, retaining any special characters which will hopefully give us enough information while ensuring the email address is not identifiable. We replace the @ with (at) so that Sentry doesn't match an email address in the action_mailer_error we send and strip it out completely.
Previously, when ActionMailer failed to parse the To email address for the confirmation email, an error would not be raised until the GovukNotifyRails::Delivery code attempts to call `message.to.map` which raises an error due to `message.to` returning a String rather than a Map. It returns a String rather than a map only when there is an error parsing the "To" field. This commit makes it so that we look at the `mail.messages.errors` to establish whether there is an error with the "To" field. If there is, we send an error to Sentry and raise a ConfirmationEmailToAddressError to be handled by the CheckYourAnswersController in a subsequent commit.
Handle the ConfirmationEmailToAddressError raised by FormSubmissionService in CheckYourAnswersController#submit_answers by re-rendering the check your answers page with a validation error message stating that the email address is invalid.
stephencdaly
force-pushed
the
handle-error-sending-confirmation-email
branch
from
d099c61 to
afc8245
Compare
|
|
🎉 A review copy of this PR has been deployed! It is made of up two components Important Not all of the functionality of forms-runner is present in review apps. You should use the full dev environment to test the functionality which is disabled here. It may take 5 minutes or so for the application to be fully deployed and working. If it still isn't ready For the sign in details and more information, see the review apps wiki page. |
3 participants
What problem does this pull request solve?
Trello card: https://trello.com/c/wujuVVVe
Previously, when ActionMailer failed to parse the To email address for the confirmation email, an error would not be raised until the GovukNotifyRails::Delivery code attempts to call
message.to.mapwhich raises an error due tomessage.toreturning a String rather than a Map. It returns a String rather than a map only when there is an error parsing the "To" field.Sentry issue: https://govuk-forms.sentry.io/issues/6656667020
This PR makes it so that we check whether there was an error constructing the confirmation email due to the recipient's email address before we try to submit the form. If there is, this results in a user being shown the validation error "Enter an email address in the correct format, like name@example.com" on the check your answers page.
An error is also sent to Sentry with the error message from ActionMailer, but with the email address redacted so we don't store PII.
Testing
I don't know how to make an email that's accepted by our email validation, but not by ActionMailer so I tested locally by manipulating the email in the code after validation to add a new line character and checking the validation error message was shown and an error was sent to Sentry.
Message sent to Sentry:
Things to consider when reviewing