CONNECTION_LOST not an Integer...

[mforbes]

Several docstrings, notably those for the writeSomeData() method of t.i.a.FileDescriptor, claim that the return value is an integer reporting the number of bytes sent, or a negative integer reporting that the connection was lost.

The implementations, however, return t.i.main.CONNECTION_LOST which is an exception, not an integer.

Apart from the incorrect docstrings, returning an exception makes it quite difficult to test for errors. For example:

bytes_sent = self.connection.writeSomeData(...)
if 0 < bytes_sent:
    Still get here, even if connection is lost.

One has to check if the return value is explicitly CONNECTION_LOST, or if it is an instance of int etc.

Suggestion: Have writeSomeData throw the exception, or revert back to using an integer...

A quick grep indicates that this would require changes in the following files.

./Twisted/twisted/internet/_javaserialport.py ./Twisted/twisted/internet/_posixserialport.py ./Twisted/twisted/internet/abstract.py ./Twisted/twisted/internet/fdesc.py ./Twisted/twisted/internet/process.py ./Twisted/twisted/internet/tcp.py ./Twisted/twisted/protocols/htb.py ./Twisted/twisted/protocols/pcp.py ./Twisted/twisted/test/test_internet.py ./Twisted/twisted/test/test_pcp.py

[therve]

Sorry, this is a documentation issue, but the behavior won't change, for several reasons (compatibility, performance, and others). Note that the fix is quite simple:

bytes_sent = self.connection.writeSomeData(...)
if bytes_sent < 0 or isinstance(bytes_sent, Exception):

[exarkun]

(In [25420]) Branching to 'writesomedata-docs-3455'

[exarkun]

Turns out, no implementation of writeSomeData returns a negative integer to indicate connection loss.

I could include this hunk in the branch if I wanted:

twisted/internet/abstract.py

@@ -111 +111 @@
             l = self.writeSomeData(buffer(self.dataBuffer, self.offset))
         else:
             l = self.writeSomeData(self.dataBuffer)
-        if l < 0 or isinstance(l, Exception):
+        if isinstance(l, Exception):
             return l
         if l == 0 and self.dataBuffer:
             result = 0

without breaking any of Twisted's unit tests, but I'm not going to because perhaps some third-party writeSomeData implementation decided to take the negative return value documentation to heart and do that instead of returning an exception.

So I've just corrected the documentation to say that writeSomeData may returned an integer or an exception and dropped all the text about negative integers.

[mwh]

Looks fine to me. Please land.

[exarkun]

(In [25452]) Merge writesomedata-docs-3455

Author: exarkun Reviewer: mwhudson Fixes: #3455

Change the API documentation for writeSomeData so that it does not claim negative values are returned to indicate connection lose. Instead document that an exception instance will be returned to indicate that.