From ebcde15eb7ac58482ec0ce9123e82f3c730cecae Mon Sep 17 00:00:00 2001 From: "Nathaniel J. Smith" Date: Sat, 16 May 2020 17:07:37 -0700 Subject: [PATCH] Make Trio's excepthook play nicely with Ubuntu's excepthook Fixes gh-1065 --- .github/workflows/ci.yml | 2 - ci.sh | 5 ++ newsfragments/1065.bugfix.rst | 8 +++ trio/_core/_multierror.py | 53 ++++++++++++++----- trio/_core/tests/test_multierror.py | 23 ++++++++ .../apport_excepthook.py | 11 ++++ 6 files changed, 88 insertions(+), 14 deletions(-) create mode 100644 newsfragments/1065.bugfix.rst create mode 100644 trio/_core/tests/test_multierror_scripts/apport_excepthook.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04a96c495..969f04eb7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,8 +2,6 @@ name: CI on: push: - branches: - - master pull_request: jobs: diff --git a/ci.sh b/ci.sh index aed1974dd..49757a9dc 100755 --- 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 + echo "not installing 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 new file mode 100644 index 000000000..737680bd4 --- /dev/null +++ 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 +`__. 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 index 00d44e30e..f98540344 100644 --- 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 index 83894f358..6debf4d45 100644 --- 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 new file mode 100644 index 000000000..ac8110f36 --- /dev/null +++ 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")])