[MERGE] Cleanup the ConnectionError for ssh failures

Richard Wilbur richard.wilbur at gmail.com
Tue Oct 3 14:24:29 BST 2006


On Fri, 2006-09-29 at 16:30 -0500, John Arbash Meinel wrote:
> I'm trying to close this bug:
> https://launchpad.net/products/bzr/+bug/49172
> 
> We've already fixed the basics, but right now we are outputing an error
> like:
> 
> Unable to connect to ssh host badhost:None: Unhandled EOFError
> 
> The :None: really looks bad. Also, there are some code paths that leak
> differently.
> 
> This patch adds a few things:
> 
> 1) SocketConnectionError
>    This error class has some fancier handling of error messages. It does
> the right thing when handling port, or if there is extra information. It
> also allows supplying a custom message, so that rather than just getting
> "Failure to connect" the raising function can inform what kind of
> connection was being attempted. This was already possible with
> ConnectionError, I just made it a little bit easier to raise a nice
> error for protocols that use host and ports.
> 
> 2) Change SSHVendor to have a _raise_connection_error, which handles the
> details of passing the error parameters around, and defaults msg to a
> better message for SSH errors.
> 
> 3) Update SSHVendor children to use this helper.
> 
> This should make SSH connection errors nicer all around.
> 
> Compare:
> % ./bzr log sftp://notahost/
> ssh: notahost: Name or service not known
> bzr: ERROR: Unable to connect to SSH host notahost; EOF during negotiation
> 
> % bzr log sftp://notahost/
> ssh: notahost: Name or service not known
> bzr: ERROR: Connection error: Unable to connect to SSH host
> notahost:None: EOF during negotiation
> 
> Now, we have a different bug with 'bzr+ssh://' because we don't actually
> detect a failed connection unless we are using paramiko. Because
> _connect() doesn't fail, only when we start trying to read/write to the
> subprocess do we get a failure. And at that time we just translate it
> into a bogus bzr server failure:
> 
> % bzr log bzr+ssh://notahost/
> ssh: notahost: Name or service not known
> bzr: ERROR: Generic bzr smart protocol error: unexpected smart server
> error: None
> 
> Versus
> % PATH='' /usr/bin/python2.4 bzr log bzr+ssh://notahost/
> bzr: ERROR: Unable to connect to SSH host notahost; (-2, 'Name or
> service not known')
> 
> 
> Anyway, the attached patch fixes up a few places that need to be cleaned
> up, and I'd like to get it merged.
> 
> John
> =:->

John,

Again, I like the more descriptive error messages as they greatly
facilitate debugging.  In this case you also got rid of a red herring in
the 'host:None' department which seemed to imply a missing parameter.

Bravissimo signore:  +1

Richard Wilbur





More information about the bazaar mailing list