From 66f9ee4afd0dae2a0db139f17b62ec2b184ac0b4 Mon Sep 17 00:00:00 2001 From: Jeremy Maitin-Shepard Date: Sun, 3 Jul 2022 17:56:14 -0700 Subject: [PATCH] Make `-P` (pdb) work better with exceptions triggered from events Previously, if an exception was raised from an event listener, and the `-P` option was specified, the debugger would be started not for the original error but for the `ExtensionError` wrapping it that was raised by `EventManager.emit`. That made it difficult to debug the error. With this change, when `-P` is specified, wrapping of errors in `ExtensionError` is disabled, which allows pdb to debug the original error. --- CHANGES | 1 + sphinx/application.py | 4 +++- sphinx/cmd/build.py | 3 ++- sphinx/events.py | 3 +++ tests/test_events.py | 23 ++++++++++++++++++++++- 5 files changed, 31 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 58a2bc0f883..dc99b1d7d60 100644 --- a/CHANGES +++ b/CHANGES @@ -30,6 +30,7 @@ Features added Bugs fixed ---------- +* #10634: Make -P (pdb) option work better with exceptions triggered from events * #10031: py domain: Fix spurious whitespace in unparsing various operators (``+``, ``-``, ``~``, and ``**``). Patch by Adam Turner. * #10460: logging: Always show node source locations as absolute paths. diff --git a/sphinx/application.py b/sphinx/application.py index 08c4af5ba9b..c11b489169e 100644 --- a/sphinx/application.py +++ b/sphinx/application.py @@ -131,7 +131,8 @@ def __init__(self, srcdir: str, confdir: Optional[str], outdir: str, doctreedir: buildername: str, confoverrides: Dict = None, status: Optional[IO] = sys.stdout, warning: Optional[IO] = sys.stderr, freshenv: bool = False, warningiserror: bool = False, tags: List[str] = None, - verbosity: int = 0, parallel: int = 0, keep_going: bool = False) -> None: + verbosity: int = 0, parallel: int = 0, keep_going: bool = False, + pdb: bool = False) -> None: self.phase = BuildPhase.INITIALIZATION self.verbosity = verbosity self.extensions: Dict[str, Extension] = {} @@ -173,6 +174,7 @@ def __init__(self, srcdir: str, confdir: Optional[str], outdir: str, doctreedir: self.warningiserror = False else: self.warningiserror = warningiserror + self.pdb = pdb logging.setup(self, self._status, self._warning) self.events = EventManager(self) diff --git a/sphinx/cmd/build.py b/sphinx/cmd/build.py index ce0b14c75f1..259af2eb2f8 100644 --- a/sphinx/cmd/build.py +++ b/sphinx/cmd/build.py @@ -272,7 +272,8 @@ def build_main(argv: List[str] = sys.argv[1:]) -> int: app = Sphinx(args.sourcedir, args.confdir, args.outputdir, args.doctreedir, args.builder, confoverrides, status, warning, args.freshenv, args.warningiserror, - args.tags, args.verbosity, args.jobs, args.keep_going) + args.tags, args.verbosity, args.jobs, args.keep_going, + args.pdb) app.build(args.force_all, filenames) return app.statuscode except (Exception, KeyboardInterrupt) as exc: diff --git a/sphinx/events.py b/sphinx/events.py index 5302cd00597..448af069570 100644 --- a/sphinx/events.py +++ b/sphinx/events.py @@ -98,6 +98,9 @@ def emit(self, name: str, *args: Any, except SphinxError: raise except Exception as exc: + if self.app.pdb: + # Just pass through the error, so that it can be debugged. + raise modname = safe_getattr(listener.handler, '__module__', None) raise ExtensionError(__("Handler %r for event %r threw an exception") % (listener.handler, name), exc, modname=modname) from exc diff --git a/tests/test_events.py b/tests/test_events.py index f36c86a87c1..8f01a673a9f 100644 --- a/tests/test_events.py +++ b/tests/test_events.py @@ -19,11 +19,16 @@ def test_event_priority(): assert result == [3, 1, 2, 5, 4] +class FakeApp: + def __init__(self, pdb: bool = False): + self.pdb = pdb + + def test_event_allowed_exceptions(): def raise_error(app): raise RuntimeError - events = EventManager(object()) # pass an dummy object as an app + events = EventManager(FakeApp()) # pass an dummy object as an app events.connect('builder-inited', raise_error, priority=500) # all errors are converted to ExtensionError @@ -33,3 +38,19 @@ def raise_error(app): # Allow RuntimeError (pass-through) with pytest.raises(RuntimeError): events.emit('builder-inited', allowed_exceptions=(RuntimeError,)) + + +def test_event_pdb(): + def raise_error(app): + raise RuntimeError + + events = EventManager(FakeApp(pdb=True)) # pass an dummy object as an app + events.connect('builder-inited', raise_error, priority=500) + + # errors aren't converted + with pytest.raises(RuntimeError): + events.emit('builder-inited') + + # Allow RuntimeError (pass-through) + with pytest.raises(RuntimeError): + events.emit('builder-inited', allowed_exceptions=(RuntimeError,))