Opened 11 years ago
Closed 11 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 , 11 years ago
| Type: | New feature → Uncategorized |
|---|
comment:2 by , 11 years ago
comment:3 by , 11 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 , 11 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 , 11 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 , 11 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.