diff --git a/docs/source/reference-core.rst b/docs/source/reference-core.rst index fe187b36ce..24d7cee380 100644 --- a/docs/source/reference-core.rst +++ b/docs/source/reference-core.rst @@ -768,9 +768,14 @@ inside the handler function(s) with the ``nonlocal`` keyword:: async with trio.open_nursery() as nursery: nursery.start_soon(broken1) +.. _strict_exception_groups: + "Strict" versus "loose" ExceptionGroup semantics ++++++++++++++++++++++++++++++++++++++++++++++++ +.. + TODO: rewrite this (and possible other) sections from the new strict-by-default perspective, under the heading "Deprecated: non-strict ExceptionGroups" - to explain that it only exists for backwards-compatibility, will be removed in future, and that we recommend against it for all new code. + Ideally, in some abstract sense we'd want everything that *can* raise an `ExceptionGroup` to *always* raise an `ExceptionGroup` (rather than, say, a single `ValueError`). Otherwise, it would be easy to accidentally write something like ``except @@ -796,9 +801,10 @@ to set the default behavior for any nursery in your program that doesn't overrid wrapping, so you'll get maximum compatibility with code that was written to support older versions of Trio. -To maintain backwards compatibility, the default is ``strict_exception_groups=False``. -The default will eventually change to ``True`` in a future version of Trio, once -Python 3.11 and later versions are in wide use. +The default is set to ``strict_exception_groups=True``, in line with the default behaviour +of ``TaskGroup`` in asyncio and anyio. We've also found that non-strict mode makes it +too easy to neglect the possibility of several exceptions being raised concurrently, +causing nasty latent bugs when errors occur under load. .. _exceptiongroup: https://pypi.org/project/exceptiongroup/ diff --git a/newsfragments/2786.breaking.rst b/newsfragments/2786.breaking.rst new file mode 100644 index 0000000000..2cf1289657 --- /dev/null +++ b/newsfragments/2786.breaking.rst @@ -0,0 +1,10 @@ +The :ref:`strict_exception_groups ` parameter now defaults to `True` in `trio.run` and `trio.lowlevel.start_guest_run`. `trio.open_nursery` still defaults to the same value as was specified in `trio.run`/`trio.lowlevel.start_guest_run`, but if you didn't specify it there then all subsequent calls to `trio.open_nursery` will change. +This is unfortunately very tricky to change with a deprecation period, as raising a `DeprecationWarning` whenever :ref:`strict_exception_groups ` is not specified would raise a lot of unnecessary warnings. + +Notable side effects of changing code to run with ``strict_exception_groups==True`` + +* If an iterator raises `StopAsyncIteration` or `StopIteration` inside a nursery, then python will not recognize wrapped instances of those for stopping iteration. +* `trio.run_process` is now documented that it can raise an `ExceptionGroup`. It previously could do this in very rare circumstances, but with :ref:`strict_exception_groups ` set to `True` it will now do so whenever exceptions occur in ``deliver_cancel`` or with problems communicating with the subprocess. + + * Errors in opening the process is now done outside the internal nursery, so if code previously ran with ``strict_exception_groups=True`` there are cases now where an `ExceptionGroup` is *no longer* added. +* `trio.TrioInternalError` ``.__cause__`` might be wrapped in one or more `ExceptionGroups ` diff --git a/pyproject.toml b/pyproject.toml index 7f8ad2ca19..f2e05a1d15 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -111,6 +111,7 @@ extend-ignore = [ 'E402', # module-import-not-at-top-of-file (usually OS-specific) 'E501', # line-too-long 'SIM117', # multiple-with-statements (messes up lots of context-based stuff and looks bad) + 'PT012', # multiple statements in pytest.raises block ] include = ["*.py", "*.pyi", "**/pyproject.toml"] diff --git a/src/trio/_core/_run.py b/src/trio/_core/_run.py index 72a582ac48..7282883723 100644 --- a/src/trio/_core/_run.py +++ b/src/trio/_core/_run.py @@ -928,7 +928,7 @@ class NurseryManager: """ - strict_exception_groups: bool = attr.ib(default=False) + strict_exception_groups: bool = attr.ib(default=True) @enable_ki_protection async def __aenter__(self) -> Nursery: @@ -995,9 +995,10 @@ def open_nursery( have exited. Args: - strict_exception_groups (bool): If true, even a single raised exception will be - wrapped in an exception group. This will eventually become the default - behavior. If not specified, uses the value passed to :func:`run`. + strict_exception_groups (bool): Unless set to False, even a single raised exception + will be wrapped in an exception group. If not specified, uses the value passed + to :func:`run`, which defaults to true. Setting it to False will be deprecated + and ultimately removed in a future version of Trio. """ if strict_exception_groups is None: @@ -2162,7 +2163,7 @@ def run( clock: Clock | None = None, instruments: Sequence[Instrument] = (), restrict_keyboard_interrupt_to_checkpoints: bool = False, - strict_exception_groups: bool = False, + strict_exception_groups: bool = True, ) -> RetT: """Run a Trio-flavored async function, and return the result. @@ -2219,9 +2220,10 @@ def run( main thread (this is a Python limitation), or if you use :func:`open_signal_receiver` to catch SIGINT. - strict_exception_groups (bool): If true, nurseries will always wrap even a single - raised exception in an exception group. This can be overridden on the level of - individual nurseries. This will eventually become the default behavior. + strict_exception_groups (bool): Unless set to False, nurseries will always wrap + even a single raised exception in an exception group. This can be overridden + on the level of individual nurseries. Setting it to False will be deprecated + and ultimately removed in a future version of Trio. Returns: Whatever ``async_fn`` returns. @@ -2279,7 +2281,7 @@ def start_guest_run( clock: Clock | None = None, instruments: Sequence[Instrument] = (), restrict_keyboard_interrupt_to_checkpoints: bool = False, - strict_exception_groups: bool = False, + strict_exception_groups: bool = True, ) -> None: """Start a "guest" run of Trio on top of some other "host" event loop. diff --git a/src/trio/_core/_tests/test_ki.py b/src/trio/_core/_tests/test_ki.py index cd98bc9bca..e4241fc762 100644 --- a/src/trio/_core/_tests/test_ki.py +++ b/src/trio/_core/_tests/test_ki.py @@ -9,6 +9,8 @@ import outcome import pytest +from trio.testing import RaisesGroup + try: from async_generator import async_generator, yield_ except ImportError: # pragma: no cover @@ -293,7 +295,8 @@ async def check_unprotected_kill() -> None: nursery.start_soon(sleeper, "s2", record_set) nursery.start_soon(raiser, "r1", record_set) - with pytest.raises(KeyboardInterrupt): + # raises inside a nursery, so the KeyboardInterrupt is wrapped in an ExceptionGroup + with RaisesGroup(KeyboardInterrupt): _core.run(check_unprotected_kill) assert record_set == {"s1 ok", "s2 ok", "r1 raise ok"} @@ -309,7 +312,8 @@ async def check_protected_kill() -> None: nursery.start_soon(_core.enable_ki_protection(raiser), "r1", record_set) # __aexit__ blocks, and then receives the KI - with pytest.raises(KeyboardInterrupt): + # raises inside a nursery, so the KeyboardInterrupt is wrapped in an ExceptionGroup + with RaisesGroup(KeyboardInterrupt): _core.run(check_protected_kill) assert record_set == {"s1 ok", "s2 ok", "r1 cancel ok"} @@ -331,6 +335,7 @@ def kill_during_shutdown() -> None: token.run_sync_soon(kill_during_shutdown) + # no nurseries involved, so the KeyboardInterrupt isn't wrapped with pytest.raises(KeyboardInterrupt): _core.run(check_kill_during_shutdown) @@ -344,6 +349,7 @@ def before_run(self) -> None: async def main_1() -> None: await _core.checkpoint() + # no nurseries involved, so the KeyboardInterrupt isn't wrapped with pytest.raises(KeyboardInterrupt): _core.run(main_1, instruments=[InstrumentOfDeath()]) diff --git a/src/trio/_core/_tests/test_run.py b/src/trio/_core/_tests/test_run.py index 8c22528989..adfee663cb 100644 --- a/src/trio/_core/_tests/test_run.py +++ b/src/trio/_core/_tests/test_run.py @@ -127,23 +127,21 @@ async def test_nursery_warn_use_async_with() -> None: async def test_nursery_main_block_error_basic() -> None: exc = ValueError("whoops") - with pytest.raises(ValueError, match="^whoops$") as excinfo: + with RaisesGroup(Matcher(check=lambda e: e is exc)): async with _core.open_nursery(): raise exc - assert excinfo.value is exc async def test_child_crash_basic() -> None: - exc = ValueError("uh oh") + my_exc = ValueError("uh oh") async def erroring() -> NoReturn: - raise exc + raise my_exc - with pytest.raises(ValueError, match="^uh oh$") as excinfo: + with RaisesGroup(Matcher(check=lambda e: e is my_exc)): # nursery.__aexit__ propagates exception from child back to parent async with _core.open_nursery() as nursery: nursery.start_soon(erroring) - assert excinfo.value is exc async def test_basic_interleave() -> None: @@ -181,14 +179,14 @@ async def main() -> None: nursery.start_soon(looper) nursery.start_soon(crasher) - with pytest.raises(ValueError, match="^argh$"): + with RaisesGroup(Matcher(ValueError, "^argh$")): _core.run(main) assert looper_record == ["cancelled"] def test_main_and_task_both_crash() -> None: - # If main crashes and there's also a task crash, then we get both in a + # If main crashes and there's also a task crash, then we get both in an # ExceptionGroup async def crasher() -> NoReturn: raise ValueError @@ -219,7 +217,7 @@ async def test_child_crash_wakes_parent() -> None: async def crasher() -> NoReturn: raise ValueError("this is a crash") - with pytest.raises(ValueError, match="^this is a crash$"): # noqa: PT012 + with RaisesGroup(Matcher(ValueError, "^this is a crash$")): async with _core.open_nursery() as nursery: nursery.start_soon(crasher) await sleep_forever() @@ -428,8 +426,8 @@ async def crasher() -> NoReturn: # This is outside the outer scope, so all the Cancelled # exceptions should have been absorbed, leaving just a regular - # KeyError from crasher() - with pytest.raises(KeyError): # noqa: PT012 + # KeyError from crasher(), wrapped in an ExceptionGroup + with RaisesGroup(KeyError): with _core.CancelScope() as outer: # Since the outer scope became cancelled before the # nursery block exited, all cancellations inside the @@ -940,10 +938,16 @@ async def main() -> None: _core.spawn_system_task(system_task) await sleep_forever() + # TrioInternalError is not wrapped with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - assert RaisesGroup(KeyError, ValueError).matches(excinfo.value.__cause__) + # the first exceptiongroup is from the first nursery opened in Runner.init() + # the second exceptiongroup is from the second nursery opened in Runner.init() + # the third exceptongroup is from the nursery defined in `system_task` above + assert RaisesGroup(RaisesGroup(RaisesGroup(KeyError, ValueError))).matches( + excinfo.value.__cause__ + ) def test_system_task_crash_plus_Cancelled() -> None: @@ -969,7 +973,11 @@ async def main() -> None: with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - assert type(excinfo.value.__cause__) is ValueError + + # See explanation for triple-wrap in test_system_task_crash_ExceptionGroup + assert RaisesGroup(RaisesGroup(RaisesGroup(ValueError))).matches( + excinfo.value.__cause__ + ) def test_system_task_crash_KeyboardInterrupt() -> None: @@ -982,7 +990,8 @@ async def main() -> None: with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - assert isinstance(excinfo.value.__cause__, KeyboardInterrupt) + # "Only" double-wrapped since ki() doesn't create an exceptiongroup + assert RaisesGroup(RaisesGroup(KeyboardInterrupt)).matches(excinfo.value.__cause__) # This used to fail because checkpoint was a yield followed by an immediate @@ -1016,7 +1025,7 @@ async def child1() -> None: async with seq(0): pass # we don't yield until seq(2) below record.append("child1 raise") - with pytest.raises(ValueError, match="^child1$") as excinfo: # noqa: PT012 + with pytest.raises(ValueError, match="^child1$") as excinfo: try: raise ValueError("child1") except ValueError: @@ -1035,7 +1044,7 @@ async def child2() -> None: assert "child1 sleep" in record record.append("child2 wake") assert sys.exc_info() == (None, None, None) - with pytest.raises(KeyError) as excinfo: # noqa: PT012 + with pytest.raises(KeyError) as excinfo: try: raise KeyError("child2") except KeyError: @@ -1090,14 +1099,12 @@ async def child() -> None: await sleep_forever() raise - with pytest.raises(KeyError) as excinfo: # noqa: PT012 + with RaisesGroup(Matcher(KeyError, check=lambda e: e.__context__ is None)): async with _core.open_nursery() as nursery: nursery.start_soon(child) await wait_all_tasks_blocked() _core.reschedule(not_none(child_task), outcome.Error(ValueError())) - assert excinfo.value.__context__ is None - # Similar to previous test -- if the `ValueError()` gets sent in via 'throw' and # propagates out, then CPython doesn't set its `__context__` so normal implicit @@ -1118,7 +1125,9 @@ async def child() -> None: except KeyError: await sleep_forever() - with pytest.raises(ValueError, match="^error text$") as excinfo: # noqa: PT012 + with RaisesGroup( + Matcher(ValueError, "error text", lambda e: isinstance(e.__context__, KeyError)) + ): async with _core.open_nursery() as nursery: nursery.start_soon(child) await wait_all_tasks_blocked() @@ -1126,8 +1135,6 @@ async def child() -> None: not_none(child_task), outcome.Error(ValueError("error text")) ) - assert isinstance(excinfo.value.__context__, KeyError) - # Similar to previous tests -- if the `ValueError()` gets sent into an inner # `await` via 'throw' and is suppressed there, then CPython loses track of @@ -1152,14 +1159,12 @@ async def inner(exc: BaseException) -> None: await sleep_forever() assert not_none(sys.exc_info()[1]) is exc - with pytest.raises(KeyError) as excinfo: # noqa: PT012 + with RaisesGroup(Matcher(KeyError, check=lambda e: e.__context__ is None)): async with _core.open_nursery() as nursery: nursery.start_soon(child) await wait_all_tasks_blocked() _core.reschedule(not_none(child_task), outcome.Error(ValueError())) - assert excinfo.value.__context__ is None - # Similar to previous tests -- if the `ValueError()` gets sent into an inner # `await` via `throw` and propagates out, then CPython incorrectly sets its @@ -1184,7 +1189,14 @@ async def inner() -> None: except IndexError: await sleep_forever() - with pytest.raises(ValueError, match="^Unique Text$") as excinfo: # noqa: PT012 + with RaisesGroup( + Matcher( + ValueError, + "^Unique Text$", + lambda e: isinstance(e.__context__, IndexError) + and isinstance(e.__context__.__context__, KeyError), + ) + ): async with _core.open_nursery() as nursery: nursery.start_soon(child) await wait_all_tasks_blocked() @@ -1192,9 +1204,6 @@ async def inner() -> None: not_none(child_task), outcome.Error(ValueError("Unique Text")) ) - assert isinstance(excinfo.value.__context__, IndexError) - assert isinstance(excinfo.value.__context__.__context__, KeyError) - async def test_nursery_exception_chaining_doesnt_make_context_loops() -> None: async def crasher() -> NoReturn: @@ -1329,8 +1338,9 @@ async def main() -> None: with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - - assert type(excinfo.value.__cause__) is KeyError + # the first exceptiongroup is from the first nursery opened in Runner.init() + # the second exceptiongroup is from the second nursery opened in Runner.init() + assert RaisesGroup(RaisesGroup(KeyError)).matches(excinfo.value.__cause__) assert record == {"2nd run_sync_soon ran", "cancelled!"} @@ -1444,7 +1454,7 @@ async def main() -> None: with pytest.raises(_core.TrioInternalError) as excinfo: _core.run(main) - assert type(excinfo.value.__cause__) is KeyError + assert RaisesGroup(KeyError).matches(excinfo.value.__cause__) assert record == ["main exiting", "2nd ran"] @@ -1655,24 +1665,34 @@ async def main() -> None: _core.run(main) - for bad_call in bad_call_run, bad_call_spawn: + async def f() -> None: # pragma: no cover + pass - async def f() -> None: # pragma: no cover - pass + async def async_gen(arg: T) -> AsyncGenerator[T, None]: # pragma: no cover + yield arg - with pytest.raises( - TypeError, - match="^Trio was expecting an async function, but instead it got a coroutine object <.*>", - ): - bad_call(f()) # type: ignore[arg-type] + # If/when RaisesGroup/Matcher is added to pytest in some form this test can be + # rewritten to use a loop again, and avoid specifying the exceptions twice in + # different ways + with pytest.raises( + TypeError, + match="^Trio was expecting an async function, but instead it got a coroutine object <.*>", + ): + bad_call_run(f()) # type: ignore[arg-type] + with pytest.raises( + TypeError, match="expected an async function but got an async generator" + ): + bad_call_run(async_gen, 0) # type: ignore - async def async_gen(arg: T) -> AsyncGenerator[T, None]: # pragma: no cover - yield arg + # bad_call_spawn calls the function inside a nursery, so the exception will be + # wrapped in an exceptiongroup + with RaisesGroup(Matcher(TypeError, "expecting an async function")): + bad_call_spawn(f()) # type: ignore[arg-type] - with pytest.raises( - TypeError, match="expected an async function but got an async generator" - ): - bad_call(async_gen, 0) # type: ignore + with RaisesGroup( + Matcher(TypeError, "expected an async function but got an async generator") + ): + bad_call_spawn(async_gen, 0) # type: ignore def test_calling_asyncio_function_gives_nice_error() -> None: @@ -1693,7 +1713,7 @@ async def misguided() -> None: async def test_asyncio_function_inside_nursery_does_not_explode() -> None: # Regression test for https://github.com/python-trio/trio/issues/552 - with pytest.raises(TypeError, match="asyncio"): # noqa: PT012 + with RaisesGroup(Matcher(TypeError, "asyncio")): async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) await create_asyncio_future_in_new_loop() @@ -1733,7 +1753,7 @@ async def noop_with_no_checkpoint() -> None: with _core.CancelScope() as cancel_scope: cancel_scope.cancel() - with pytest.raises(KeyError): + with RaisesGroup(KeyError): async with _core.open_nursery(): raise KeyError @@ -1867,7 +1887,7 @@ async def test_task_nursery_stack() -> None: assert task._child_nurseries == [] async with _core.open_nursery() as nursery1: assert task._child_nurseries == [nursery1] - with pytest.raises(KeyError): # noqa: PT012 + with RaisesGroup(KeyError): async with _core.open_nursery() as nursery2: assert task._child_nurseries == [nursery1, nursery2] raise KeyError @@ -1960,7 +1980,7 @@ async def start_sleep_then_crash(nursery: _core.Nursery) -> None: async def test_nursery_explicit_exception() -> None: - with pytest.raises(KeyError): + with RaisesGroup(KeyError): async with _core.open_nursery(): raise KeyError() @@ -2005,9 +2025,22 @@ async def __anext__(self) -> list[int]: nexts = self.nexts items: list[int] = [-1] * len(nexts) - async with _core.open_nursery() as nursery: - for i, f in enumerate(nexts): - nursery.start_soon(self._accumulate, f, items, i) + try: + async with _core.open_nursery() as nursery: + for i, f in enumerate(nexts): + nursery.start_soon(self._accumulate, f, items, i) + except ExceptionGroup as e: + # With strict_exception_groups enabled, users now need to unwrap + # StopAsyncIteration and re-raise it. + # This would be relatively clean on python3.11+ with except*. + # We could also use RaisesGroup, but that's primarily meant as + # test infra, not as a runtime tool. + if len(e.exceptions) == 1 and isinstance( + e.exceptions[0], StopAsyncIteration + ): + raise e.exceptions[0] from None + else: # pragma: no cover + raise AssertionError("unknown error in _accumulate") from e return items @@ -2232,7 +2265,7 @@ async def detachable_coroutine( # Check the exception paths too task = None pdco_outcome = None - with pytest.raises(KeyError): + with RaisesGroup(KeyError): async with _core.open_nursery() as nursery: nursery.start_soon(detachable_coroutine, outcome.Error(KeyError()), "uh oh") throw_in = ValueError() @@ -2404,7 +2437,7 @@ async def crasher() -> NoReturn: # (See https://github.com/python-trio/trio/pull/1864) await do_a_cancel() - with pytest.raises(ValueError, match="^this is a crash$"): + with RaisesGroup(Matcher(ValueError, "^this is a crash$")): async with _core.open_nursery() as nursery: # cover NurseryManager.__aexit__ nursery.start_soon(crasher) @@ -2428,8 +2461,8 @@ async def crasher() -> NoReturn: old_flags = gc.get_debug() try: - with pytest.raises( # noqa: PT012 - ValueError, match="^this is a crash$" + with RaisesGroup( + Matcher(ValueError, "^this is a crash$") ), _core.CancelScope() as outer: async with _core.open_nursery() as nursery: gc.collect() @@ -2510,98 +2543,74 @@ async def task() -> None: assert destroyed -def test_run_strict_exception_groups() -> None: - """ - Test that nurseries respect the global context setting of strict_exception_groups. - """ +def _create_kwargs(strictness: bool | None) -> dict[str, bool]: + """Turn a bool|None into a kwarg dict that can be passed to `run` or `open_nursery`""" - async def main() -> NoReturn: - async with _core.open_nursery(): - raise Exception("foo") + if strictness is None: + return {} + return {"strict_exception_groups": strictness} - with RaisesGroup( - Matcher(Exception, match="^foo$"), - match="^Exceptions from Trio nursery \\(1 sub-exception\\)$", - ): - _core.run(main, strict_exception_groups=True) - -def test_run_strict_exception_groups_nursery_override() -> None: +@pytest.mark.parametrize("run_strict", [True, False, None]) +@pytest.mark.parametrize("open_nursery_strict", [True, False, None]) +@pytest.mark.parametrize("multiple_exceptions", [True, False]) +def test_setting_strict_exception_groups( + run_strict: bool | None, open_nursery_strict: bool | None, multiple_exceptions: bool +) -> None: """ - Test that a nursery can override the global context setting of - strict_exception_groups. + Test default values and that nurseries can both inherit and override the global context + setting of strict_exception_groups. """ - async def main() -> NoReturn: - async with _core.open_nursery(strict_exception_groups=False): - raise Exception("foo") - - with pytest.raises(Exception, match="^foo$"): - _core.run(main, strict_exception_groups=True) - - -async def test_nursery_strict_exception_groups() -> None: - """Test that strict exception groups can be enabled on a per-nursery basis.""" - with RaisesGroup(Matcher(Exception, match="^foo$")): - async with _core.open_nursery(strict_exception_groups=True): - raise Exception("foo") - - -async def test_nursery_loose_exception_groups() -> None: - """Test that loose exception groups can be enabled on a per-nursery basis.""" - async def raise_error() -> NoReturn: raise RuntimeError("test error") - with pytest.raises(RuntimeError, match="^test error$"): - async with _core.open_nursery(strict_exception_groups=False) as nursery: + async def main() -> None: + """Open a nursery, and raise one or two errors inside""" + async with _core.open_nursery(**_create_kwargs(open_nursery_strict)) as nursery: nursery.start_soon(raise_error) - m = Matcher(RuntimeError, match="^test error$") + if multiple_exceptions: + nursery.start_soon(raise_error) - with RaisesGroup( - m, - m, - match="Exceptions from Trio nursery \\(2 sub-exceptions\\)", - check=lambda x: x.__notes__ == [_core._run.NONSTRICT_EXCEPTIONGROUP_NOTE], - ): - async with _core.open_nursery(strict_exception_groups=False) as nursery: - nursery.start_soon(raise_error) - nursery.start_soon(raise_error) + def run_main() -> None: + # mypy doesn't like kwarg magic + _core.run(main, **_create_kwargs(run_strict)) # type: ignore[arg-type] + matcher = Matcher(RuntimeError, "^test error$") -async def test_nursery_collapse_strict() -> None: - """ - Test that a single exception from a nested nursery with strict semantics doesn't get - collapsed when CancelledErrors are stripped from it. - """ - - async def raise_error() -> NoReturn: - raise RuntimeError("test error") - - with RaisesGroup(RuntimeError, RaisesGroup(RuntimeError)): - async with _core.open_nursery() as nursery: - nursery.start_soon(sleep_forever) - nursery.start_soon(raise_error) - async with _core.open_nursery(strict_exception_groups=True) as nursery2: - nursery2.start_soon(sleep_forever) - nursery2.start_soon(raise_error) - nursery.cancel_scope.cancel() + if multiple_exceptions: + with RaisesGroup(matcher, matcher): + run_main() + elif open_nursery_strict or ( + open_nursery_strict is None and run_strict is not False + ): + with RaisesGroup(matcher): + run_main() + else: + with pytest.raises(RuntimeError, match="^test error$"): + run_main() -async def test_nursery_collapse_loose() -> None: +@pytest.mark.parametrize("strict", [True, False, None]) +async def test_nursery_collapse(strict: bool | None) -> None: """ - Test that a single exception from a nested nursery with loose semantics gets - collapsed when CancelledErrors are stripped from it. + Test that a single exception from a nested nursery gets collapsed correctly + depending on strict_exception_groups value when CancelledErrors are stripped from it. """ async def raise_error() -> NoReturn: raise RuntimeError("test error") - with RaisesGroup(RuntimeError, RuntimeError): + # mypy requires explicit type for conditional expression + maybe_wrapped_runtime_error: type[RuntimeError] | RaisesGroup[RuntimeError] = ( + RuntimeError if strict is False else RaisesGroup(RuntimeError) + ) + + with RaisesGroup(RuntimeError, maybe_wrapped_runtime_error): async with _core.open_nursery() as nursery: nursery.start_soon(sleep_forever) nursery.start_soon(raise_error) - async with _core.open_nursery() as nursery2: + async with _core.open_nursery(**_create_kwargs(strict)) as nursery2: nursery2.start_soon(sleep_forever) nursery2.start_soon(raise_error) nursery.cancel_scope.cancel() diff --git a/src/trio/_subprocess.py b/src/trio/_subprocess.py index 37513380f3..f81f94964e 100644 --- a/src/trio/_subprocess.py +++ b/src/trio/_subprocess.py @@ -652,6 +652,7 @@ async def my_deliver_cancel(process): and the process exits with a nonzero exit status OSError: if an error is encountered starting or communicating with the process + ExceptionGroup: if exceptions occur in ``deliver_cancel``, or when exceptions occur when communicating with the subprocess. If strict_exception_groups is set to false in the global context, then single exceptions will be collapsed. .. note:: The child process runs in the same process group as the parent Trio process, so a Ctrl+C will be delivered simultaneously to both @@ -725,9 +726,11 @@ async def read_output( async for chunk in stream: chunks.append(chunk) + # Opening the process does not need to be inside the nursery, so we put it outside + # so any exceptions get directly seen by users. + # options needs a complex TypedDict. The overload error only occurs on Unix. + proc = await open_process(command, **options) # type: ignore[arg-type, call-overload, unused-ignore] async with trio.open_nursery() as nursery: - # options needs a complex TypedDict. The overload error only occurs on Unix. - proc = await open_process(command, **options) # type: ignore[arg-type, call-overload, unused-ignore] try: if input is not None: assert proc.stdin is not None diff --git a/src/trio/_tests/test_highlevel_open_tcp_stream.py b/src/trio/_tests/test_highlevel_open_tcp_stream.py index 5f738ba4cc..c7d46253e8 100644 --- a/src/trio/_tests/test_highlevel_open_tcp_stream.py +++ b/src/trio/_tests/test_highlevel_open_tcp_stream.py @@ -42,14 +42,14 @@ def close(self) -> None: assert c.closed c = CloseMe() - with pytest.raises(RuntimeError): # noqa: PT012 + with pytest.raises(RuntimeError): with close_all() as to_close: to_close.add(c) raise RuntimeError assert c.closed c = CloseMe() - with pytest.raises(OSError, match="os error text"): # noqa: PT012 + with pytest.raises(OSError, match="os error text"): with close_all() as to_close: to_close.add(CloseKiller()) to_close.add(c) diff --git a/src/trio/_tests/test_highlevel_serve_listeners.py b/src/trio/_tests/test_highlevel_serve_listeners.py index c0ded5e28f..a14f6a2e9c 100644 --- a/src/trio/_tests/test_highlevel_serve_listeners.py +++ b/src/trio/_tests/test_highlevel_serve_listeners.py @@ -5,19 +5,22 @@ from typing import TYPE_CHECKING, Awaitable, Callable, NoReturn import attr -import pytest import trio from trio import Nursery, StapledStream, TaskStatus from trio.testing import ( + Matcher, MemoryReceiveStream, MemorySendStream, MockClock, + RaisesGroup, memory_stream_pair, wait_all_tasks_blocked, ) if TYPE_CHECKING: + import pytest + from trio._channel import MemoryReceiveChannel, MemorySendChannel from trio.abc import Stream @@ -110,11 +113,13 @@ async def test_serve_listeners_accept_unrecognized_error() -> None: async def raise_error() -> NoReturn: raise error # noqa: B023 # Set from loop + def check_error(e: BaseException) -> bool: + return e is error # noqa: B023 + listener.accept_hook = raise_error - with pytest.raises(type(error)) as excinfo: + with RaisesGroup(Matcher(check=check_error)): await trio.serve_listeners(None, [listener]) # type: ignore[arg-type] - assert excinfo.value is error async def test_serve_listeners_accept_capacity_error( @@ -158,7 +163,8 @@ async def connection_watcher( assert len(nursery.child_tasks) == 10 raise Done - with pytest.raises(Done): # noqa: PT012 + # the exception is wrapped twice because we open two nested nurseries + with RaisesGroup(RaisesGroup(Done)): async with trio.open_nursery() as nursery: handler_nursery: trio.Nursery = await nursery.start(connection_watcher) await nursery.start( diff --git a/src/trio/_tests/test_signals.py b/src/trio/_tests/test_signals.py index 0d38168d57..5e639652ef 100644 --- a/src/trio/_tests/test_signals.py +++ b/src/trio/_tests/test_signals.py @@ -6,6 +6,7 @@ import pytest import trio +from trio.testing import RaisesGroup from .. import _core from .._signals import _signal_handler, get_pending_signal_count, open_signal_receiver @@ -74,7 +75,7 @@ async def naughty() -> None: async def test_open_signal_receiver_conflict() -> None: - with pytest.raises(trio.BusyResourceError): # noqa: PT012 + with RaisesGroup(trio.BusyResourceError): with open_signal_receiver(signal.SIGILL) as receiver: async with trio.open_nursery() as nursery: nursery.start_soon(receiver.__anext__) @@ -173,7 +174,7 @@ def raise_handler(signum: int, frame: FrameType | None) -> NoReturn: raise RuntimeError(signum) with _signal_handler({signal.SIGILL, signal.SIGFPE}, raise_handler): - with pytest.raises(RuntimeError) as excinfo: # noqa: PT012 + with pytest.raises(RuntimeError) as excinfo: with open_signal_receiver(signal.SIGILL, signal.SIGFPE) as receiver: signal_raise(signal.SIGILL) signal_raise(signal.SIGFPE) diff --git a/src/trio/_tests/test_socket.py b/src/trio/_tests/test_socket.py index 1a55647437..bce3971b35 100644 --- a/src/trio/_tests/test_socket.py +++ b/src/trio/_tests/test_socket.py @@ -653,7 +653,7 @@ async def res( await res("1.2.3.4") # type: ignore[arg-type] with pytest.raises(ValueError, match=address): await res(("1.2.3.4",)) # type: ignore[arg-type] - with pytest.raises( # noqa: PT012 + with pytest.raises( ValueError, match=address, ): diff --git a/src/trio/_tests/test_ssl.py b/src/trio/_tests/test_ssl.py index d8ce80df47..5ad23a4df1 100644 --- a/src/trio/_tests/test_ssl.py +++ b/src/trio/_tests/test_ssl.py @@ -23,7 +23,12 @@ from trio import StapledStream from trio._tests.pytest_plugin import skip_if_optional_else_raise from trio.abc import ReceiveStream, SendStream -from trio.testing import MemoryReceiveStream, MemorySendStream +from trio.testing import ( + Matcher, + MemoryReceiveStream, + MemorySendStream, + RaisesGroup, +) try: import trustme @@ -356,9 +361,7 @@ async def do_test( func1: str, args1: tuple[object, ...], func2: str, args2: tuple[object, ...] ) -> None: s = PyOpenSSLEchoStream() - with pytest.raises( # noqa: PT012 - _core.BusyResourceError, match="simultaneous" - ): + with RaisesGroup(Matcher(_core.BusyResourceError, "simultaneous")): async with _core.open_nursery() as nursery: nursery.start_soon(getattr(s, func1), *args1) nursery.start_soon(getattr(s, func2), *args2) @@ -745,9 +748,7 @@ async def do_test( func1: Callable[[S], Awaitable[None]], func2: Callable[[S], Awaitable[None]] ) -> None: s, _ = ssl_lockstep_stream_pair(client_ctx) - with pytest.raises( # noqa: PT012 - _core.BusyResourceError, match="another task" - ): + with RaisesGroup(Matcher(_core.BusyResourceError, "another task")): async with _core.open_nursery() as nursery: nursery.start_soon(func1, s) nursery.start_soon(func2, s) diff --git a/src/trio/_tests/test_subprocess.py b/src/trio/_tests/test_subprocess.py index 976180ab22..a4f07189e4 100644 --- a/src/trio/_tests/test_subprocess.py +++ b/src/trio/_tests/test_subprocess.py @@ -21,6 +21,9 @@ import pytest +import trio +from trio.testing import Matcher, RaisesGroup + from .. import ( Event, Process, @@ -562,6 +565,27 @@ async def custom_deliver_cancel(proc: Process) -> None: assert custom_deliver_cancel_called +def test_bad_deliver_cancel() -> None: + async def custom_deliver_cancel(proc: Process) -> None: + proc.terminate() + raise ValueError("foo") + + async def do_stuff() -> None: + async with _core.open_nursery() as nursery: + nursery.start_soon( + partial(run_process, SLEEP(9999), deliver_cancel=custom_deliver_cancel) + ) + await wait_all_tasks_blocked() + nursery.cancel_scope.cancel() + + # double wrap from our nursery + the internal nursery + with RaisesGroup(RaisesGroup(Matcher(ValueError, "^foo$"))): + _core.run(do_stuff, strict_exception_groups=True) + + with pytest.raises(ValueError, match="^foo$"): + _core.run(do_stuff, strict_exception_groups=False) + + async def test_warn_on_failed_cancel_terminate(monkeypatch: pytest.MonkeyPatch) -> None: original_terminate = Process.terminate @@ -594,7 +618,7 @@ async def test_warn_on_cancel_SIGKILL_escalation( # the background_process_param exercises a lot of run_process cases, but it uses # check=False, so lets have a test that uses check=True as well async def test_run_process_background_fail() -> None: - with pytest.raises(subprocess.CalledProcessError): + with RaisesGroup(subprocess.CalledProcessError): async with _core.open_nursery() as nursery: proc: Process = await nursery.start(run_process, EXIT_FALSE) assert proc.returncode == 1 @@ -620,6 +644,17 @@ async def test_for_leaking_fds() -> None: assert set(SyncPath("/dev/fd").iterdir()) == starting_fds +async def test_run_process_internal_error(monkeypatch: pytest.MonkeyPatch) -> None: + # There's probably less extreme ways of triggering errors inside the nursery + # in run_process. + async def very_broken_open(*args: object, **kwargs: object) -> str: + return "oops" + + monkeypatch.setattr(trio._subprocess, "open_process", very_broken_open) + with RaisesGroup(AttributeError, AttributeError): + await run_process(EXIT_TRUE, capture_stdout=True) + + # regression test for #2209 async def test_subprocess_pidfd_unnotified() -> None: noticed_exit = None diff --git a/src/trio/_tests/test_testing.py b/src/trio/_tests/test_testing.py index 75be7a6aa2..9a8c4b87e3 100644 --- a/src/trio/_tests/test_testing.py +++ b/src/trio/_tests/test_testing.py @@ -6,6 +6,8 @@ import pytest +from trio.testing import RaisesGroup + from .. import _core, sleep, socket as tsocket from .._core._tests.tutil import can_bind_ipv6 from .._highlevel_generic import StapledStream, aclose_forcefully @@ -157,7 +159,7 @@ async def test_assert_no_checkpoints(recwarn: pytest.WarningsRecorder) -> None: await partial_yield() # And both together also count as a checkpoint - with pytest.raises(AssertionError): # noqa: PT012 + with pytest.raises(AssertionError): with assert_no_checkpoints(): await _core.checkpoint_if_cancelled() await _core.cancel_shielded_checkpoint() @@ -234,8 +236,6 @@ async def child(i: int) -> None: ################################################################ - - async def test__assert_raises() -> None: with pytest.raises(AssertionError): with _assert_raises(RuntimeError): @@ -292,7 +292,7 @@ async def getter(expect: bytes) -> None: nursery.start_soon(putter, b"xyz") # Two gets at the same time -> BusyResourceError - with pytest.raises(_core.BusyResourceError): # noqa: PT012 + with RaisesGroup(_core.BusyResourceError): async with _core.open_nursery() as nursery: nursery.start_soon(getter, b"asdf") nursery.start_soon(getter, b"asdf") @@ -426,7 +426,7 @@ async def do_receive_some(max_bytes: int | None) -> bytes: mrs.put_data(b"abc") assert await do_receive_some(None) == b"abc" - with pytest.raises(_core.BusyResourceError): # noqa: PT012 + with RaisesGroup(_core.BusyResourceError): async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some, 10) nursery.start_soon(do_receive_some, 10) diff --git a/src/trio/_tests/test_util.py b/src/trio/_tests/test_util.py index dd44765ba1..e5422770cb 100644 --- a/src/trio/_tests/test_util.py +++ b/src/trio/_tests/test_util.py @@ -6,6 +6,7 @@ import pytest import trio +from trio.testing import Matcher, RaisesGroup from .. import _core from .._core._tests.tutil import ( @@ -49,7 +50,7 @@ async def test_ConflictDetector() -> None: with ul2: print("ok") - with pytest.raises(_core.BusyResourceError, match="ul1"): # noqa: PT012 + with pytest.raises(_core.BusyResourceError, match="ul1"): with ul1: with ul1: pass # pragma: no cover @@ -58,7 +59,7 @@ async def wait_with_ul1() -> None: with ul1: await wait_all_tasks_blocked() - with pytest.raises(_core.BusyResourceError, match="ul1"): # noqa: PT012 + with RaisesGroup(Matcher(_core.BusyResourceError, "ul1")): async with _core.open_nursery() as nursery: nursery.start_soon(wait_with_ul1) nursery.start_soon(wait_with_ul1) diff --git a/src/trio/_tests/test_windows_pipes.py b/src/trio/_tests/test_windows_pipes.py index 0f968e26ce..38a25cdc54 100644 --- a/src/trio/_tests/test_windows_pipes.py +++ b/src/trio/_tests/test_windows_pipes.py @@ -96,7 +96,7 @@ async def test_close_during_write() -> None: async with _core.open_nursery() as nursery: async def write_forever() -> None: - with pytest.raises(_core.ClosedResourceError) as excinfo: # noqa: PT012 + with pytest.raises(_core.ClosedResourceError) as excinfo: while True: await w.send_all(b"x" * 4096) assert "another task" in str(excinfo.value) diff --git a/src/trio/testing/_check_streams.py b/src/trio/testing/_check_streams.py index d50e8d864d..c54c99c1fe 100644 --- a/src/trio/testing/_check_streams.py +++ b/src/trio/testing/_check_streams.py @@ -2,8 +2,17 @@ from __future__ import annotations import random +import sys from contextlib import contextmanager, suppress -from typing import TYPE_CHECKING, Awaitable, Callable, Generic, Tuple, TypeVar +from typing import ( + TYPE_CHECKING, + Awaitable, + Callable, + Generator, + Generic, + Tuple, + TypeVar, +) from .. import CancelScope, _core from .._abc import AsyncResource, HalfCloseableStream, ReceiveStream, SendStream, Stream @@ -11,13 +20,15 @@ from ._checkpoints import assert_checkpoints if TYPE_CHECKING: - from collections.abc import Generator from types import TracebackType from typing_extensions import ParamSpec, TypeAlias ArgsT = ParamSpec("ArgsT") +if sys.version_info < (3, 11): + from exceptiongroup import BaseExceptionGroup + Res1 = TypeVar("Res1", bound=AsyncResource) Res2 = TypeVar("Res2", bound=AsyncResource) StreamMaker: TypeAlias = Callable[[], Awaitable[Tuple[Res1, Res2]]] @@ -45,14 +56,21 @@ async def __aexit__( # This is used in this file instead of pytest.raises in order to avoid a dependency # on pytest, as the check_* functions are publicly exported. @contextmanager -def _assert_raises(exc: type[BaseException]) -> Generator[None, None, None]: +def _assert_raises( + expected_exc: type[BaseException], wrapped: bool = False +) -> Generator[None, None, None]: __tracebackhide__ = True try: yield - except exc: - pass + except BaseExceptionGroup as exc: + assert wrapped, "caught exceptiongroup, but expected an unwrapped exception" + # assert in except block ignored below + assert len(exc.exceptions) == 1 # noqa: PT017 + assert isinstance(exc.exceptions[0], expected_exc) # noqa: PT017 + except expected_exc: + assert not wrapped, "caught exception, but expected an exceptiongroup" else: - raise AssertionError(f"expected exception: {exc}") + raise AssertionError(f"expected exception: {expected_exc}") async def check_one_way_stream( @@ -137,7 +155,7 @@ async def send_empty_then_y() -> None: nursery.start_soon(do_send_all, b"x") assert await do_receive_some(None) == b"x" - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(do_receive_some, 1) nursery.start_soon(do_receive_some, 1) @@ -335,7 +353,7 @@ async def receiver() -> None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s, r): # simultaneous wait_send_all_might_not_block fails - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(s.wait_send_all_might_not_block) nursery.start_soon(s.wait_send_all_might_not_block) @@ -344,7 +362,7 @@ async def receiver() -> None: # this test might destroy the stream b/c we end up cancelling # send_all and e.g. SSLStream can't handle that, so we have to # recreate afterwards) - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(s.wait_send_all_might_not_block) nursery.start_soon(s.send_all, b"123") @@ -352,7 +370,7 @@ async def receiver() -> None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s, r): # send_all and send_all blocked simultaneously should also raise # (but again this might destroy the stream) - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(s.send_all, b"123") nursery.start_soon(s.send_all, b"123") @@ -534,7 +552,7 @@ async def expect_x_then_eof(r: HalfCloseableStream) -> None: if clogged_stream_maker is not None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # send_all and send_eof simultaneously is not ok - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(s1.send_all, b"x") await _core.wait_all_tasks_blocked() @@ -543,7 +561,7 @@ async def expect_x_then_eof(r: HalfCloseableStream) -> None: async with _ForceCloseBoth(await clogged_stream_maker()) as (s1, s2): # wait_send_all_might_not_block and send_eof simultaneously is not # ok either - with _assert_raises(_core.BusyResourceError): + with _assert_raises(_core.BusyResourceError, wrapped=True): async with _core.open_nursery() as nursery: nursery.start_soon(s1.wait_send_all_might_not_block) await _core.wait_all_tasks_blocked()