[MERGE] clean error msg on bzr+ssh timeouts (#115601)
John Arbash Meinel
john at arbash-meinel.com
Wed Oct 24 18:43:51 BST 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Ian Clatworthy wrote:
> When trying to access a branch via bzr+ssh that is inaccessible, the
> code dumps a traceback instead of giving a clean error message. This has
> been reported several times now in various forms - #115601 has 3
> duplicates registered. This change fixes those bugs.
>
> I'm pretty sure the code is fine though I'm less sure about the tests.
> The tests do detect the 2 scenarios - disconnected during startup and
> disconnected later - but perhaps they ought to be done at the different
> level, e.g. up higher? There are a heap of smart servers tests so there
> could well be much better ways to test this. If so, please speak up.
> Even if these tests are acceptable, I'm keen to learn the better way if
> it exists for next time.
>
> Ian C.
>
+ if new_char == '':
+ # end of file encountered reading from server
+ if len(line) == 0:
+ # disconnected before any data read
+ raise errors.ConnectionErrorOnStartUp(
+ "please check connectivity and permissions")
+ else:
+ # disconnected after some data received
+ raise errors.ConnectionReset("connection timeout")
line += new_char
- - assert new_char != '', "end of file reading from server."
return line
^- As Martin commented, 'connection timeout' is a bit suspect, versus just
'disconnected'.
Also, I'm not sure that "len(line) == 0" is the best check. Because if you have
made a couple requests, and then it disconnects, it seems perfectly likely that
you will get nothing read for *this* request.
It also reminds me of how weird it is to spin on reading 1 character at a time.
It does comment that "tuples are short", though I think readv() may send more
data through the args then might be expected.
I wonder if it wouldn't make a lot of sense to length-prefix all requests, with
a fixed width field. (A 32-bit unsigned integer would be great here, but if we
want to keep the requests ascii, we could just do a 8-character number.)
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHH4RWJdeBCYSNAAMRAj11AKCje8Pbd9G0Y8Pt2B2XP3refD7oPgCfWeXX
wTQu6tKzFOEh2pjN5wp7Las=
=U+iT
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list