| kostajh | |
| Oct 2 2023, 10:51 AM2023-10-02 10:51:58 (UTC+0) |
We don't log errors when we send emails. Instead we throw a generic exception for the caller to the API.
We shouldn't expose internal details to the caller of the API, but we should log errors so that we can determine what the specific problem is.
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Log when an email fails to send | mediawiki/extensions/ReportIncident | master | +141 -30 |
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | kostajh | T337566 [EPIC]: Incident Reporting System - Minimal Testable Product (MTP) | |||
| Resolved | Dreamy_Jazz | T347843 [S] Log errors from email sending failures |
A failed email generates a corresponding log entry with error details, in the ReportIncident channel at the error level
Should this also include when the email is not sent due to the sender email / recipient email(s) not being defined? Currently these are logged as warnings.
In T347843#9231759, @Dreamy_Jazz wrote:A failed email generates a corresponding log entry with error details, in the ReportIncident channel at the error level
Should this also include when the email is not sent due to the sender email / recipient email(s) not being defined? Currently these are logged as warnings.
Yes, it would make sense to log at error level.
Change 969969 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):
[mediawiki/extensions/ReportIncident@master] Log when an email fails to send
Change 969969 merged by jenkins-bot:
[mediawiki/extensions/ReportIncident@master] Log when an email fails to send
This cannot be QA'd on patchdemo or betawikis, as it relies on accessing the server logs.
I propose that considering the queue of QA tickets, this being a small change that made no user facing changes, and @Djackson-ctr not having a local environment, this can ticket can be skipped for QA. Leaving up to the QA team to determine. Can write local environment specific steps if you would prefer.
Yes I agree with you @Dreamy_Jazz that this can skip QA... I will push it to Done. Thank You, sir.
