[MERGE] clean error msg on bzr+ssh timeouts (#115601)

[John Arbash Meinel]

-----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-----