Opened 7 years ago

Closed 6 years ago

Last modified 6 years ago

#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 incorrectlydjango.core.mail.send_mail()'s fail_silently kwarg is documented confusingly

comment:3 by Tim Graham, 7 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

in reply to:  description comment:4 by Sub, 7 years ago

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 if 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 ティン・ルーフ:

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?

Version 0, edited 7 years ago by Sub (next)

comment:5 by Sub, 7 years ago

Owner: changed from nobody to Sub
Status: newassigned

comment:6 by Tim Graham <timograham@…>, 6 years ago

Resolution: fixed
Status: assignedclosed

In 085ebc5f:

Fixed #29430 -- Clarified send_mail()'s fail_silently docs.

comment:7 by Tim Graham <timograham@…>, 6 years ago

In 84afe81b:

[2.1.x] Fixed #29430 -- Clarified send_mail()'s fail_silently docs.

Backport of 085ebc5f1a0e96784516e551cb9225cc6836f1d0 from master

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