#2215 defect new
If lineReceived returns true value, the connection is shut down and this value used as an error message
|Reported by:||Marcin Kasperski||Owned by:|
Description (last modified by )
In short: if lineReceived returns some true value (personally I met this problem when I returned deferred), then the whole connection is aborted, moreover, this value is returned as error message. It seems this behaviour is not documented anywhere.
class SomeProtocol(basic.LineReceiver): (...) def lineReceived(self, line): (...) return defer.succeed(997) # magic value I can recognize easily class SomeFactory(protocol.ReconnectingClientFactory): (...) def clientConnectionLost(self, connector, reason): logger.warn("Connection lost. Reason: %s\nTraceback: %s" % ( reason.getErrorMessage(), reason.getTraceback())) protocol.ReconnectingClientFactory.clientConnectionLost(self, connector, reason) reactor.connectTCP(HOST, PORT, SomeFactory(...)) reactor.run()
myapp : WARNING Connection lost. Reason: <Deferred at 0xB551E1ECL current result: 997> Traceback: Traceback (most recent call last): Failure: <type 'instance'>: <Deferred at 0xB551E1ECL current result: 997>
(minimal solution) Document this behavior, so people using lineReceiver know that returning value from this method is forbidden.
(better solution) Remove this ugly convention, use exceptions for error reporting instead, and let people return any results they like.
The crucial place in twisted code seems to be twisted/protocols/basic.py, function LineReceiver.dataReceived, which contains sth like that:
while self.line_mode and not self.paused (...) why = self.lineReceived(line) if why or self.transport and self.transport.disconnecting: return why
As this is WITHIN the loop, we see that in fact returning anything true from lineReceived aborts the loop.
Two extra notes:
- it seems there is the same problem with dataReceived, maybe also other functions
- returning value from lineReceived can be useful for instance for unit testing (personally I got this error when I wanted to unit-test some my lineReceived implementation - and had to return deferred to give test procedure opportunity to sync on it)