Author r.david.murray
Recipients BreamoreBoy, christian.heimes, gjb1002, gregmalcolm, r.david.murray, rodolpho, stranger4good, werneck, zanella
Date 2010-08-01.04:58:33
SpamBayes Score 2.63171e-09
Marked as misclassified No
Message-id <1280638721.0.0.919258902823.issue2118@psf.upfronthosting.co.za>
In-reply-to
Content
A little backstory: the patch was created during the PyOhio sprint, and as we recreated the patch for py3k we noticed that SMTPConnectError is providing an *SMTP* error code.  The gaierrors are obviously not SMTP errors.  To allow the agreed upon semantics (catching SMTPConnectError catches errors resulting from attempting to establish a session), we created a subclass of SMTPConnectError called SMTPSocketConnectError that has errno and strerror attributes instead of smtp_code and smtp_error. The giaerror errno is not an SMTP error code, and can not be guaranteed to be a disjoint set of number, nor is there an SMTP error code that we can return and then put the errno in an appropriate message.  Obviously code that inspects smtp_code will need to handle this subclass specially, but since previously it had to handle the giaerror separately this does not seem like a big issue.  Code that only cares about catching the error and possibly using the str of the error does not need to care.

An alternative to this patch would be a doc fix patch that changed the wording to indicate that it is not *all* exceptions that are caught (which certainly isn't true...we raise ValueError for an invalid port number, and that seems correct), but rather all *SMTP protocol* related exceptions.  However, it does seem very convenient to be able to just catch SMTPConnectError in order to catch any errors resulting from an attempt to establish a connection using valid parameters.

Two further things to note: neither this patch nor the original patch actually fix the bug reported by the OP, nor does the test code test it.  In the original report it is getreply that is raising the error, and the patches do not wrap getreply in the try/except.  Because testing this case appears non-trivial, we elected not to tackle it, but just reimplemented the original patch, which does catch one class of socket related connect errors.  If the subclassed-error approach is accepted, the patch can be expanded to wrap the getreply call and test it as well.

The second thing is a response to Breamorboy: the debug code does indeed have slightly different semantics in Python3 (good catch), but that's a separate issue :)  (The print statement is because it was a patch against py2.)  It really only needs a doc fix. The debug code is useful for debugging, and it is part of the public API of the module.  It would be nice to have it unit tested, but it isn't as big a deal as other code since it is unlikely to be used in production, only during development.

Finally, if the approach in this patch is accepted it can only go into 3.2, since it represents a significant behavior change.
History
Date User Action Args
2010-08-01 04:58:42r.david.murraysetrecipients: + r.david.murray, gjb1002, christian.heimes, stranger4good, zanella, werneck, rodolpho, BreamoreBoy, gregmalcolm
2010-08-01 04:58:40r.david.murraysetmessageid: <1280638721.0.0.919258902823.issue2118@psf.upfronthosting.co.za>
2010-08-01 04:58:38r.david.murraylinkissue2118 messages
2010-08-01 04:58:33r.david.murraycreate