Merge pull request #347 from nmalaguti/feature/monkeypatch-traceback

Monkey patch traceback.TracebackException to support MultiError

docs/source/reference-core.rst

@@ -982,8 +982,6 @@ Nursery objects provide the following interface:
 Working with :exc:`MultiError`\s
 ++++++++++++++++++++++++++++++++
 
-.. autofunction:: format_exception
-
 .. autoexception:: MultiError
 
    .. attribute:: exceptions
@@ -1046,6 +1044,38 @@ you return a new exception object, then the new object's
 ``__context__`` attribute will automatically be set to the original
 exception.
 
+We also monkey patch :class:`traceback.TracebackException` to be able
+to handle formatting :exc:`MultiError`\s. This means that anything that
+formats exception messages like :mod:`logging` will work out of the
+box::
+
+    import logging
+
+    logging.basicConfig()
+
+    try:
+        raise MultiError([ValueError("foo"), KeyError("bar")])
+    except:
+        logging.exception("Oh no!")
+        raise
+
+Will properly log the inner exceptions:
+
+.. code-block:: none
+
+    ERROR:root:Oh no!
+    Traceback (most recent call last):
+      File "<stdin>", line 2, in <module>
+    trio.MultiError: ValueError('foo',), KeyError('bar',)
+
+    Details of embedded exception 1:
+
+      ValueError: foo
+
+    Details of embedded exception 2:
+
+      KeyError: 'bar'
+
 
 Task-local storage
 ------------------

trio/_core/_multierror.py

@@ -7,6 +7,8 @@
 
 import attr
 
+from .._deprecate import deprecated
+
 __all__ = ["MultiError", "format_exception"]
 
 ################################################################
@@ -341,12 +343,14 @@ def concat_tb(head, tail):
 
 ################################################################
 # MultiError traceback formatting
+#
+# What follows is terrible, terrible monkey patching of
+# traceback.TracebackException to add support for handling
+# MultiErrors
 ################################################################
 
 
-# format_exception's semantics for limit= are odd: they apply separately to
-# each traceback. I'm not sure how much sense this makes, but we copy it
-# anyway.
+@deprecated("0.2.0", issue=347, instead="traceback.format_exception")
 def format_exception(etype, value, tb, *, limit=None, chain=True):
     """Like :func:`traceback.format_exception`, but with special support for
     printing :exc:`MultiError` objects.
@@ -355,60 +359,80 @@ def format_exception(etype, value, tb, *, limit=None, chain=True):
     thread at any time.
 
     """
-    return _format_exception_multi(set(), etype, value, tb, limit, chain)
+    return traceback.format_exception(
+        etype, value, tb, limit=limit, chain=chain
+    )
 
 
-def _format_exception_multi(seen, etype, value, tb, limit, chain):
-    if id(value) in seen:
-        return ["<duplicate exception {!r}>\n".format(value)]
-    seen.add(id(value))
+traceback_exception_original_init = traceback.TracebackException.__init__
+
+
+def traceback_exception_init(
+        self,
+        exc_type,
+        exc_value,
+        exc_traceback,
+        *,
+        limit=None,
+        lookup_lines=True,
+        capture_locals=False,
+        _seen=None
+):
+    if _seen is None:
+        _seen = set()
+
+    # Capture the original exception and its cause and context as TracebackExceptions
+    traceback_exception_original_init(
+        self,
+        exc_type,
+        exc_value,
+        exc_traceback,
+        limit=limit,
+        lookup_lines=lookup_lines,
+        capture_locals=capture_locals,
+        _seen=_seen
+    )
 
-    chunks = []
-    if chain:
-        if value.__cause__ is not None:
-            v = value.__cause__
-            chunks += _format_exception_multi(
-                seen, type(v), v, v.__traceback__, limit=limit, chain=True
-            )
-            chunks += [
-                "\nThe above exception was the direct cause of the "
-                "following exception:\n\n",
-            ]
-        elif value.__context__ is not None and not value.__suppress_context__:
-            v = value.__context__
-            chunks += _format_exception_multi(
-                seen, type(v), v, v.__traceback__, limit=limit, chain=True
-            )
-            chunks += [
-                "\nDuring handling of the above exception, another "
-                "exception occurred:\n\n",
-            ]
+    # Capture each of the exceptions in the MultiError along with each of their causes and contexts
+    if isinstance(exc_value, MultiError):
+        embedded = []
+        for exc in exc_value.exceptions:
+            if exc not in _seen:
+                embedded.append(
+                    traceback.TracebackException.from_exception(
+                        exc,
+                        limit=limit,
+                        lookup_lines=lookup_lines,
+                        capture_locals=capture_locals,
+                        # copy the set of _seen exceptions so that duplicates
+                        # shared between sub-exceptions are not omitted
+                        _seen=set(_seen)
+                    )
+                )
+        self.embedded = embedded
+    else:
+        self.embedded = []
 
-    chunks += traceback.format_exception(
-        etype, value, tb, limit=limit, chain=False
-    )
 
-    if isinstance(value, MultiError):
-        for i, exc in enumerate(value.exceptions):
-            chunks += [
-                "\nDetails of embedded exception {}:\n\n".format(i + 1),
-            ]
-            sub_chunks = _format_exception_multi(
-                seen,
-                type(exc),
-                exc,
-                exc.__traceback__,
-                limit=limit,
-                chain=chain
-            )
-            for chunk in sub_chunks:
-                chunks.append(textwrap.indent(chunk, " " * 2))
+traceback.TracebackException.__init__ = traceback_exception_init
+traceback_exception_original_format = traceback.TracebackException.format
+
+
+def traceback_exception_format(self, *, chain=True):
+    yield from traceback_exception_original_format(self, chain=chain)
+
+    for i, exc in enumerate(self.embedded):
+        yield "\nDetails of embedded exception {}:\n\n".format(i + 1)
+        yield from (
+            textwrap.indent(line, " " * 2) for line in exc.format(chain=chain)
+        )
+
 
-    return chunks
+traceback.TracebackException.format = traceback_exception_format
 
 
 def trio_excepthook(etype, value, tb):
-    for chunk in format_exception(etype, value, tb):
+    for chunk in traceback.format_exception(etype, value, tb):
         sys.stderr.write(chunk)
 
 

trio/_core/tests/test_multierror.py

@@ -1,6 +1,7 @@
+import logging
 import pytest
 
-from traceback import extract_tb, print_exception
+from traceback import extract_tb, print_exception, format_exception, _cause_message
 import sys
 import os
 import re
@@ -9,10 +10,9 @@
 
 from .tutil import slow
 
+from ..._deprecate import TrioDeprecationWarning
 from .._multierror import (
-    MultiError,
-    format_exception,
-    concat_tb,
+    MultiError, concat_tb, format_exception as trio_format_exception
 )
 
 
@@ -307,7 +307,7 @@ def test_assert_match_in_seq():
         assert_match_in_seq(["a", "b"], "xx b xx a xx")
 
 
-def test_format_exception_multi():
+def test_format_exception():
     def einfo(exc):
         return (type(exc), exc, exc.__traceback__)
 
@@ -329,6 +329,8 @@ def einfo(exc):
     assert "in raiser2_2" in formatted
     assert "direct cause" in formatted
     assert "During handling" not in formatted
+    # ensure cause included
+    assert _cause_message in formatted
 
     exc = get_exc(raiser1)
     exc.__context__ = get_exc(raiser2)
@@ -391,7 +393,8 @@ def einfo(exc):
     assert "in raiser1_3" in formatted
     assert "raiser2_string" not in formatted
     assert "in raiser2_2" not in formatted
-    assert "duplicate exception" in formatted
+    # ensure duplicate exception is not included as cause
+    assert _cause_message not in formatted
 
     # MultiError
     formatted = "".join(format_exception(*einfo(make_tree())))
@@ -423,6 +426,95 @@ def einfo(exc):
         formatted
     )
 
+    # Prints duplicate exceptions in sub-exceptions
+    exc1 = get_exc(raiser1)
+
+    def raise1_raiser1():
+        try:
+            raise exc1
+        except:
+            raise ValueError("foo")
+
+    def raise2_raiser1():
+        try:
+            raise exc1
+        except:
+            raise KeyError("bar")
+
+    exc2 = get_exc(raise1_raiser1)
+    exc3 = get_exc(raise2_raiser1)
+
+    try:
+        raise MultiError([exc2, exc3])
+    except MultiError as e:
+        exc = e
+
+    formatted = "".join(format_exception(*einfo(exc)))
+    print(formatted)
+
+    assert_match_in_seq(
+        [
+            r"Traceback",
+            # Outer exception is MultiError
+            r"MultiError:",
+            # First embedded exception is the embedded ValueError with cause of raiser1
+            r"\nDetails of embedded exception 1",
+            # Print details of exc1
+            r"  Traceback",
+            r"in get_exc",
+            r"in raiser1",
+            r"ValueError: raiser1_string",
+            # Print details of exc2
+            r"\n  During handling of the above exception, another exception occurred:",
+            r"  Traceback",
+            r"in get_exc",
+            r"in raise1_raiser1",
+            r"  ValueError: foo",
+            # Second embedded exception is the embedded KeyError with cause of raiser1
+            r"\nDetails of embedded exception 2",
+            # Print details of exc1 again
+            r"  Traceback",
+            r"in get_exc",
+            r"in raiser1",
+            r"ValueError: raiser1_string",
+            # Print details of exc3
+            r"\n  During handling of the above exception, another exception occurred:",
+            r"  Traceback",
+            r"in get_exc",
+            r"in raise2_raiser1",
+            r"  KeyError: 'bar'",
+        ],
+        formatted
+    )
+
+    # Deprecation
+
+    with pytest.warns(TrioDeprecationWarning) as record:
+        exc_info = einfo(make_tree())
+        assert format_exception(*exc_info) == trio_format_exception(*exc_info)
+
+    assert 'trio.format_exception is deprecated since Trio 0.2.0; ' \
+           'use traceback.format_exception instead' in record[0].message.args[0]
+
+
+def test_logging(caplog):
+    exc1 = get_exc(raiser1)
+    exc2 = get_exc(raiser2)
+
+    m = MultiError([exc1, exc2])
+
+    message = "test test test"
+    try:
+        raise m
+    except MultiError as exc:
+        logging.getLogger().exception(message)
+        # Join lines together
+        formatted = "".join(
+            format_exception(type(exc), exc, exc.__traceback__)
+        )
+        assert message in caplog.text
+        assert formatted in caplog.text
+
 
 def run_script(name, use_ipython=False):
     import trio

trio/_toplevel_core_reexports.py

@@ -14,10 +14,21 @@
 # a test to make sure that every _core export does get re-exported in one of
 # these places or another.
 __all__ = [
-    "TrioInternalError", "RunFinishedError", "WouldBlock", "Cancelled",
-    "ResourceBusyError", "MultiError", "format_exception", "run",
-    "open_nursery", "open_cancel_scope", "current_effective_deadline",
-    "STATUS_IGNORED", "current_time", "current_instruments", "TaskLocal"
+    "TrioInternalError",
+    "RunFinishedError",
+    "WouldBlock",
+    "Cancelled",
+    "ResourceBusyError",
+    "MultiError",
+    "run",
+    "format_exception",
+    "open_nursery",
+    "open_cancel_scope",
+    "current_effective_deadline",
+    "STATUS_IGNORED",
+    "current_time",
+    "current_instruments",
+    "TaskLocal",
 ]
 
 from . import _core