Make the SMTP more efficient if an error passes silently when creating a connection

[Antonis Christofides]

My server had a 500 error, and Django was trying to email the error information to the ADMINS. Upon reaching ​https://github.com/django/django/blob/0ed7d15/django/core/mail/backends/smtp.py#L64, there was an exception because of a gevent bug.

In that case, fail_silently is True. The result is that the open() method returns as if everything had gone fine. This means that, subsequently, the send_messages() method will be called, and so on.

Impact: Maybe this is just a bit ugly, however if you continue to work as if there had been no error when an error has actually occurred, you are asking for trouble. ​http://stackoverflow.com/questions/35315397/ shows why this issue made debugging another problem a day or two longer.

It should also be possible to log the error that occurs during the sending of the email, although this is probably a different issue.

[Tim Graham]

This looks like intentional behavior as described in #19637. I'm not sure what change we could make other than document it. Do you have a proposal?

[Antonis Christofides]

It is intentional that the failure of the emailing be silent. However, the proper way of achieving this is the following pseudocode:

try:
    connect_to_the_mail_server();
    say_ehlo();
    start_tls();
    say_ehlo();
    login();
    specify_mailfrom_rcptto_and_data();
    close_connection_to_the_mail_server();
except:
    pass;

Instead, what it now does is this:

try:
    connect_to_the_mail_server();
    say_ehlo();
    start_tls();
    say_ehlo();
    login();
except:
    pass;

try:
    specify_mailfrom_rcptto_and_data();
except:
    pass;

try:
    close_connection_to_the_mail_server();
except:
    pass;

(The first block is what open() does, the second is what send_messages() does, and the third is what close() does; these are methods in django.core.mail.backend.smtp.EmailBackend.)

Call me a perfectionist (with deadlines), but it's really ugly to proceed with the second block if an error has occurred on the first block. I haven't seen any really bad consequences (other than that it made it harder for me to debug an unrelated problem I was having), but it's risky.

It's not hard to fix with a little restructuring of the code (unit testing such restructuring is the hardest part), but I'm not volunteering to do it, sorry :-)

[Tim Graham]

I think the attached patch might do it. We'll just need a test.

[Tim Graham]

​PR looks good, pending a few cosmetic cleanups.

[Tim Graham <timograham@…>]

In 42dc9d04:

Fixed #26210 -- Prevented SMTP backend from trying to send mail after a connection failure.