Patchwork [4,of,5] lfs: handle URLErrors to add additional information

login
register
mail settings
Submitter Matt Harbison
Date Nov. 16, 2018, 2:53 a.m.
Message ID <965734e0066e1956a13a.1542336833@Envy>
Download mbox | patch
Permalink /patch/36621/
State Accepted
Headers show

Comments

Matt Harbison - Nov. 16, 2018, 2:53 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1542323309 18000
#      Thu Nov 15 18:08:29 2018 -0500
# Node ID 965734e0066e1956a13a28c0ab758b7abc330dc8
# Parent  8004aa5e1952fad796b927f829b618927512e4a7
lfs: handle URLErrors to add additional information

Sometimes the blob server is hit first (e.g. on push), and sometimes it's hit
last (e.g. pull).  Throw in depth first subrepo operations, and things quickly
get insane.  It wasn't even mentioning LFS, so just saying "connection refused"
can be confusing- especially if the blob server is a secondary server and
connecting to the repo server works.

The exception handler for the transfer handler will print the full path to the
blob, but that seems fine given that it might be necessary to debug a second
server.  (We don't yet support a standalone blob server, so the handler for the
Batch API will cover 99.9% of the current problems.  But it might as well be
handled now while I'm thinking about it.)

The function for translating to a message was mostly borrowed from
scmutil.catchall().
Yuya Nishihara - Nov. 17, 2018, 4:34 a.m.
On Thu, 15 Nov 2018 21:53:53 -0500, Matt Harbison wrote:
> +  $ hg -R httpclone update default --config lfs.url=http://localhost:$HGPORT2/missing
> +  abort: LFS error: No connection could be made because the target machine actively refused it!

Globbed out this message. On my Linux box, it just says "Connection refused".

Patch

diff --git a/hgext/lfs/blobstore.py b/hgext/lfs/blobstore.py
--- a/hgext/lfs/blobstore.py
+++ b/hgext/lfs/blobstore.py
@@ -17,6 +17,7 @@  import socket
 from mercurial.i18n import _
 
 from mercurial import (
+    encoding,
     error,
     pathutil,
     pycompat,
@@ -26,6 +27,10 @@  from mercurial import (
     worker,
 )
 
+from mercurial.utils import (
+    stringutil,
+)
+
 from ..largefiles import lfutil
 
 # 64 bytes for SHA256
@@ -231,6 +236,30 @@  class local(object):
         False otherwise."""
         return self.cachevfs.exists(oid) or self.vfs.exists(oid)
 
+def _urlerrorreason(urlerror):
+    '''Create a friendly message for the given URLError to be used in an
+    LfsRemoteError message.
+    '''
+    inst = urlerror
+
+    if isinstance(urlerror.reason, Exception):
+        inst = urlerror.reason
+
+    if util.safehasattr(inst, 'reason'):
+        try: # usually it is in the form (errno, strerror)
+            reason = inst.reason.args[1]
+        except (AttributeError, IndexError):
+            # it might be anything, for example a string
+            reason = inst.reason
+        if isinstance(reason, pycompat.unicode):
+            # SSLError of Python 2.7.9 contains a unicode
+            reason = encoding.unitolocal(reason)
+        return reason
+    elif getattr(inst, "strerror", None):
+        return encoding.strtolocal(inst.strerror)
+    else:
+        return stringutil.forcebytestr(urlerror)
+
 class _gitlfsremote(object):
 
     def __init__(self, repo, url):
@@ -279,6 +308,11 @@  class _gitlfsremote(object):
             }
             hint = hints.get(ex.code, _('api=%s, action=%s') % (url, action))
             raise LfsRemoteError(_('LFS HTTP error: %s') % ex, hint=hint)
+        except util.urlerr.urlerror as ex:
+            hint = (_('the "lfs.url" config may be used to override %s')
+                      % self.baseurl)
+            raise LfsRemoteError(_('LFS error: %s') % _urlerrorreason(ex),
+                                 hint=hint)
         try:
             response = json.loads(rawjson)
         except ValueError:
@@ -409,6 +443,11 @@  class _gitlfsremote(object):
                 self.ui.debug('%s: %s\n' % (oid, ex.read()))
             raise LfsRemoteError(_('HTTP error: %s (oid=%s, action=%s)')
                                  % (ex, oid, action))
+        except util.urlerr.urlerror as ex:
+            hint = (_('attempted connection to %s')
+                      % util.urllibcompat.getfullurl(request))
+            raise LfsRemoteError(_('LFS error: %s') % _urlerrorreason(ex),
+                                 hint=hint)
 
     def _batch(self, pointers, localstore, action):
         if action not in ['upload', 'download']:
diff --git a/tests/test-lfs-serve-access.t b/tests/test-lfs-serve-access.t
--- a/tests/test-lfs-serve-access.t
+++ b/tests/test-lfs-serve-access.t
@@ -81,6 +81,11 @@  Reasonable hint for a misconfigured blob
   (the "lfs.url" config may be used to override http://localhost:$HGPORT/missing)
   [255]
 
+  $ hg -R httpclone update default --config lfs.url=http://localhost:$HGPORT2/missing
+  abort: LFS error: No connection could be made because the target machine actively refused it!
+  (the "lfs.url" config may be used to override http://localhost:$HGPORT2/missing)
+  [255]
+
 Blob URIs are correct when --prefix is used
 
   $ hg clone --debug http://localhost:$HGPORT/subdir/mount/point cloned2