Opened 3 weeks ago

Closed 2 weeks ago

Last modified 13 days ago

#37261 closed Bug (needsinfo)

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

Reported by: Adam Johnson Owned by: Adam Johnson
Component: Core (Mail) Version: 6.1
Severity: Release blocker Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

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!

Change History (7)

comment:1 by Mike Edmunds, 3 weeks ago

Resolution: invalid
Status: assignedclosed

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).

in reply to:  1 comment:2 by Adam Johnson, 3 weeks ago

Replying to 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 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 `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.

comment:3 by Adam Johnson, 3 weeks ago

Resolution: invalid
Status: closednew

Right, I spotted in the fail_silently thread, 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.

comment:4 by Mike Edmunds, 3 weeks ago

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.

comment:5 by Adam Johnson, 3 weeks ago

It totally makes sense to me to raise configuration errors in most use cases, but not where the emails are best-effort debugging aids, like in the two features here.

in reply to:  5 comment:6 by Natalia Bidart, 2 weeks ago

Resolution: needsinfo
Status: newclosed

Replying to Adam Johnson:

It totally makes sense to me to raise configuration errors in most use cases, but not where the emails are best-effort debugging aids, like in the two features here.

Hello! A few thoughts: I don't share the "best-effort debugging aid" framing. ADMINS and MANAGERS are both empty by default and, sendtestemail aside, their only effect is to turn on exactly these emails. A project that sets them has made a deliberate choice to be notified. For anything genuinely best-effort you would reach for logging, error tracking, or telemetry that already routes into an alerting system. So a mailer that has silently stopped delivering is a real operational problem, and Mike's concern about hiding it is one I share: except Exception: pass means the notifications you asked for stop arriving and nothing tells you. We need loud failures in this case.

Before considering next steps, is there a report of this actually affecting a production project? If someone has run into this with a real provider outage or a misconfigured mailer in 6.1, that would be helpful to know and understand, to settle both the severity and the question of which exceptions matter in practice. Mike's point in comment:4 that we never identified a concrete case for ignoring expected SMTP failures is still kind of unanswered.

comment:7 by Mike Edmunds, 13 days ago

There's perhaps an argument that this should be covered explicitly in the deprecation notes and migration docs. Something along the lines of:

  • If a project is using AdminEmailHandler and/or BrokenLinkEmailsMiddleware
  • and it depends on the (undocumented, but pre-existing) behavior that those silently ignore transmission errors to ADMINS/MANAGERS (or that they ignore certain SMTP configuration errors)
  • then when the project opts into MAILERS, it will need to define a custom MAILERS config with "fail_silently": True in the OPTIONS and use that config for AdminEmailHandler's using argument or a BrokenLinkEmailsMiddleware subclass that overrides the using attribute.

[I'd also be tempted to suggest those projects could benefit from switching to infrastructure that queues and retries intermittent email failures, rather than relying on ignoring them. Particularly if they send any other email at all.]

I kind of think documentation is the best we can do for this. I can't really come up with a reliable, useful way to issue deprecation warnings for "you seem to be relying on AdminEmailHandler or BrokenLinkEmailsMiddleware ignoring transmission errors, which will change in Django 7.0."

Note: See TracTickets for help on using tickets.
Back to Top