Merge pull request #1528 from njsmith/apport-excepthook

pquentin · web-flow · commit 66172c86075c · 2020-05-17T19:31:37.000+04:00
Make Trio's excepthook play nicely with Ubuntu's excepthook
diff --git a/ci.sh b/ci.sh
@@ -407,6 +407,11 @@ else
     # Actual tests
     python -m pip install -r test-requirements.txt
 
+    # So we can run the test for our apport/excepthook interaction working
+    if [ -e /etc/lsb-release ] && grep -q Ubuntu /etc/lsb-release; then
+        sudo apt install -q python3-apport
+    fi
+
     # If we're testing with a LSP installed, then it might break network
     # stuff, so wait until after we've finished setting everything else
     # up.
diff --git a/newsfragments/1065.bugfix.rst b/newsfragments/1065.bugfix.rst
@@ -0,0 +1,8 @@
+On Ubuntu systems, the system Python includes a custom
+unhandled-exception hook to perform `crash reporting
+<https://wiki.ubuntu.com/Apport>`__. Unfortunately, Trio wants to use
+the same hook to print nice `MultiError` tracebacks, causing a
+conflict. Previously, Trio would detect the conflict, print a warning,
+and you just wouldn't get nice `MultiError` tracebacks. Now, Trio has
+gotten clever enough to integrate its hook with Ubuntu's, so the two
+systems should Just Work together.
diff --git a/trio/_core/_multierror.py b/trio/_core/_multierror.py
@@ -434,8 +434,8 @@ def trio_excepthook(etype, value, tb):
         sys.stderr.write(chunk)
 
 
-IPython_handler_installed = False
-warning_given = False
+monkeypatched_or_warned = False
+
 if "IPython" in sys.modules:
     import IPython
     ip = IPython.get_ipython()
@@ -448,7 +448,7 @@ def trio_excepthook(etype, value, tb):
                 "tracebacks.",
                 category=RuntimeWarning
             )
-            warning_given = True
+            monkeypatched_or_warned = True
         else:
 
             def trio_show_traceback(self, etype, value, tb, tb_offset=None):
@@ -457,15 +457,44 @@ def trio_show_traceback(self, etype, value, tb, tb_offset=None):
                 trio_excepthook(etype, value, tb)
 
             ip.set_custom_exc((MultiError,), trio_show_traceback)
-            IPython_handler_installed = True
+            monkeypatched_or_warned = True
 
 if sys.excepthook is sys.__excepthook__:
     sys.excepthook = trio_excepthook
-else:
-    if not IPython_handler_installed and not warning_given:
-        warnings.warn(
-            "You seem to already have a custom sys.excepthook handler "
-            "installed. I'll skip installing Trio's custom handler, but this "
-            "means MultiErrors will not show full tracebacks.",
-            category=RuntimeWarning
-        )
+    monkeypatched_or_warned = True
+
+# Ubuntu's system Python has a sitecustomize.py file that import
+# apport_python_hook and replaces sys.excepthook.
+#
+# The custom hook captures the error for crash reporting, and then calls
+# sys.__excepthook__ to actually print the error.
+#
+# We don't mind it capturing the error for crash reporting, but we want to
+# take over printing the error. So we monkeypatch the apport_python_hook
+# module so that instead of calling sys.__excepthook__, it calls our custom
+# hook.
+#
+# More details: https://github.com/python-trio/trio/issues/1065
+if sys.excepthook.__name__ == "apport_excepthook":
+    import apport_python_hook
+    assert sys.excepthook is apport_python_hook.apport_excepthook
+
+    # Give it a descriptive name as a hint for anyone who's stuck trying to
+    # debug this mess later.
+    class TrioFakeSysModuleForApport:
+        pass
+
+    fake_sys = TrioFakeSysModuleForApport()
+    fake_sys.__dict__.update(sys.__dict__)
+    fake_sys.__excepthook__ = trio_excepthook
+    apport_python_hook.sys = fake_sys
+
+    monkeypatched_or_warned = True
+
+if not monkeypatched_or_warned:
+    warnings.warn(
+        "You seem to already have a custom sys.excepthook handler "
+        "installed. I'll skip installing Trio's custom handler, but this "
+        "means MultiErrors will not show full tracebacks.",
+        category=RuntimeWarning
+    )
diff --git a/trio/_core/tests/test_multierror.py b/trio/_core/tests/test_multierror.py
@@ -709,3 +709,26 @@ def test_ipython_custom_exc_handler():
     )
     # Make sure our other warning doesn't show up
     assert "custom sys.excepthook" not in completed.stdout.decode("utf-8")
+
+
+@slow
+@pytest.mark.skipif(
+    not Path("/usr/lib/python3/dist-packages/apport_python_hook.py").exists(),
+    reason="need Ubuntu with python3-apport installed"
+)
+def test_apport_excepthook_monkeypatch_interaction():
+    completed = run_script("apport_excepthook.py")
+    stdout = completed.stdout.decode("utf-8")
+
+    # No warning
+    assert "custom sys.excepthook" not in stdout
+
+    # Proper traceback
+    assert_match_in_seq(
+        [
+            "Details of embedded",
+            "KeyError",
+            "Details of embedded",
+            "ValueError",
+        ], stdout
+    )
diff --git a/trio/_core/tests/test_multierror_scripts/apport_excepthook.py b/trio/_core/tests/test_multierror_scripts/apport_excepthook.py
@@ -0,0 +1,11 @@
+# The apport_python_hook package is only installed as part of Ubuntu's system
+# python, and not available in venvs. So before we can import it we have to
+# make sure it's on sys.path.
+import sys
+sys.path.append("/usr/lib/python3/dist-packages")
+import apport_python_hook
+apport_python_hook.install()
+
+import trio
+
+raise trio.MultiError([KeyError("key_error"), ValueError("value_error")])
