[MERGE] Fix infinite busy-loop caused by connection loss during read of response body in HPSS v1 and v2.

Andrew Bennetts andrew at canonical.com
Wed Jun 11 12:42:26 BST 2008


Martin Pool wrote:
> Martin Pool has voted tweak.
> Status is now: Conditionally approved
> Comment:
> +            if bytes == '':
> +                # end of file encountered reading from server
> +                raise errors.ConnectionReset(
> +                    "Connection lost while reading response body.")
>              _body_decoder.accept_bytes(bytes)
>
> To be a bit pedantic, this is not really "connection reset" (ie  
> ECONNRESET), but rather "unexpected end of stream" or "connection closed  
> unexpectedly".  We may never want to handle them differently  
> programmatically but it's good to at least be precise to the user to  
> help debugging.

The error string the user will see in this case is “Connection closed:
Connection lost while reading response body. ”

The exception class itself probably could have a better name, though.  It's used
for both failure to establish a connection, and interrupted connections.  Do you
think we should split those?  I think that probably makes sense.  Twisted makes
a distinction between “connection lost” and “connection failed”, which seems
right to me.  If you agree I'll followup with a change to replace
ConnectionReset with ConnectionLost and ConnectionFailed (maybe keeping
ConnectionReset as a common base class?).

> Did you check if this could happen in the server code too?

Yes, I audited the server code.  It doesn't have this problem, thankfully.

Thanks for the review, I'll merge this now.  Let me know what you think about
renaming/splitting the ConnectionReset exception class and I'll make a patch for
that separately.

-Andrew.




More information about the bazaar mailing list