[Django] #37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport errors when MAILERS is configured

[Django]

#37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport
errors when MAILERS is configured
-------------------------------------------+------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson
Type: Bug | Status: assigned
Component: Core (Mail) | Version: 6.1
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 1
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------------+------------------------------
Since #35514, Django’s built-in error reporting emails (admin emails and
broken-link emails) raise on any transport error when the new dictionary-
based `MAILERS` setting is configured, instead of failing silently as they
did in Django 6.0.

Specifically:

* `AdminEmailHandler.emit()` has no surrounding try/except, and Python's
logging
machinery does not contain exceptions raised by `emit()`, so the
transport error is
raised at the `logger.error(...)`/`logger.exception(...)` call site —
i.e. inside
Django's own error-handling path while reporting a 500.

* `BrokenLinkEmailsMiddleware.process_response()` raises on every referred
404,
turning it into a 500.

The new "Migrating email to mailers" howto recommends wrapping sends in an
error handler with `try:` / `except Exception: pass` "to avoid cascading
failures inan error handler that sends mail".
We need to apply that advice to Django's own built-in error handlers!
--
Ticket URL: <https://code.djangoproject.com/ticket/37261>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport
errors when MAILERS is configured

---------------------------------+----------------------------------------

Reporter: Adam Johnson | Owner: Adam Johnson

Type: Bug | Status: closed

Component: Core (Mail) | Version: 6.1

Severity: Release blocker | Resolution: invalid

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

---------------------------------+----------------------------------------
Changes (by Mike Edmunds):

* resolution: => invalid
* status: assigned => closed

Comment:

This is as designed. See
https://github.com/django/deps/blob/main/accepted/0018-mailers.md
#fail_silently-compatibility.

There's more discussion of the rationale behind this and several
alternatives we considered in the forum: https://forum.djangoproject.com/t
/deprecating-or-changing-how-django-core-mail-handles-fail-
silently/44278/19 (and the lengthy series of preceding comments).
--
Ticket URL: <https://code.djangoproject.com/ticket/37261#comment:1>

[Django]

#37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport
errors when MAILERS is configured
---------------------------------+----------------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson
Type: Bug | Status: closed
Component: Core (Mail) | Version: 6.1
Severity: Release blocker | Resolution: invalid
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Comment (by Adam Johnson):

Replying to [comment:1 Mike Edmunds]:

> This is as designed. See
https://github.com/django/deps/blob/main/accepted/0018-mailers.md
#fail_silently-compatibility.
>
>

There, I see:

> Django's two internal uses of fail_silently=True will be replaced with
MailerDoesNotExist checks. (See AdminEmailHandler and
BrokenLinkEmailsMiddleware earlier.)

But I don't see rationale about why not `try: ... except Exception: ...`
instead.


Replying to [comment:1 Mike Edmunds]:

> There's more discussion of the rationale behind this and several
alternatives we considered in the forum: https://forum.djangoproject.com/t
/deprecating-or-changing-how-django-core-mail-handles-fail-
silently/44278/19 (and the lengthy series of preceding comments).
>

I read the thread, and it seems to cover `fail_silently` in general but
not these two specific use cases.

---

I understand that removing `fail_silently` was completely agreed-on, but I
think dropping its "continue on failure" behaviour in these two specific
cases was overreach. To me, `AdminEmailHandler` and
`BrokenLinkEmailsMiddleware` only send emails as a best-effort. The
failure to send an email should NOT crash the process/request:

* `AdminEmailHandler`: A process that tries to log to emails admins should
continue regardless of whether the email got through. Other logging
handlers are peppered with `try: ... except Exception: pass`. *All* of
Python’s built-in handler classes do this - see the `emit()` methods in
the `logging/handler.py` source, including
[https://github.com/python/cpython/blob/87b120fdb58afd8f7cb79daa87122334e295b70c/Lib/logging/handlers.py#L1149-L1150
`SMTPHandler.emit()`]

* `BrokenLinkEmailsMiddleware`: A broken link should 404 regardless of
whether the email could be sent. Crashing and returning an error page is
unacceptable degradation when an email service provider is down, and could
cause search indexers to start treating a given page differently.
--
Ticket URL: <https://code.djangoproject.com/ticket/37261#comment:2>

[Django]

#37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport
errors when MAILERS is configured
---------------------------------+----------------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson

Type: Bug | Status: new

Component: Core (Mail) | Version: 6.1
Severity: Release blocker | Resolution:

Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Changes (by Adam Johnson):

* resolution: invalid =>
* status: closed => new

Comment:

Right, I spotted in the fail_silently thread,
[https://forum.djangoproject.com/t/deprecating-or-changing-how-django-
core-mail-handles-fail-silently/44278/8 post 8], you replied to Jacob:

> Jacob:
> > I’m writing a reusable app: I’d like to send some mail, so long as my
users have email configured at all. [emphasis added]
>
> :light_bulb:Ah-hah! That’s what I had been missing, and totally makes
sense to me. It would explain why Django uses fail_silently=True in
AdminLogHandler and BrokenLinksMiddleware, as well as several examples I
saw in the wild.

It seems the only rationale for the `fail_silently` usage in these two
features that was discussed in the thread was this one. It doesn't cover
my one: "these features shouldn't break things when email providers are
down".

I think therefore we need input from others.
--
Ticket URL: <https://code.djangoproject.com/ticket/37261#comment:3>

[Django]

#37261: AdminEmailHandler and BrokenLinkEmailsMiddleware raise on email transport
errors when MAILERS is configured
---------------------------------+----------------------------------------
Reporter: Adam Johnson | Owner: Adam Johnson
Type: Bug | Status: new
Component: Core (Mail) | Version: 6.1
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+----------------------------------------

Comment (by Mike Edmunds):

The exact earlier behavior is available with `MAILERS` by configuring the
default mailer with `"OPTIONS": {"fail_silently": True`}`. fwiw, it
ignores some (but not all) SMTP configuration errors, and some (but not
all) send-time errors. That SMTP backend configuration option is not
deprecated (although it is discouraged in the docs).

There was general agreement that `AdminEmailHandler` and
`BrokenLinkEmailsMiddleware` should ''not'' silently swallow configuration
errors that prevented sending the error emails altogether, and it was a
historic accident that they did in earlier releases. A similar argument
applied to ''unexpected'' SMTP transmission failures. We didn't identify a
use case for ''expected'' SMTP failures that should be ignored. It sounds
like you may have one, but I'm not certain it generalizes to all email
backends. Ignoring ''all'' exceptions is considerably more broad and
(imho) problematic because it hides configuration problems.

Agreed, more discussion is needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/37261#comment:4>