Opened 4 years ago

Closed 4 years ago

#23944 closed Uncategorized (wontfix)

Django should use custom exceptions for errors thrown by send_mail (etc.), not smtplib.SMTPException

Reported by: jordanreiter Owned by: nobody
Component: Core (Mail) Version: 1.7
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Since Django allows all kinds of backends — not just SMPT — for sending mail, it should have its own exceptions that different mail backends can subclass.

Otherwise, if you have code like this:

try:
    send_mail(...)
except smptlib.SMTPException:
    # do something

And then switch to a backend that doesn't use smtplib, you won't catch the error.

I think the exceptions should be available from django.core.mail.exceptions and then subclassed within each backend.

Change History (6)

comment:1 Changed 4 years ago by jordanreiter

Type: New featureUncategorized

comment:2 Changed 4 years ago by Tim Graham

It seems to me that applications may want to handle different email exceptions in a particular way. If we start hiding the original exception, that's no longer possible (or at least more difficult). I think having to update the exceptions that your application handles if you switch email backends is reasonable. On the other hand, that might not be feasible for third-party apps. If you have ideas on how you would implement this, sketching that out a bit more would be helpful.

comment:3 Changed 4 years ago by jordanreiter

If you have ideas on how you would implement this, sketching that out a bit more would be helpful.

Sure. I was thinking that django.core.mail.backends.smtp.SMTPException et al would subclass both django.core.mail.exceptions.EmailError and smtplib.SMTPException. That way existing code that caught smtplib.SMTPException would behave the same as it does now, but newer code could catch EmailException instead.

I think it's necessary because there are tons of third party apps that use send_mail and if you write your own backend (or use another library) then you can run into problems.

Obviously until most apps use EmailException they will still fall flat, but right now it's pretty much enshrined that they will break if you're using a non-smtplib-based library.

For example, if you use are using Amazon SES (via boto) as the backend, the exception that comes back is likely to be boto.SESConnection.ResponseError.

Establishing custom exceptions would have two benefits:

  • establishing a standard that developers of non-SMTP mail backends can use when raising exceptions
  • decoupling exceptions from the smtp library altogether

I was thinking that the exceptions that would be available in django.core.mail.exceptions would be EmailError, EmailAuthenticationError, EmailConnectError, RecipientsRefused, and SenderRefused. It seems like these are all functions that most backends would implement and, if they don't, then they wouldn't throw those exceptions and so they don't need to be implemented for that backend.

For the Amazon SES example, failing to send correct API credentials would result in SESAuthenticationError which subclasses EmailAuthenticationError while encountering a situation where for some reason the API was failing to respond correctly would result in SESConnectError which subclasses EmailConnectError. And so on.

comment:4 Changed 4 years ago by Berker Peksag

I don't think subclassing will be enough in some cases. Some exceptions will have additional attributes, and the user may need to catch these exceptions and do something with their attributes. For example:

try:
    send_mail()
except FooException as exc:
    if exc.err_code == 42:
        logger.exception("...")
    raise

With a generic exception, the user will also need to check whether err_code is defined or not:

from django.core.mail.exceptions import EmailError

try:
    send_mail()
except EmailError as exc:
    if getattr(exc, "err_code", 0) == 42:
        logger.exception("...")
    raise

or:

from django.core.mail.exceptions import EmailError

try:
    send_mail()
except EmailError as exc:
    if issubclass(exc, FooException) and exc.err_code == 42:
        logger.exception("...")
    raise

or, catch the FooException explicitly:

from django.core.mail.exceptions import EmailError

try:
    send_mail()
except FooException as exc:
    if exc.err_code == 42:
        logger.exception("...")
    raise
except EmailError:
    pass

comment:5 Changed 4 years ago by Tim Graham

I am not sure there's a compelling use case for third-party apps handling the different types of email failures that makes this extra complexity worthwhile. It seems to me most apps could simply catch Exception if send_mail() fails -- or use fail_silently=True and check the number of messages sent and handle an error based on its needs.

comment:6 Changed 4 years ago by Tim Graham

Resolution: wontfix
Status: newclosed

Please start a discussion on the DevelopersMailingList if you'd like to discuss this idea further.

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