Record and print an internal stack trace when Taskmaster detects errors for easier debugging. (Gary Oberbrunner) Better debugging of Python exceptions (like TypeErrors) in SConscript files.

7 files changed, 124 insertions, 32 deletions

diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 57b6fa7..6368d78 100644
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -52,6 +52,8 @@ RELEASE 0.XX - XXX
 
   - Allow the source of Command() to be a directory.
 
+  - Better error handling of things like raw TypeErrors in SConscripts.
+
   From Gary Oberbrunner:
 
   - Report the target being built in error messages when building
@@ -65,6 +67,8 @@ RELEASE 0.XX - XXX
   - Fix the value returned by the Node.prevsiginfo() method to conform
     to a previous change when checking whether a node is current.
 
+  - Supply a stack trace if the Taskmaster catches an exception.
+
   From Laurent Pelecq:
 
   - When the -debug=pdb option is specified, use pdb.Pdb().runcall() to
diff --git a/src/engine/SCons/Errors.py b/src/engine/SCons/Errors.py
index cc8bb3d..0a57614 100644
--- a/src/engine/SCons/Errors.py
+++ b/src/engine/SCons/Errors.py
@@ -61,3 +61,10 @@ class ConfigureDryRunError(UserError):
     but the user requested a dry-run"""
     def __init__(self,file):
         UserError.__init__(self,"Cannot update configure test (%s) within a dry-run." % str(file))
+
+class TaskmasterException(Exception):
+    def __init__(self, type, value, traceback, *args):
+        self.type = type
+        self.value = value
+        self.traceback = traceback
+        self.args = args
diff --git a/src/engine/SCons/ErrorsTests.py b/src/engine/SCons/ErrorsTests.py
index 810f840..4771a3f 100644
--- a/src/engine/SCons/ErrorsTests.py
+++ b/src/engine/SCons/ErrorsTests.py
@@ -65,6 +65,15 @@ class ErrorsTestCase(unittest.TestCase):
         except SCons.Errors.UserError, e:
             assert e.args == "Cannot update configure test (FileName) within a dry-run."
 
+    def test_TaskmasterException(self):
+        """Test the TaskmasterException."""
+        try:
+            raise SCons.Errors.TaskmasterException("one", "two", "three")
+        except SCons.Errors.TaskmasterException, e:
+            assert e.type == "one", e.type
+            assert e.value == "two", e.value
+            assert e.traceback == "three", e.traceback
+
 
 if __name__ == "__main__":
     suite = unittest.makeSuite(ErrorsTestCase, 'test_')
diff --git a/src/engine/SCons/Script/SConscript.py b/src/engine/SCons/Script/SConscript.py
index 313a3f2..2984160 100644
--- a/src/engine/SCons/Script/SConscript.py
+++ b/src/engine/SCons/Script/SConscript.py
@@ -309,6 +309,30 @@ def SConscript(*ls, **kw):
     else:
         return tuple(results)
 
+def is_our_exec_statement(line):
+    return not line is None and line[:12] == "exec _file_ "
+
+def SConscript_exception(file=sys.stderr):
+    """Print an exception stack trace just for the SConscript file(s).
+    This will show users who have Python errors where the problem is,
+    without cluttering the output with all of the internal calls leading
+    up to where we exec the SConscript."""
+    stack = traceback.extract_tb(sys.exc_traceback)
+    last_text = ""
+    i = 0
+    for frame in stack:
+        if is_our_exec_statement(last_text):
+            break
+        i = i + 1
+        last_text = frame[3]
+    type = str(sys.exc_type)
+    if type[:11] == "exceptions.":
+        type = type[11:]
+    file.write('%s: %s:\n' % (type, sys.exc_value))
+    for fname, line, func, text in stack[i:]:
+        file.write('  File "%s", line %d:\n' % (fname, line))
+        file.write('    %s\n' % text)
+
 def annotate(node):
     """Annotate a node with the stack frame describing the
     SConscript file and line number that created it."""
@@ -319,7 +343,7 @@ def annotate(node):
         # magic "exec _file_ " string, then this frame describes the
         # SConscript file and line number that caused this node to be
         # created.  Record the tuple and carry on.
-        if not last_text is None and last_text[:12] == "exec _file_ ":
+        if is_our_exec_statement(last_text):
             node.creator = frame
             return
         last_text = frame[3]
diff --git a/src/engine/SCons/Script/__init__.py b/src/engine/SCons/Script/__init__.py
index c171a91..a413054 100644
--- a/src/engine/SCons/Script/__init__.py
+++ b/src/engine/SCons/Script/__init__.py
@@ -133,28 +133,41 @@ class BuildTask(SCons.Taskmaster.Task):
                 print tree
 
     def failed(self):
-        e = sys.exc_value
+        # Handle the failure of a build task.  The primary purpose here
+        # is to display the various types of Errors and Exceptions
+        # appropriately.
         status = 2
-        if sys.exc_type == SCons.Errors.BuildError:
+        e = sys.exc_value
+        t = sys.exc_type
+        tb = None
+        if t is SCons.Errors.TaskmasterException:
+            # The Taskmaster received an Error or Exception while trying
+            # to process or build the Nodes and dependencies, which it
+            # wrapped up for us in the object recorded as the value of
+            # the Exception, so process the wrapped info instead of the
+            # TaskmasterException itself.
+            t = e.type
+            tb = e.traceback
+            e = e.value
+
+        if t == SCons.Errors.BuildError:
             sys.stderr.write("scons: *** [%s] %s\n" % (e.node, e.errstr))
             if e.errstr == 'Exception':
                 traceback.print_exception(e.args[0], e.args[1], e.args[2])
-        elif sys.exc_type == SCons.Errors.UserError:
-            # We aren't being called out of a user frame, so
-            # don't try to walk the stack, just print the error.
-            sys.stderr.write("\nscons: *** %s\n" % e)
-        elif sys.exc_type == SCons.Errors.StopError:
-            s = str(e)
-            if not keep_going_on_error:
-                s = s + '  Stop.'
-            sys.stderr.write("scons: *** %s\n" % s)
-        elif sys.exc_type == SCons.Errors.ExplicitExit:
+        elif t == SCons.Errors.ExplicitExit:
             status = e.status
             sys.stderr.write("scons: *** [%s] Explicit exit, status %s\n" % (e.node, e.status))
         else:
             if e is None:
-                e = sys.exc_type
-            sys.stderr.write("scons: *** %s\n" % e)
+                e = t
+            s = str(e)
+            if t == SCons.Errors.StopError and not keep_going_on_error:
+                s = s + '  Stop.'
+            sys.stderr.write("scons: *** %s\n" % s)
+
+        if tb:
+            sys.stderr.write("scons: internal stack trace:\n")
+            traceback.print_tb(tb, file=sys.stderr)
 
         self.do_failed(status)
 
@@ -290,11 +303,11 @@ def _scons_internal_warning(e):
     sys.stderr.write("\nscons: warning: %s\n" % e)
     sys.stderr.write('File "%s", line %d, in %s\n' % (filename, lineno, routine))
 
-def _scons_other_errors():
+def _scons_internal_error():
     """Handle all errors but user errors. Print out a message telling
     the user what to do in this case and print a normal trace.
     """
-    print 'other errors'
+    print 'internal error'
     traceback.print_exc()
     sys.exit(2)
 
@@ -984,12 +997,18 @@ def main():
         sys.exit(2)
     except SyntaxError, e:
         _scons_syntax_error(e)
+    except SCons.Errors.InternalError:
+        _scons_internal_error()
     except SCons.Errors.UserError, e:
         _scons_user_error(e)
     except SCons.Errors.ConfigureDryRunError, e:
         _scons_configure_dryrun_error(e)
     except:
-        _scons_other_errors()
+        # An exception here is likely a builtin Python exception Python
+        # code in an SConscript file.  Show them precisely what the
+        # problem was and where it happened.
+        SCons.Script.SConscript.SConscript_exception()
+        sys.exit(2)
 
     if print_time:
         total_time = time.time()-start_time
diff --git a/src/engine/SCons/Taskmaster.py b/src/engine/SCons/Taskmaster.py
index 2fbc6e2..7937569 100644
--- a/src/engine/SCons/Taskmaster.py
+++ b/src/engine/SCons/Taskmaster.py
@@ -31,6 +31,7 @@ __revision__ = "__FILE__ __REVISION__ __DATE__ __DEVELOPER__"
 
 import string
 import sys
+import traceback
 
 import SCons.Node
 import SCons.Errors
@@ -238,7 +239,10 @@ class Taskmaster:
                 # children (like a child couldn't be linked in to a
                 # BuildDir, or a Scanner threw something).  Arrange to
                 # raise the exception when the Task is "executed."
-                self.exception_set(sys.exc_type, sys.exc_value)
+                x = SCons.Errors.TaskmasterException(sys.exc_type,
+                                                     sys.exc_value,
+                                                     sys.exc_traceback)
+                self.exception_set(x)
                 self.candidates.pop()
                 self.ready = node
                 break
@@ -262,7 +266,10 @@ class Taskmaster:
                 # the kids are derived (like a child couldn't be linked
                 # from a repository).  Arrange to raise the exception
                 # when the Task is "executed."
-                self.exception_set(sys.exc_type, sys.exc_value)
+                x = SCons.Errors.TaskmasterException(sys.exc_type,
+                                                     sys.exc_value,
+                                                     sys.exc_traceback)
+                self.exception_set(x)
                 self.candidates.pop()
                 self.ready = node
                 break
@@ -338,7 +345,10 @@ class Taskmaster:
             # a child couldn't be linked in to a BuildDir when deciding
             # whether this node is current).  Arrange to raise the
             # exception when the Task is "executed."
-            self.exception_set(sys.exc_type, sys.exc_value)
+            x = SCons.Errors.TaskmasterException(sys.exc_type,
+                                                 sys.exc_value,
+                                                 sys.exc_traceback)
+            self.exception_set(x)
         self.ready = None
 
         return task
@@ -373,18 +383,23 @@ class Taskmaster:
         self.candidates.extend(self.pending)
         self.pending = []
 
-    def exception_set(self, type, value):
+    def exception_set(self, type, value=None):
         """Record an exception type and value to raise later, at an
         appropriate time."""
         self.exc_type = type
         self.exc_value = value
+        self.exc_traceback = traceback
 
     def exception_raise(self):
         """Raise any pending exception that was recorded while
         getting a Task ready for execution."""
         if self.exc_type:
             try:
-                raise self.exc_type, self.exc_value
+                try:
+                    raise self.exc_type, self.exc_value
+                except TypeError:
+                    # exc_type was probably an instance,
+                    # so raise it by itself.
+                    raise self.exc_type
             finally:
                 self.exception_set(None, None)
-
diff --git a/src/engine/SCons/TaskmasterTests.py b/src/engine/SCons/TaskmasterTests.py
index 3616370..4dbf8b3 100644
--- a/src/engine/SCons/TaskmasterTests.py
+++ b/src/engine/SCons/TaskmasterTests.py
@@ -404,8 +404,11 @@ class TaskmasterTestCase(unittest.TestCase):
         n1 = Node("n1")
         tm = SCons.Taskmaster.Taskmaster(targets = [n1], tasker = MyTask)
         t = tm.next_task()
-        assert tm.exc_type == MyException, tm.exc_type
-        assert str(tm.exc_value) == "from make_ready()", tm.exc_value
+        assert isinstance(tm.exc_type, SCons.Errors.TaskmasterException), repr(tm.exc_type)
+        assert tm.exc_value is None, tm.exc_value
+        e = tm.exc_type
+        assert e.type == MyException, e.type
+        assert str(e.value) == "from make_ready()", str(e.value)
 
 
     def test_children_errors(self):
@@ -417,11 +420,16 @@ class TaskmasterTestCase(unittest.TestCase):
         class ExitNode(Node):
             def children(self):
                 sys.exit(77)
+
         n1 = StopNode("n1")
         tm = SCons.Taskmaster.Taskmaster([n1])
         t = tm.next_task()
-        assert tm.exc_type == SCons.Errors.StopError, "Did not record StopError on node"
-        assert str(tm.exc_value) == "stop!", "Unexpected exc_value `%s'" % tm.exc_value
+        assert isinstance(tm.exc_type, SCons.Errors.TaskmasterException), repr(tm.exc_type)
+        assert tm.exc_value is None, tm.exc_value
+        e = tm.exc_type
+        assert e.type == SCons.Errors.StopError, e.type
+        assert str(e.value) == "stop!", "Unexpected exc_value `%s'" % e.value
+
         n2 = ExitNode("n2")
         tm = SCons.Taskmaster.Taskmaster([n2])
         t = tm.next_task()
@@ -576,10 +584,12 @@ class TaskmasterTestCase(unittest.TestCase):
         exc_caught = None
         try:
             t.prepare()
-        except MyException, v:
-            assert str(v) == "exception value", v
+        except MyException, e:
             exc_caught = 1
+        except:
+            pass
         assert exc_caught, "did not catch expected MyException"
+        assert str(e) == "exception value", e
         assert built_text is None, built_text
 
     def test_execute(self):
@@ -652,6 +662,10 @@ class TaskmasterTestCase(unittest.TestCase):
         assert tm.exc_type == 1, tm.exc_type
         assert tm.exc_value == 2, tm.exc_value
 
+        tm.exception_set(3)
+        assert tm.exc_type == 3, tm.exc_type
+        assert tm.exc_value is None, tm.exc_value
+
         tm.exception_set(None, None)
         assert tm.exc_type is None, tm.exc_type
         assert tm.exc_value is None, tm.exc_value
@@ -661,7 +675,7 @@ class TaskmasterTestCase(unittest.TestCase):
             tm.exception_raise()
         except:
             assert sys.exc_type == "exception 1", sys.exc_type
-            assert sys.exc_value is None, sys.exc_type
+            assert sys.exc_value is None, sys.exc_value
         else:
             assert 0, "did not catch expected exception"
 
@@ -670,7 +684,7 @@ class TaskmasterTestCase(unittest.TestCase):
             tm.exception_raise()
         except:
             assert sys.exc_type == "exception 2", sys.exc_type
-            assert sys.exc_value == "xyzzy", sys.exc_type
+            assert sys.exc_value == "xyzzy", sys.exc_value
         else:
             assert 0, "did not catch expected exception"