[issue1186900] nntplib shouldn't raise generic EOFError

[Irit Katriel]

Irit Katriel  added the comment:

nntplib is deprecated as per PEP 594, so there won't be further enhancements to 
it.

--
nosy: +iritkatriel
resolution:  -> wont fix
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Martin Panter]

Martin Panter added the comment:

NNTPConnectError does still seem a slightly awkward name. I would go for 
NNTPConnectionError instead, but I’m also happy to put my bikeshed paint away 
let this patch be applied as is :)

Handling of NNTPTemporaryError with a code of 400 is similar to handling of 
this EOFError. But I guess there is not much you could do with the API unless 
you made a separate subclass for 400 errors (like all the new 
EnvironmentError/OSError subclasses), which would be rather severe. My current 
workaround looks a bit like this:

try:
[_, info] = nntp.body(...)
except NNTPTemporaryError as err:
[code, *msg] = err.response.split(maxsplit=1)
if code != 400:
raise
except EOFError:  # Or NNTPConnect(ion)Error
msg = ()
else:
break  # Handle successful response
[msg] = msg or (Server shut down connection,)
# Handle connection shutdown by server

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Martin Panter]

Martin Panter added the comment:

Some more points:

* I suggest adding something like this to the documentation:

exception nntplib.NNTPConnect[ion]Error
  Exception raised when the server unexpectedly shuts down the connection.

* The tests should use BytesIO rather than StringIO. Other than that, I think 
monkey-patching the file attribute in the tests is fine; that’s probably the 
way I would do it!

* The new exception should be added to __all__.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Martin Panter]

Martin Panter added the comment:

PPS: Documentation should probably have the “New in version 3.5” tag as well

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

Thank yo so much, Martin! I will incorporate these changes and add them soon!

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Antoine Pitrou]

Antoine Pitrou added the comment:

Indeed, I find the description rather vague and potentially misleading. 
Connection establishment would usually refer to the TCP connection, but an 
EOFError is actually a high-level, logical error (it has nothing to do with 
networking per se: it's probably more of a server failure - or a client bug 
perhaps :-)).

--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

I'ved changed the comment to say Connection closed unexpectedly.

--
Added file: http://bugs.python.org/file36264/nntplib_error_v2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Martin Panter]

Martin Panter added the comment:

I could be wrong, but isn’t this error raised when expecting a response from 
any command, not just during “connection establishment”? Perhaps change the 
docstring to say something like “Connection closed unexpectedly” instead.

--
nosy: +vadmium

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Jessica McKellar]

Changes by Jessica McKellar jesst...@mit.edu:


--
keywords: +needs review
stage: test needed - patch review

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Berker Peksag]

Berker Peksag added the comment:

diff -r 8f85262fbe8a Lib/nntplib.py
--- a/Lib/nntplib.pySun Jul 06 02:24:24 2014 -0400
+++ b/Lib/nntplib.pyThu Jul 10 16:10:38 2014 -0700
@@ -122,6 +122,9 @@
 Error in response data
 pass
 
+class NNTPConnectError(NNTPError):
+Error during connection establishment.
+pass

Could you also document the new exception? (See Doc/library/nntplib.rst and 
please add a versionadded directive)

The pass statement is redundant, but we could keep it to be consistent with 
rest of the library.


@@ -435,7 +438,7 @@
 raise NNTPDataError('line too long')
 if self.debugging  1:
 print('*get*', repr(line))
-if not line: raise EOFError
+if not line: raise NNTPConnectError()

if not line:
raise NNTPConnectError

looks more readable to me.

Also, you could add a more descriptive error message.

--
nosy: +berker.peksag
versions: +Python 3.5 -Python 3.2

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Ezio Melotti]

Ezio Melotti added the comment:

Wouldn't this be backward incompatible?
Even if the EOFError that is raised is not documented explicitly, people might 
be catching it, and switching to a new exception would break their programs.  
Maybe NNTPConnectError should inherit from EOFError too?

--
nosy: +ezio.melotti

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

That's a good point. I can add that so the NNTPConnectError can inherit the 
EOFError

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

Here is an updated patch.

--
Added file: http://bugs.python.org/file35941/nntplib_error.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

I am going to fix it so that it raises the NNTPConnectionError rather than 
update the documentation.

--
nosy: +Lita.Cho, jesstess

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Lita Cho]

Lita Cho added the comment:

I have a fix and added some test coverage in order to make sure the 
NNTFConnectError was being called. However, in the test case, I am monkey 
patching. If there is a way to do this with mock, I would appreciate the 
feedback.

--
keywords: +patch
Added file: http://bugs.python.org/file35922/nntplib_error.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Mark Lawrence]

Changes by Mark Lawrence breamore...@yahoo.co.uk:


--
nosy:  -BreamoreBoy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Ankur Ankan]

Changes by Ankur Ankan ankuran...@gmail.com:


--
nosy: +Ankur.Ankan

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Mark Lawrence]

Mark Lawrence breamore...@yahoo.co.uk added the comment:

The OP would accept a documentation change if the code's not going to be 
changed.

--
nosy: +BreamoreBoy
versions: +Python 3.2 -Python 2.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue1186900] nntplib shouldn't raise generic EOFError

[Daniel Diniz]

Changes by Daniel Diniz aja...@gmail.com:


--
stage:  - test needed
versions: +Python 2.7

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue1186900
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com