#29430 closed Cleanup/optimization (fixed)
django.core.mail.send_mail()'s fail_silently kwarg is documented confusingly
| Reported by: | ティン・ルーフ | Owned by: | Sub |
|---|---|---|---|
| Component: | Documentation | Version: | 2.0 |
| Severity: | Normal | Keywords: | |
| Cc: | Triage Stage: | Accepted | |
| Has patch: | no | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | yes | UI/UX: | no |
Description (last modified by )
From https://docs.djangoproject.com/en/2.0/topics/email/#send-mail: “If it’s False, send_mail will raise an smtplib.SMTPException.”
This is often untrue, and certainly misleading. Certainly, if it's False, send_mail is capable of raising SMTPException, but it's not guaranteed.
I was going to open a pull request and change it to “If it's True, send_mail can raise smtplib.SMTPException”, but I'm not confident that other exceptions cannot be raised, or that send_mail will never raise SMTPException if fail_silently is True.
The main thing I would like would be for the intent of fail_silently to be expressed; what kind of errors is it trying to suppress, for example? What kind of emails could a Django site be sending that are unimportant enough that failing silently is a good idea?
Change History (7)
comment:1 by , 7 years ago
| Description: | modified (diff) |
|---|
comment:2 by , 7 years ago
| Summary: | django.core.mail.send_mail()'s fail_silently kwarg is documented incorrectly → django.core.mail.send_mail()'s fail_silently kwarg is documented confusingly |
|---|
comment:3 by , 7 years ago
| Triage Stage: | Unreviewed → Accepted |
|---|---|
| Type: | Uncategorized → Cleanup/optimization |
comment:5 by , 7 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
From an initial glance, I can see that fail_silently is only used in the smtp.py service file to decide whether an exception should be raised.
See lines 127-128: https://github.com/django/django/blob/5a6f70b4281817656db2f36c5919036d38fcce7f/django/core/mail/backends/smtp.py
The exception to be raised in this case is smtplib.SMTPException. I agree that it does not guarantee that this exception will be raised, and could be misinterpreted. Maybe it would be clearer as "If it’s False, send_mail will raise an smtplib.SMTPException when an error occurs." I think it should definitely be explaining the False case, (not True) as we want to explain the what happens if the failure isn't ignored.
Replying to ティン・ルーフ: