From 245b7f4f64786078b40635c4d7849a6f2594e3ed Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Tue, 30 Jul 2024 14:40:52 +0200 Subject: [PATCH 1/6] gh-87320: dont crash in code module with bad sys.excepthook --- Lib/code.py | 15 ++++++++++-- Lib/test/test_code_module.py | 11 +++++++++ Lib/test/test_pyrepl/test_pyrepl.py | 24 +++++++++++++++++++ ...4-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst | 2 ++ 4 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst diff --git a/Lib/code.py b/Lib/code.py index a55fced0704b1d..adce8e46444de1 100644 --- a/Lib/code.py +++ b/Lib/code.py @@ -129,7 +129,7 @@ def showsyntaxerror(self, filename=None, **kwargs): else: # If someone has set sys.excepthook, we let that take precedence # over self.write - sys.excepthook(type, value, tb) + self._call_excepthook(type, value, tb) def showtraceback(self, **kwargs): """Display the exception that just occurred. @@ -150,10 +150,21 @@ def showtraceback(self, **kwargs): else: # If someone has set sys.excepthook, we let that take precedence # over self.write - sys.excepthook(ei[0], ei[1], last_tb) + self._call_excepthook(ei[0], ei[1], last_tb) finally: last_tb = ei = None + def _call_excepthook(self, typ, value, tb): + try: + sys.excepthook(typ, value, tb) + except Exception as e: + e.__context__ = None + print('Error in sys.excepthook:', file=sys.stderr) + sys.__excepthook__(type(e), e, e.__traceback__.tb_next) + print(file=sys.stderr) + print('Original exception was:', file=sys.stderr) + sys.__excepthook__(typ, value, tb) + def write(self, data): """Write a string. diff --git a/Lib/test/test_code_module.py b/Lib/test/test_code_module.py index 259778a5cade98..ee0a65ce2cf29b 100644 --- a/Lib/test/test_code_module.py +++ b/Lib/test/test_code_module.py @@ -77,6 +77,17 @@ def test_sysexcepthook(self): self.console.interact() self.assertTrue(hook.called) + def test_sysexcepthook_crashing_doesnt_close_repl(self): + self.infunc.side_effect = ["1/0", "a = 123", "print(a)", EOFError('Finished')] + self.sysmod.excepthook = 1 + self.console.interact() + self.assertEqual(['write', ('123', ), {}], self.stdout.method_calls[0]) + error = "".join(call.args[0] for call in self.stderr.method_calls if call[0] == 'write') + self.assertIn("Error in sys.excepthook:", error) + self.assertEqual(error.count("'int' object is not callable"), 1) + self.assertIn("Original exception was:", error) + self.assertIn("division by zero", error) + def test_banner(self): # with banner self.infunc.side_effect = EOFError('Finished') diff --git a/Lib/test/test_pyrepl/test_pyrepl.py b/Lib/test/test_pyrepl/test_pyrepl.py index 3a1bacef8a1756..d5eafc5eb58cac 100644 --- a/Lib/test/test_pyrepl/test_pyrepl.py +++ b/Lib/test/test_pyrepl/test_pyrepl.py @@ -1049,6 +1049,30 @@ def test_python_basic_repl(self): self.assertNotIn("Exception", output) self.assertNotIn("Traceback", output) + @force_not_colorized + def test_bad_sys_excepthook_doesnt_crash_pyrepl(self): + env = os.environ.copy() + commands = ("import sys\n" + "sys.excepthook = 1\n" + "1/0\n" + "exit()\n") + + def check(output, exitcode): + self.assertIn("Error in sys.excepthook:", output) + self.assertEqual(output.count("'int' object is not callable"), 1) + self.assertIn("Original exception was:", output) + self.assertIn("division by zero", output) + self.assertEqual(exitcode, 0) + env.pop("PYTHON_BASIC_REPL", None) + output, exit_code = self.run_repl(commands, env=env) + if "can\'t use pyrepl" in output: + self.skipTest("pyrepl not available") + check(output, exit_code) + + env["PYTHON_BASIC_REPL"] = "1" + output, exit_code = self.run_repl(commands, env=env) + check(output, exit_code) + def test_not_wiping_history_file(self): # skip, if readline module is not available import_module('readline') diff --git a/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst b/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst new file mode 100644 index 00000000000000..b20953db87bcf0 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst @@ -0,0 +1,2 @@ +Don't crash :class:`code.InteractiveInterpreter` when calling +:func:`sys.excepthook` fails. From 2686ca4657b9382e40e26caad366770c5c1852ef Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Tue, 30 Jul 2024 16:49:04 +0200 Subject: [PATCH 2/6] make sure to catch BaseException too --- Lib/code.py | 2 +- Lib/test/test_code_module.py | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/Lib/code.py b/Lib/code.py index adce8e46444de1..811a2b08b89f4b 100644 --- a/Lib/code.py +++ b/Lib/code.py @@ -157,7 +157,7 @@ def showtraceback(self, **kwargs): def _call_excepthook(self, typ, value, tb): try: sys.excepthook(typ, value, tb) - except Exception as e: + except BaseException as e: e.__context__ = None print('Error in sys.excepthook:', file=sys.stderr) sys.__excepthook__(type(e), e, e.__traceback__.tb_next) diff --git a/Lib/test/test_code_module.py b/Lib/test/test_code_module.py index ee0a65ce2cf29b..ba669065921af2 100644 --- a/Lib/test/test_code_module.py +++ b/Lib/test/test_code_module.py @@ -88,6 +88,21 @@ def test_sysexcepthook_crashing_doesnt_close_repl(self): self.assertIn("Original exception was:", error) self.assertIn("division by zero", error) + def test_sysexcepthook_raising_BaseException(self): + self.infunc.side_effect = ["1/0", "a = 123", "print(a)", EOFError('Finished')] + s = "not so fast" + def raise_base(*args, **kwargs): + raise BaseException(s) + self.sysmod.excepthook = raise_base + self.console.interact() + self.assertEqual(['write', ('123', ), {}], self.stdout.method_calls[0]) + error = "".join(call.args[0] for call in self.stderr.method_calls if call[0] == 'write') + self.stack.close() + self.assertIn("Error in sys.excepthook:", error) + self.assertEqual(error.count("not so fast"), 1) + self.assertIn("Original exception was:", error) + self.assertIn("division by zero", error) + def test_banner(self): # with banner self.infunc.side_effect = EOFError('Finished') From 5d25c52d152db30495987a60a593469f1b31e6ac Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Tue, 30 Jul 2024 16:54:25 +0200 Subject: [PATCH 3/6] dont compute lines if we have an overwritten sys.excepthook --- Lib/code.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/code.py b/Lib/code.py index 811a2b08b89f4b..40ff74d840ebcd 100644 --- a/Lib/code.py +++ b/Lib/code.py @@ -144,8 +144,8 @@ def showtraceback(self, **kwargs): sys.last_traceback = last_tb sys.last_exc = ei[1] try: - lines = traceback.format_exception(ei[0], ei[1], last_tb.tb_next, colorize=colorize) if sys.excepthook is sys.__excepthook__: + lines = traceback.format_exception(ei[0], ei[1], last_tb.tb_next, colorize=colorize) self.write(''.join(lines)) else: # If someone has set sys.excepthook, we let that take precedence From c8bec90d226cfe7d3e6ceba1bad19993697dac7a Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Tue, 30 Jul 2024 17:13:01 +0200 Subject: [PATCH 4/6] remove debugging remnant --- Lib/test/test_code_module.py | 1 - 1 file changed, 1 deletion(-) diff --git a/Lib/test/test_code_module.py b/Lib/test/test_code_module.py index ba669065921af2..41bf1b50ade164 100644 --- a/Lib/test/test_code_module.py +++ b/Lib/test/test_code_module.py @@ -97,7 +97,6 @@ def raise_base(*args, **kwargs): self.console.interact() self.assertEqual(['write', ('123', ), {}], self.stdout.method_calls[0]) error = "".join(call.args[0] for call in self.stderr.method_calls if call[0] == 'write') - self.stack.close() self.assertIn("Error in sys.excepthook:", error) self.assertEqual(error.count("not so fast"), 1) self.assertIn("Original exception was:", error) From 997ed0926d724599f703417197cf63a3788837bb Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Tue, 30 Jul 2024 20:31:16 +0200 Subject: [PATCH 5/6] let SystemExit through --- Lib/code.py | 2 ++ Lib/test/test_code_module.py | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/Lib/code.py b/Lib/code.py index 40ff74d840ebcd..cd3889067d068d 100644 --- a/Lib/code.py +++ b/Lib/code.py @@ -157,6 +157,8 @@ def showtraceback(self, **kwargs): def _call_excepthook(self, typ, value, tb): try: sys.excepthook(typ, value, tb) + except SystemExit: + raise except BaseException as e: e.__context__ = None print('Error in sys.excepthook:', file=sys.stderr) diff --git a/Lib/test/test_code_module.py b/Lib/test/test_code_module.py index 41bf1b50ade164..5dc89108f0ad88 100644 --- a/Lib/test/test_code_module.py +++ b/Lib/test/test_code_module.py @@ -102,6 +102,14 @@ def raise_base(*args, **kwargs): self.assertIn("Original exception was:", error) self.assertIn("division by zero", error) + def test_sysexcepthook_raising_SystemExit_gets_through(self): + self.infunc.side_effect = ["1/0"] + def raise_base(*args, **kwargs): + raise SystemExit + self.sysmod.excepthook = raise_base + with self.assertRaises(SystemExit): + self.console.interact() + def test_banner(self): # with banner self.infunc.side_effect = EOFError('Finished') From 84eafcb879a96a02e744d9ed8b03a6e35bb10199 Mon Sep 17 00:00:00 2001 From: CF Bolz-Tereick Date: Wed, 31 Jul 2024 12:08:54 +0200 Subject: [PATCH 6/6] try to improve the news --- .../Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst b/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst index b20953db87bcf0..4322b719c690c2 100644 --- a/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst +++ b/Misc/NEWS.d/next/Library/2024-07-30-14-46-16.gh-issue-87320.-Yk1wb.rst @@ -1,2 +1,3 @@ -Don't crash :class:`code.InteractiveInterpreter` when calling -:func:`sys.excepthook` fails. +In :class:`code.InteractiveInterpreter`, handle exceptions caused by calling a +non-default :func:`sys.excepthook`. Before, the exception bubbled up to the +caller, ending the :term:`REPL`.