#3523: Standardize all failed email logger error messages - [RH] #3937

therealslimhsiehdy · 2025-07-01T21:37:22Z

Ticket

Resolves #3523

Changes

Standardize the email sending logger errors

Context for reviewers

We are hoping to add more information to loggers when emails don't send out. They are now all logger.errors and have clearer error messages.

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Met the acceptance criteria, or will meet them in a subsequent PR
Created/modified automated tests
Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
Interactions with external systems are wrapped in try/except
Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

Tag gov-designers in this PR's Reviewers section for design review. If code is not user-facing, delete design reviewer checklist
Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Pulled this branch locally and tested it
Verified code meets all checks above. Address any checks that are not satisfied
Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
Checked that all code is adequately covered by tests
Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Meets all designs and user flows provided by design/product
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
(Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

Checked that the design translated visually
Checked behavior. Comment any found issues or broken flows.
Checked different states (empty, one, some, error)
Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
- Chrome
- Microsoft Edge
- FireFox
- Safari
(Rarely needed) Tested as both an analyst and applicant user

References

Code review best practices

Screenshots

github-actions · 2025-07-01T21:41:39Z

🥳 Successfully deployed to developer sandbox rh.

github-actions · 2025-07-01T22:09:39Z

🥳 Successfully deployed to developer sandbox rh.

… rh/3523-standardize

github-actions · 2025-07-01T22:27:50Z

🥳 Successfully deployed to developer sandbox rh.

github-actions · 2025-07-01T22:29:57Z

🥳 Successfully deployed to developer sandbox rh.

erinysong · 2025-07-02T16:39:33Z

Noticed calling email_current_metadata_report will log a success message even if the email fails to send (EmailSendingError). This is currently getting fixed but happy to move that in scope to #1317 referenced in the codebase if that's more appropriate

[02/Jul/2025 16:26:35] ERROR [registrar.management.commands.email_current_metadata_report:91] Failed to send metadata email:
  Subject: metadata_subject.txt
  To: [email protected]
  Error: 
Traceback (most recent call last):
  File "/app/registrar/management/commands/email_current_metadata_report.py", line 82, in email_current_metadata_report
    raise EmailSendingError
registrar.utility.email.EmailSendingError
[02/Jul/2025 16:26:35] INFO [registrar.management.commands.email_current_metadata_report:54] Success! Created domain-metadata-07022025.zip and successfully sent out an email!

erinysong · 2025-07-02T16:42:13Z

Clarifying question: do we want to log the name of the file where we're storing our email subjects or the subject text? I noticed we're logging the filenames eg Subject: metadata_subject.txt and was wondering if this was because logging the subject filename is more helpful to someone (presumably dev?) checking logs

erinysong

left some comments while I'm going through the rest of these! happy to go over them on slack and

erinysong · 2025-07-02T16:52:55Z

src/registrar/management/commands/send_domain_invitations.py

-                f"email did not send successfully to {email_data['email']} "
-                f"for {[domain for domain in email_data['domains']]}"
-                f": {err}"
+                "Failed to send transition domain invitation email:\n"


Got the following error when trying to test this script and prompting an error. I did this locally by automatically throwing an EmailSendingError in the try block's first line. For setup, I created a transition domain and tried running the script to prompt the email error but let me know if I did anything different from its expected use!

Traceback (most recent call last): File "/app/registrar/management/commands/send_domain_invitations.py", line 124, in send_email raise EmailSendingError registrar.utility.email.EmailSendingError During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/app/./manage.py", line 20, in <module> main() File "/app/./manage.py", line 16, in main execute_from_command_line(sys.argv) File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line utility.execute() File "/usr/local/lib/python3.10/site-packages/django/core/management/__init__.py", line 436, in execute self.fetch_command(subcommand).run_from_argv(self.argv) File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 412, in run_from_argv self.execute(*args, **cmd_options) File "/usr/local/lib/python3.10/site-packages/django/core/management/base.py", line 458, in execute output = self.handle(*args, **options) File "/app/registrar/management/commands/send_domain_invitations.py", line 66, in handle self.send_emails() File "/app/registrar/management/commands/send_domain_invitations.py", line 115, in send_emails self.send_email(email_data) File "/app/registrar/management/commands/send_domain_invitations.py", line 141, in send_email f" Subject: {email_data['subject']}\n" KeyError: 'subject'

Oh my gosh great catch - I will edit the line to f" Subject: transition_domain_invitation_subject.txt\n" as we have the subject above, and the way I was trying to pull that info is incorrect

erinysong · 2025-07-02T17:00:05Z

src/registrar/models/domain_request.py

@@ -975,7 +975,7 @@ def _send_status_update_email(

        context: dict -> The context sent to the template

-        send_email: bool -> Used to bypass the send_templated_email function, in the event
+        send_email: bool -> Used to bypass the  function, in the event


Is there a reason we remove the function name here / don't replace it with another function name if send_templated_email is inaccurate?

WHEW another great catch, this was an accident and will add back

erinysong · 2025-07-02T18:13:52Z

src/registrar/models/domain_request.py

+                f"  Subject: {email_template_subject}\n"
+                f"  To: {recipient.email}\n"
+                f"  CC: {', '.join(cc_addresses)}\n"
+                f"  BCC: {bcc_address}",


Think this comma needs to be removed and replaced with a newline! I think it leads to an error in the fstring because the compiler thinks that next part of the fstring is an art to logger.error

app-1 | File "/app/registrar/models/domain_request.py", line 1097, in submit app-1 | self._send_status_update_email( app-1 | File "/app/registrar/models/domain_request.py", line 1037, in _send_status_update_email app-1 | logger.error( app-1 | Message: 'Failed to send status update to creator email:\n Type: submission confirmation\n Subject: emails/submission_confirmation_subject.txt\n To: [email protected]\n CC: \n BCC: ' app-1 | Arguments: (' Error: ',)

erinysong

tested everything else and error messages are updated. thank you for cleaning this up! just had a few comments from the earlier pass

erinysong · 2025-07-02T18:22:11Z

src/registrar/utility/email_invitations.py

-                "Could not send notification email to %s for domain %s",
-                user.email,
-                domain.name,
+        except EmailSendingError as err:


May be out of scope but I noticed when we fail to email domain managers that a manager has been added, we add an alert saying so

But when we remove a domain manager and fail to email the other domain managers, that update doesn't show

Are the alert error messages also in scope of this ticket or is that separate? We can create another ticket if it is out of scope!

Let's create another ticket as this one is focused just on logs and not the error displays! Another great catch btw!!! 💯

therealslimhsiehdy · 2025-07-02T18:33:41Z

Clarifying question: do we want to log the name of the file where we're storing our email subjects or the subject text? I noticed we're logging the filenames eg Subject: metadata_subject.txt and was wondering if this was because logging the subject filename is more helpful to someone (presumably dev?) checking logs

@erinysong I opted to go with the .txt file, as that's what we're passing into send_templated_file and is easier to pull and pulling the subject text could add a bit more complexity than is needed 😅, BUT I'm not hard set on it and open to suggestions!

… rh/3523-standardize

github-actions · 2025-07-02T19:25:58Z

🥳 Successfully deployed to developer sandbox rh.

github-actions · 2025-07-02T19:26:00Z

🥳 Successfully deployed to developer sandbox rh.

Ah sorry I meant to hit "dismiss review" and not "re-request review" bc I had added in the new changes/feedback, sorry!

erinysong · 2025-07-03T16:31:34Z

Clarifying question: do we want to log the name of the file where we're storing our email subjects or the subject text? I noticed we're logging the filenames eg Subject: metadata_subject.txt and was wondering if this was because logging the subject filename is more helpful to someone (presumably dev?) checking logs

@erinysong I opted to go with the .txt file, as that's what we're passing into send_templated_file and is easier to pull and pulling the subject text could add a bit more complexity than is needed 😅, BUT I'm not hard set on it and open to suggestions!

Got it thank you! Totally get that since we read the subject line elsewhere. Just out of curiosity, have we tried running django.template.loader's get_template on the subject text file in these catch statements and if we had, what was the output of running that?

If that doesn't work I'm thinking we can change that field name in the logs to "Subject template" (what we call the param in send_templated_email) since that's a more accurate description of what we're displaying in the log

Update all failed email logger error messages

943d196

github-project-automation bot added this to .gov Product Board Jul 1, 2025

Fix error message unit tests

7365748

therealslimhsiehdy changed the title ~~[#3523] - Standardize all failed email logger error messages - [RH]~~ #3523 - Standardize all failed email logger error messages - [RH] Jul 1, 2025

therealslimhsiehdy requested a review from a team July 1, 2025 22:22

Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…

ff1ae77

… rh/3523-standardize

therealslimhsiehdy marked this pull request as ready for review July 1, 2025 22:23

Forgot to remove a commented out line

dbc5646

therealslimhsiehdy changed the title ~~#3523 - Standardize all failed email logger error messages - [RH]~~ #3523: Standardize all failed email logger error messages - [RH] Jul 1, 2025

erinysong self-assigned this Jul 1, 2025

erinysong previously requested changes Jul 2, 2025

View reviewed changes

github-project-automation bot moved this to 👀 In review in .gov Product Board Jul 2, 2025

erinysong reviewed Jul 2, 2025

View reviewed changes

therealslimhsiehdy added 2 commits July 2, 2025 11:36

Fix logic for metadata and address other needed changes

0ee9acb

Merge branch 'main' of https://github.com/cisagov/manage.get.gov into…

a73f998

… rh/3523-standardize

therealslimhsiehdy requested a review from erinysong July 2, 2025 20:53

#3523: Standardize all failed email logger error messages - [RH] #3937

Are you sure you want to change the base?

#3523: Standardize all failed email logger error messages - [RH] #3937

Uh oh!

Conversation

therealslimhsiehdy commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Ticket

Changes

Context for reviewers

Setup

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

Ensured code standards are met (Original Developer)

Validated user-facing changes (if applicable)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

Validated user-facing changes as a developer

As a designer reviewer, I have

Verified that the changes match the design intention

Validated user-facing changes as a designer

References

Screenshots

Uh oh!

github-actions bot commented Jul 1, 2025

Uh oh!

github-actions bot commented Jul 1, 2025

Uh oh!

github-actions bot commented Jul 1, 2025

Uh oh!

github-actions bot commented Jul 1, 2025

Uh oh!

erinysong commented Jul 2, 2025

Uh oh!

erinysong commented Jul 2, 2025

Uh oh!

erinysong left a comment

Choose a reason for hiding this comment

Uh oh!

erinysong Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

therealslimhsiehdy Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

erinysong Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

therealslimhsiehdy Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

erinysong Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

erinysong left a comment

Choose a reason for hiding this comment

Uh oh!

erinysong Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

therealslimhsiehdy Jul 2, 2025

Choose a reason for hiding this comment

Uh oh!

therealslimhsiehdy commented Jul 2, 2025

Uh oh!

github-actions bot commented Jul 2, 2025

Uh oh!

github-actions bot commented Jul 2, 2025

Uh oh!

erinysong commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

therealslimhsiehdy commented Jul 1, 2025 •

edited

Loading

erinysong commented Jul 3, 2025 •

edited

Loading