Opened 10 years ago
Closed 10 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 by , 10 years ago
Type: | New feature → Uncategorized |
---|
comment:2 by , 10 years ago
comment:3 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
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 by , 10 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
Please start a discussion on the DevelopersMailingList if you'd like to discuss this idea further.
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.