| msg62416 - (view) |
Author: (stranger4good) |
Date: 2008-02-14 22:23 |
| smtplib.SMTP() raises socket.error rather than SMTPConnectError just try this on a non-responding address >>> srv=smtplib.SMTP('192.168.13.22') Traceback (most recent call last): File "", line 1, in File "c:\python25\lib\smtplib.py", line 244, in __init__ (code, msg) = self.connect(host, port) File "c:\python25\lib\smtplib.py", line 311, in connect (code, msg) = self.getreply() File "c:\python25\lib\smtplib.py", line 352, in getreply line = self.file.readline() File "C:\Python25\lib\socket.py", line 346, in readline data = self._sock.recv(self._rbufsize) socket.error: (10054, 'Connection reset by peer') |
|
|
| msg62423 - (view) |
Author: Christian Heimes (christian.heimes) *  |
Date: 2008-02-15 08:16 |
| Why is this a bug? Do the docs promise that smtplib only raises SMTPConnectError? |
|
|
| msg62447 - (view) |
Author: (stranger4good) |
Date: 2008-02-16 01:38 |
| Not in so many words, but yes it does I cite: exception SMTPException Base exception class for all exceptions raised by this module ... exception SMTPConnectError Error occurred during establishment of a connection with the server. ... Note the word "all". So, at least one should be able to catch SMTPException, but it is not the case Just my 2 cents. I do believe it is a great piece of work. Regards --- Christian Heimes <report@bugs.python.org> wrote: > > Christian Heimes added the comment: > > Why is this a bug? Do the docs promise that smtplib > only raises > SMTPConnectError? > > ---------- > nosy: +tiran > priority: -> normal > > __________________________________ > Tracker <report@bugs.python.org> > <http://bugs.python.org/issue2118> > __________________________________ > ____________________________________________________________________________________ Be a better friend, newshound, and know-it-all with Yahoo! Mobile. Try it now. http://mobile.yahoo.com/;_ylt=Ahu06i62sR8HDtDypao8Wcj9tAcJ |
|
|
| msg62451 - (view) |
Author: Christian Heimes (christian.heimes) * |
Date: 2008-02-16 07:39 |
| I see! You are right. Either the docs or the code need an update |
|
|
| msg62747 - (view) |
Author: Pedro Werneck (werneck) |
Date: 2008-02-23 15:23 |
| It seems the right thing to do would be to have it raise a base exception, but SMTPConnectError docstring states "Error during connection establishment.", so I chosen to use it with the errno and message from socket.error, even if it's supposed to happen only after connection is already stablished, since it's a subclass of SMTPResponseException. As a bonus I removed a explicit raise socket.error from line 290, when an user pass a non-integer port. |
|
|
| msg62785 - (view) |
Author: Pedro Werneck (werneck) |
Date: 2008-02-23 17:50 |
| Previous patch didn't passed the tests right. This patch fixes both the code, unindenting port number conversion to integer and the test. |
|
|
| msg68515 - (view) |
Author: Rodolpho Eckhardt (rodolpho) |
Date: 2008-06-21 16:50 |
| Verified Werneck's patch and it also works on 2.6 and 3.0. However, the previous code used to present a "friendly" message about non-numeric ports: "socket.error: nonnumeric port" and now it raises "ValueError: invalid literal for int() with base 10". Should it be changed to inform that the exception is due to a non-numeric port? (Just wrap int(port) with a try and change the raised exception) |
|
|
| msg110631 - (view) |
Author: Mark Lawrence (BreamoreBoy) * |
Date: 2010-07-18 11:09 |
| I applied the patch to test_smtplib.py only and all tests passed, surely that's not correct. Removing the explicit test for a non-numeric port in smtplib.py seems strange to me, could someone please explain the rationale behind this. I also noticed this line in the patch. if self.debuglevel > 0: print>>stderr, "connect:", msg And this comment in set_debuglevel in smtplib.py "A non-false value results in debug messages for connection and for all messages sent to and received from the server." IIRC if a string was passed into set_debuglevel the comparison would fail in py3k, am I correct? The print statement would not work in py3k. Given that the debuglevel code obviously doesn't get tested perhaps it should simply be stripped from the code? |
|
|
| msg112261 - (view) |
Author: Greg Malcolm (gregmalcolm) |
Date: 2010-08-01 03:32 |
| I've uploaded a patch for 3.2 that throws a ValueError exception for non numeric port numbers and SMTPSocketConnectError for socket connection failures. This patch introduces an API change, by creating the SMTPSocketConnectError which provides information about the socket error. The API documentation will therefore need to be updated if this patch is accepted. |
|
|
| msg112264 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-08-01 04:58 |
| 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. |
|
|
| msg116266 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2010-09-13 01:19 |
| I discussed this issue with Antoine Pitrou on #python-dev, and his opinion is that SMTPSocketConnectError doesn't add enough value to be worthwhile. So he is in favor of making this a doc fix. However, the suggestion also came up to have SMTPException subclass from IOError instead of Exception, since every place where an SMTPException is raised IO is involved. This change would mean that code that didn't care what kind of IO error causes the connection to fail could trap just IOError, and code that did care could trap SMTPConnectError and IOError separately. It is possible this would have backward compatibility issues, so the base class change is probably suitable only for 3.2, but a doc change clarifying that non-SMTP errors can be raised by connect should be backported. |
|
|
| msg186744 - (view) |
Author: Ned Jackson Lovely (n) * |
Date: 2013-04-13 16:12 |
| Attaching a patch to make SMTPException an IOError, with corresponding update to docs to point out that __init__ on the SMTP object will raise IOErrors in general, and some SMTPExceptions in particular. Boston Python Sprint Apr 2013 |
|
|
| msg186748 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-04-13 16:26 |
| This looks good for 3.4. Ned, would you also be willing to prepare a doc patch for 3.3 that mentions that IOError may be raised? (I think the 3.3 patch will also apply to 2.7.) |
|
|
| msg186758 - (view) |
Author: Ned Jackson Lovely (n) * |
Date: 2013-04-13 16:47 |
| ...and the 3.3 doc patch. |
|
|
| msg186792 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2013-04-13 18:43 |
| New changeset 6ca27b5de309 by R David Murray in branch '2.7': #2118: clarify smtplib exception documentation. http://hg.python.org/cpython/rev/6ca27b5de309 New changeset 36d07a877f33 by R David Murray in branch '3.3': #2118: clarify smtplib exception documentation. http://hg.python.org/cpython/rev/36d07a877f33 New changeset 75e7bd46fcd7 by R David Murray in branch 'default': Merge #2118: clarify smtplib exception documentation. http://hg.python.org/cpython/rev/75e7bd46fcd7 |
|
|
| msg186795 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-04-13 18:50 |
| New changeset 1158c38bf2dc by R David Murray in branch 'default': #2118: Make SMTPException a subclass of IOError. http://hg.python.org/cpython/rev/1158c38bf2dc |
|
|
| msg186797 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-04-13 18:54 |
| Thanks, Ned. After taking a look at the library documentation, I decided to change the docstring in a somewhat different way. In general in our docs we do not document all the possible exceptions a library can raise, but just the library-specific exceptions and the interesting special cases. So what I did was to remove the language that implied that *only* SMTPExceptions were raised. And then in the 3.4 docs noted that it is a subclass of IOError. |
|
|
| msg186810 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-04-13 19:50 |
| Since 3.3 IOError is an alias of OSError. We get rid of reference of IOError and other OSError aliases in the documentation and code, replacing them with OSError. |
|
|
| msg186812 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2013-04-13 19:54 |
| socket.error is another alias of OSError. |
|
|
| msg186841 - (view) |
Author: Andrew Svetlov (asvetlov) * |
Date: 2013-04-13 20:52 |
| Agree with Serhiy. Better to direcly use OSError than IOError alias. |
|
|
| msg186850 - (view) |
Author: Ned Jackson Lovely (n) * |
Date: 2013-04-13 21:13 |
| s/IOError/OSError/ |
|
|
| msg186908 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2013-04-14 10:46 |
| New changeset adc72ff451dc by R David Murray in branch 'default': #2118: IOError is deprecated, use OSError. http://hg.python.org/cpython/rev/adc72ff451dc |
|
|