Patchwork D8209: phabricator: avoid a stacktrace when command arguments are missing

login
register
mail settings
Submitter phabricator
Date March 4, 2020, 5:59 a.m.
Message ID <differential-rev-PHID-DREV-mvcbu4s3vwjbc4vt5k43-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45444/
State Superseded
Headers show

Comments

phabricator - March 4, 2020, 5:59 a.m.
mharbison72 created this revision.
Herald added subscribers: mercurial-devel, Kwan.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  Previously, the TypeError wasn't properly converted to a SignatureError when
  improper arguments were supplied to the inner function, because the stack depth
  is 2 inside the vcrcommand decorator.  The `__name__` and `__doc__` attributes
  need to be reassigned to the new wrapper so that the help summary is available.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8209

AFFECTED FILES
  hgext/phabricator.py
  mercurial/util.py
  tests/test-phabricator.t

CHANGE DETAILS




To: mharbison72, #hg-reviewers
Cc: Kwan, mercurial-devel

Patch

diff --git a/tests/test-phabricator.t b/tests/test-phabricator.t
--- a/tests/test-phabricator.t
+++ b/tests/test-phabricator.t
@@ -29,6 +29,21 @@ 
   >  --test-vcr "$VCR/phabread-conduit-error.json" D4480 | head
   abort: Conduit Error (ERR-INVALID-AUTH): API token "cli-notavalidtoken" has the wrong length. API tokens should be 32 characters long.
 
+Missing arguments print the command help
+
+  $ hg phabread
+  hg phabread: invalid arguments
+  hg phabread DREVSPEC [OPTIONS]
+  
+  print patches from Phabricator suitable for importing
+  
+  options:
+  
+    --stack read dependencies
+  
+  (use 'hg phabread -h' to show more help)
+  [255]
+
 Basic phabread:
   $ hg phabread --test-vcr "$VCR/phabread-4480.json" D4480 | head
   # HG changeset patch
diff --git a/mercurial/util.py b/mercurial/util.py
--- a/mercurial/util.py
+++ b/mercurial/util.py
@@ -1846,14 +1846,14 @@ 
     return pycompat.ossep.join(([b'..'] * len(a)) + b) or b'.'
 
 
-def checksignature(func):
+def checksignature(func, depth=1):
     '''wrap a function with code to check for calling errors'''
 
     def check(*args, **kwargs):
         try:
             return func(*args, **kwargs)
         except TypeError:
-            if len(traceback.extract_tb(sys.exc_info()[2])) == 1:
+            if len(traceback.extract_tb(sys.exc_info()[2])) == depth:
                 raise error.SignatureError
             raise
 
diff --git a/hgext/phabricator.py b/hgext/phabricator.py
--- a/hgext/phabricator.py
+++ b/hgext/phabricator.py
@@ -257,15 +257,17 @@ 
                         return fn(*args, **kwargs)
             return fn(*args, **kwargs)
 
-        inner.__name__ = fn.__name__
-        inner.__doc__ = fn.__doc__
+        cmd = util.checksignature(inner, depth=2)
+        cmd.__name__ = fn.__name__
+        cmd.__doc__ = fn.__doc__
+
         return command(
             name,
             fullflags,
             spec,
             helpcategory=helpcategory,
             optionalrepo=optionalrepo,
-        )(inner)
+        )(cmd)
 
     return decorate