Improve error messages for SSH protocol handshake

If you have an artifact remote cache in your config using the ssh://
protocol and some SSH issue prevents us from connecting, we now show
the user the stderr output from the `ssh` program, e.g.

    [--:--:--] WARNING Failed to fetch remote refs from ssh://artifacts@172.17.0.2:22200/artifacts/: BuildStream did not connect successfully to the shared cache ssh://artifacts@172.17.0.2:22200/artifacts/: Permission denied (publickey,keyboard-interactive).

Previously we would just show the error message raised by the protocol
code, which was less useful, e.g.:

    [--:--:--] WARNING Failed to fetch remote refs from ssh://artifacts@172.17.0.2:22200/artifacts/: BuildStream did not connect successfully to the shared cache ssh://artifacts@172.17.0.2:22200/artifacts/: Expected reply, got none

1 files changed, 21 insertions, 15 deletions

diff --git a/buildstream/_artifactcache/pushreceive.py b/buildstream/_artifactcache/pushreceive.py
index 4d85627a7..e96cba59f 100644
--- a/buildstream/_artifactcache/pushreceive.py
+++ b/buildstream/_artifactcache/pushreceive.py
@@ -656,22 +656,28 @@ def initialize_push_connection(remote):
     writer = PushMessageWriter(ssh.stdin)
     reader = PushMessageReader(ssh.stdout)
 
-    writer.send_hello()
-    args = reader.receive_info()
+    try:
+        writer.send_hello()
+        args = reader.receive_info()
+        writer.send_done()
 
-    if 'pull_url' in args:
-        pull_url = args['pull_url']
-    else:
-        raise PushException(
-            "Remote cache did not tell us its pull URL. This cache probably "
-            "requires updating to a newer version of `bst-artifact-receive`.")
-
-    writer.send_done()
-
-    ssh.wait()
-    if ssh.returncode != 0:
-        error = ssh.stderr.read().decode('unicode-escape')
-        raise PushException(error)
+        if 'pull_url' in args:
+            pull_url = args['pull_url']
+        else:
+            raise PushException(
+                "Remote cache did not tell us its pull URL. This cache probably "
+                "requires updating to a newer version of `bst-artifact-receive`.")
+    except PushException as protocol_error:
+        # If we get a read error on the wire, let's first see if SSH reported
+        # an error such as 'Permission denied'. If so this will be much more
+        # useful to the user than the "Expected reply, got none" sort of
+        # message that reader.receive_info() will have raised.
+        ssh.wait()
+        if ssh.returncode != 0:
+            ssh_error = ssh.stderr.read().decode('unicode-escape')
+            raise PushException("SSH error: {}".format(ssh_error))
+        else:
+            raise protocol_error
 
     return pull_url