[PATCH 1 of 4] lfs: improve the client message when the server signals an object error

[Matt Harbison]

# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1519620242 18000
#      Sun Feb 25 23:44:02 2018 -0500
# Node ID 0cd7c41df75ac914d95f398105317afb53f7e5db
# Parent  97ab6f2dc3c3b0d689e90b4872b3874e649ce4e6
lfs: improve the client message when the server signals an object error

Two things here.  First, the previous message included a snippet of JSON, which
tends to be long (and in the case of lfs-test-server, has no error message).
Instead, give a concise message where possible, and leave the JSON to a debug
output.  Second, the server can signal issues other than a missing individual
file.  This change shows a corrupt file, but I'm debating letting the corrupt
file get downloaded, because 1) the error code doesn't really fit, and 2) having
it locally makes forensics easier.  Maybe need a config knob for that.

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -263,23 +263,34 @@ class _gitlfsremote(object):
             # server implementation (ex. lfs-test-server)  does not set "error"
             # but just removes "download" from "actions". Treat that case
             # as the same as 404 error.
-            notfound = (response.get('error', {}).get('code') == 404
-                        or (action == 'download'
-                            and action not in response.get('actions', [])))
-            if notfound:
-                ptrmap = {p.oid(): p for p in pointers}
-                p = ptrmap.get(response['oid'], None)
-                if p:
-                    filename = getattr(p, 'filename', 'unknown')
-                    raise LfsRemoteError(
-                        _(('LFS server error. Remote object '
-                          'for "%s" not found: %r')) % (filename, response))
+            if 'error' not in response:
+                if (action == 'download'
+                    and action not in response.get('actions', [])):
+                    code = 404
                 else:
-                    raise LfsRemoteError(
-                        _('LFS server error. Unsolicited response for oid %s')
-                        % response['oid'])
-            if 'error' in response:
-                raise LfsRemoteError(_('LFS server error: %r') % response)
+                    continue
+            else:
+                # An error dict without a code doesn't make much sense, so
+                # treat as a server error.
+                code = response.get('error').get('code', 500)
+
+            ptrmap = {p.oid(): p for p in pointers}
+            p = ptrmap.get(response['oid'], None)
+            if p:
+                filename = getattr(p, 'filename', 'unknown')
+                errors = {
+                    404: 'The object does not exist',
+                    410: 'The object was removed by the owner',
+                    422: 'Validation error',
+                    500: 'Internal server error',
+                }
+                msg = errors.get(code, 'status code %d' % code)
+                raise LfsRemoteError(_(('LFS server error for "%s": %s'))
+                                     % (filename, msg))
+            else:
+                raise LfsRemoteError(
+                    _('LFS server error. Unsolicited response for oid %s')
+                    % response['oid'])
 
     def _extractobjects(self, response, pointers, action):
         """extract objects from response of the batch API
diff --git a/tests/test-lfs-test-server.t b/tests/test-lfs-test-server.t
--- a/tests/test-lfs-test-server.t
+++ b/tests/test-lfs-test-server.t
@@ -449,7 +449,7 @@ TODO: give the proper error indication f
   Content-Type: text/plain; charset=utf-8 (git-server !)
   Date: $HTTP_DATE$ (git-server !)
   abort: corrupt remote lfs object: d11e1a642b60813aee592094109b406089b8dff4cb157157f753418ec7857998 (git-server !)
-  abort: LFS server error. Remote object for "c" not found: *! (glob) (hg-server !)
+  abort: LFS server error for "c": Validation error! (hg-server !)
   [255]
 
 The corrupted blob is not added to the usercache or local store
@@ -807,7 +807,7 @@ Check error message when the remote miss
     ]
     "transfer": "basic" (hg-server !)
   }
-  abort: LFS server error. Remote object for "b" not found:(.*)! (re)
+  abort: LFS server error for "b": The object does not exist!
   [255]
 
 Check error message when object does not exist:
@@ -918,7 +918,7 @@ Check error message when object does not
     ]
     "transfer": "basic" (hg-server !)
   }
-  abort: LFS server error. Remote object for "a" not found:(.*)! (re)
+  abort: LFS server error for "a": The object does not exist!
   [255]
 
   $ $PYTHON $RUNTESTDIR/killdaemons.py $DAEMON_PIDS