# HG changeset patch # User Matt Harbison # Date 1519620242 18000 # Node ID 67db84842356c545fe2571c27ca69369df04a4f4 # Parent 79af9ae46a78786205fe6a73cb617486efcf3c86 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 -r 79af9ae46a78 -r 67db84842356 hgext/lfs/blobstore.py --- a/hgext/lfs/blobstore.py Sat Mar 31 15:54:26 2018 -0400 +++ b/hgext/lfs/blobstore.py Sun Feb 25 23:44:02 2018 -0500 @@ -263,23 +263,34 @@ # 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 -r 79af9ae46a78 -r 67db84842356 tests/test-lfs-test-server.t --- a/tests/test-lfs-test-server.t Sat Mar 31 15:54:26 2018 -0400 +++ b/tests/test-lfs-test-server.t Sun Feb 25 23:44:02 2018 -0500 @@ -449,7 +449,7 @@ 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 @@ ] "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 @@ ] "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