Skip to content

Commit

Permalink
Merge pull request #2886 from jakkdl/remove_multierror
Browse files Browse the repository at this point in the history
change strict_exception_groups default to True
  • Loading branch information
Zac-HD authored Jan 22, 2024
2 parents d3bb19b + 0b45ec9 commit e0d8edf
Show file tree
Hide file tree
Showing 17 changed files with 277 additions and 178 deletions.
12 changes: 9 additions & 3 deletions docs/source/reference-core.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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/

Expand Down
10 changes: 10 additions & 0 deletions newsfragments/2786.breaking.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
The :ref:`strict_exception_groups <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 <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 <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 <ExceptionGroup>`
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
20 changes: 11 additions & 9 deletions src/trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 8 additions & 2 deletions src/trio/_core/_tests/test_ki.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"}

Expand All @@ -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"}

Expand All @@ -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)

Expand All @@ -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()])

Expand Down
Loading

0 comments on commit e0d8edf

Please sign in to comment.