diff --git a/newsfragments/3290.bugfix.rst b/newsfragments/3290.bugfix.rst new file mode 100644 index 000000000..9d0fd68bd --- /dev/null +++ b/newsfragments/3290.bugfix.rst @@ -0,0 +1,3 @@ +A failing exception ``__repr__`` no longer interrupts nursery bookkeeping while +constructing cancellation diagnostics. Cancellation reasons use a placeholder +when representing the exception raises an error, including ``RecursionError``. diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index ddd4ea7ee..76980c197 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -319,6 +319,15 @@ def expire(self, now: float) -> bool: return did_something +def _safe_exception_repr(exc: BaseException) -> str: + try: + return repr(exc) + except BaseException: + # Formatting diagnostics must not interfere with nursery cleanup, even if + # a user-defined __repr__ raises KeyboardInterrupt or SystemExit. + return "" + + @attrs.define class CancelReason: """Attached to a :class:`CancelScope` upon cancellation with details of the source of the @@ -1265,7 +1274,7 @@ def _child_finished( CancelReason( source="nursery", source_task=repr(task), - reason=f"child task raised exception {outcome.error!r}", + reason=f"child task raised exception {_safe_exception_repr(outcome.error)}", ), ) self._check_nursery_closed() @@ -1282,7 +1291,7 @@ async def _nested_child_finished( reason=CancelReason( source="nursery", source_task=repr(self._parent_task), - reason=f"Code block inside nursery contextmanager raised exception {nested_child_exc!r}", + reason=f"Code block inside nursery contextmanager raised exception {_safe_exception_repr(nested_child_exc)}", ), ) self._nested_child_running = False diff --git a/src/trio/_core/_tests/test_cancelled.py b/src/trio/_core/_tests/test_cancelled.py index 0280b6068..5af9f9ff9 100644 --- a/src/trio/_core/_tests/test_cancelled.py +++ b/src/trio/_core/_tests/test_cancelled.py @@ -1,5 +1,8 @@ import pickle import re +import subprocess +import sys +import textwrap from math import inf import pytest @@ -10,6 +13,10 @@ from trio.testing import wait_all_tasks_blocked from .test_ki import ki_self +from .tutil import slow + +if sys.version_info < (3, 11): + from exceptiongroup import ExceptionGroup def test_Cancelled_init() -> None: @@ -154,6 +161,108 @@ async def cancelled_task() -> None: await nursery.start(failing_task) +@pytest.mark.parametrize("from_child", [False, True]) +@pytest.mark.parametrize("grouped", [False, True]) +@pytest.mark.parametrize( + "repr_error", [ValueError, RecursionError, KeyboardInterrupt, SystemExit] +) +async def test_nursery_exception_repr_failure( + from_child: bool, + grouped: bool, + repr_error: type[BaseException], +) -> None: + class UnprintableError(Exception): + def __repr__(self) -> str: + raise repr_error + + original = UnprintableError() + error: Exception = original + if grouped: + for _ in range(20): + error = ExceptionGroup("nested", [error]) + + reasons: list[str | None] = [] + sources: list[str] = [] + finished: list[bool] = [] + + async def sibling() -> None: + try: + await trio.sleep_forever() + except Cancelled as exc: + reasons.append(exc.reason) + sources.append(exc.source) + raise + finally: + finished.append(True) + + async def failing_task() -> None: + raise error + + with pytest.raises(ExceptionGroup) as excinfo: + async with trio.open_nursery() as nursery: + nursery.start_soon(sibling) + await wait_all_tasks_blocked() + if from_child: + nursery.start_soon(failing_task) + else: + raise error + + propagated: BaseException = excinfo.value + while isinstance(propagated, ExceptionGroup): + assert len(propagated.exceptions) == 1 + propagated = propagated.exceptions[0] + assert propagated is original + assert finished == [True] + assert sources == ["nursery"] + prefix = "child task" if from_child else "Code block inside nursery contextmanager" + assert reasons == [f"{prefix} raised exception "] + # The nursery has finished unwinding; cancellation must not leak into its parent. + await trio.lowlevel.checkpoint() + + +@slow +def test_nursery_exception_repr_recursion() -> None: + # A real recursion overflow disables coverage tracing on PyPy. Isolate it so + # coverage can still measure the remaining tests in the parent process. + code = textwrap.dedent( + """ + import sys + import trio + + if sys.version_info < (3, 11): + from exceptiongroup import ExceptionGroup + + class RecursiveError(Exception): + def __repr__(self): + return repr(self) + + original = RecursiveError() + + async def failing_task(): + raise original + + async def main(): + try: + async with trio.open_nursery() as nursery: + nursery.start_soon(failing_task) + except ExceptionGroup as exc: + assert len(exc.exceptions) == 1 + assert exc.exceptions[0] is original + else: + raise AssertionError("original exception was lost") + + trio.run(main) + """, + ) + result = subprocess.run( + [sys.executable, "-c", code], + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stderr + + async def test_cancel_reason_not_overwritten() -> None: with trio.CancelScope() as cs: cs.cancel()