Patchwork [2,of,2,RFC] ui: add support for fully printing chained exception tracebacks

login
register
mail settings
Submitter Matt Harbison
Date Jan. 7, 2013, 4:40 a.m.
Message ID <a3db4f4e519b4d800584.1357533655@Envy>
Download mbox | patch
Permalink /patch/409/
State Changes Requested
Delegated to: Bryan O'Sullivan
Headers show

Comments

Matt Harbison - Jan. 7, 2013, 4:40 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison at yahoo.com>
# Date 1357527434 18000
# Node ID a3db4f4e519b4d8005845853e22ef026bd5bd702
# Parent  c8d4cc041ba1ad2572d7d139802831e0ac062bfb
ui: add support for fully printing chained exception tracebacks

Many subrepo functions were recently wrapped to enable reporting the path of the
subrepo where the exception occurred, by catching the exception and raising
another with a modified message.  The downside is that any tracebacks will be
printed from where the second exception is raised, not the original.  That
information may be critical to debugging user reported issues.  This prints out
the missing part of the stack where the original exception was raised, along
with the stack for the secondary exception.

Is there a better way to do this?  I thought about either replacing SubrepoAbort
with a general error.ChainedAbort or slipping such a superclass in beneath
SubrepoAbort, but it doesn't seem worth it since subrepos are the only place I
know of attempting chained exceptions.  Therefore a property seemed better.  The
print syntax looks funny, but I read something somewhere that
print(...,file=...) is only supported in 2.6 and later.

In dispatch._runcatch(), there is an exception handler that calls
traceback.print_exc() if --debugger is specified, which should probably also
print the chained exception.  (So these changes should probably be rolled up
into a utility function).

In the next line, pdb.post_mortem(sys.exc_info()[2]) is called, referencing the
secondary exception.  Is there anything that could/should be done here?  I
didn't see any references to fabricating a traceback out of the two existing
ones, thus all of the string joining and concatenating.

What's the best way to deal with the test?  I globbed the stuff that would
trigger a failure everytime, but it will get flagged anytime the call sequence
is changed to add or remove a method along the way.

Anything else I missed?
Bryan O'Sullivan - Jan. 7, 2013, 7:15 p.m.
On Sun, Jan 06, 2013 at 11:40:55PM -0500, Matt Harbison wrote:
> 
> Is there a better way to do this?  I thought about either replacing SubrepoAbort
> with a general error.ChainedAbort or slipping such a superclass in beneath
> SubrepoAbort, but it doesn't seem worth it since subrepos are the only place I
> know of attempting chained exceptions.

That seems reasonable enough for now.

> The print syntax looks funny, but I read something somewhere that
> print(...,file=...) is only supported in 2.6 and later.

Maybe use ui.write_err instead?

Patch

diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
--- a/mercurial/subrepo.py
+++ b/mercurial/subrepo.py
@@ -5,7 +5,7 @@ 
 # This software may be used and distributed according to the terms of the
 # GNU General Public License version 2 or any later version.
 
-import errno, os, re, xml.dom.minidom, shutil, posixpath
+import errno, os, re, xml.dom.minidom, shutil, posixpath, sys
 import stat, subprocess, tarfile
 from i18n import _
 import config, scmutil, util, node, error, cmdutil, bookmarks, match as matchmod
@@ -19,6 +19,7 @@ 
     def __init__(self, *args, **kw):
         super(SubrepoAbort, self).__init__(*args, **kw)
         self.subrepo = kw.get('subrepo')
+        self.cause = kw.get('cause')
 
 def annotatesubrepoerror(func):
     def decoratedmethod(self, *args, **kargs):
@@ -31,7 +32,8 @@ 
             subrepo = subrelpath(self)
             errormsg = _('%s (in subrepo %s)') % (str(ex), subrepo)
             # avoid handling this exception by raising a SubrepoAbort exception
-            raise SubrepoAbort(errormsg, hint=ex.hint, subrepo=subrepo)
+            raise SubrepoAbort(errormsg, hint=ex.hint, subrepo=subrepo,
+                               cause=sys.exc_info())
         return res
     return decoratedmethod
 
diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -686,11 +686,21 @@ 
         only to call in exception handler. returns true if traceback
         printed.'''
         if self.tracebackflag:
-            if exc:
+            if exc is None:
+                exc = sys.exc_info()
+            cause=getattr(exc[1], 'cause', None)
+
+            if cause is not None:
+                tbc=traceback.format_tb(cause[2])
+                tbe=traceback.format_tb(exc[2])
+                feo=traceback.format_exception_only(cause[0], cause[1])
+                # exclude the frame where 'exc' was thrown
+                print >> self.ferr, 'Traceback (most recent call last):\n' \
+                                  + ''.join(tbe[:-1]) \
+                                  + ''.join(tbc) + ''.join(feo),
+            else:
                 traceback.print_exception(exc[0], exc[1], exc[2],
                                           file=self.ferr)
-            else:
-                traceback.print_exc(file=self.ferr)
         return self.tracebackflag
 
     def geteditor(self):
diff --git a/tests/test-subrepo.t b/tests/test-subrepo.t
--- a/tests/test-subrepo.t
+++ b/tests/test-subrepo.t
@@ -673,8 +673,14 @@ 
     File "*/mercurial/subrepo.py", line *, in submerge (glob)
       mctx.sub(s).get(r)
     File "*/mercurial/subrepo.py", line *, in decoratedmethod (glob)
-      raise SubrepoAbort(errormsg, hint=ex.hint, subrepo=subrepo)
-  SubrepoAbort: default path for subrepository not found (in subrepo sub/repo) (glob)
+      res = func(self, *args, **kargs)
+    File "*/mercurial/subrepo.py", line *, in get (glob)
+      self._get(state)
+    File "*/mercurial/subrepo.py", line *, in _get (glob)
+      srcurl = _abssource(self._repo)
+    File "*/mercurial/subrepo.py", line *, in _abssource (glob)
+      raise util.Abort(_("default path for subrepository not found"))
+  Abort: default path for subrepository not found
   abort: default path for subrepository not found (in subrepo sub/repo) (glob)
   [255]