Skip to content

Commit

Permalink
Don't double-wrap exceptions in nurseries (#2826)
Browse files Browse the repository at this point in the history
* add test

* Fix unnecessary wrapping of exception groups

* add test where child task raises an exception group

* refactor tests

* move tests into _core/_tests/test_run.py

* Update trio/_core/_tests/test_run.py

Co-authored-by: Thomas Grainger <tagrain@gmail.com>

---------

Co-authored-by: CoolCat467 <52022020+CoolCat467@users.noreply.github.com>
Co-authored-by: Thomas Grainger <tagrain@gmail.com>
  • Loading branch information
3 people authored Oct 26, 2023
1 parent b472f30 commit c9e01f4
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 1 deletion.
7 changes: 7 additions & 0 deletions trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,13 @@ def aborted(raise_cancel: _core.RaiseCancelT) -> Abort:

popped = self._parent_task._child_nurseries.pop()
assert popped is self

# don't unnecessarily wrap an exceptiongroup in another exceptiongroup
# see https://github.com/python-trio/trio/issues/2611
if len(self._pending_excs) == 1 and isinstance(
self._pending_excs[0], BaseExceptionGroup
):
return self._pending_excs[0]
if self._pending_excs:
try:
return MultiError(
Expand Down
61 changes: 60 additions & 1 deletion trio/_core/_tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
)

if sys.version_info < (3, 11):
from exceptiongroup import ExceptionGroup
from exceptiongroup import BaseExceptionGroup, ExceptionGroup


T = TypeVar("T")
Expand Down Expand Up @@ -2538,3 +2538,62 @@ async def test_cancel_scope_no_cancellederror() -> None:
raise ExceptionGroup("test", [RuntimeError(), RuntimeError()])

assert not scope.cancelled_caught


"""These tests are for fixing https://github.com/python-trio/trio/issues/2611
where exceptions raised before `task_status.started()` got wrapped twice.
"""


async def raise_before(*, task_status: _core.TaskStatus[None]) -> None:
raise ValueError


async def raise_after_started(*, task_status: _core.TaskStatus[None]) -> None:
task_status.started()
raise ValueError


async def raise_custom_exception_group_before(
*, task_status: _core.TaskStatus[None]
) -> None:
raise ExceptionGroup("my group", [ValueError()])


def _check_exception(exc: pytest.ExceptionInfo[BaseException]) -> None:
assert isinstance(exc.value, BaseExceptionGroup)
assert len(exc.value.exceptions) == 1
assert isinstance(exc.value.exceptions[0], ValueError)


async def _start_raiser(
raiser: Callable[[], Awaitable[None]], strict: bool | None = None
) -> None:
async with _core.open_nursery(strict_exception_groups=strict) as nursery:
await nursery.start(raiser)


@pytest.mark.parametrize("strict", [False, True])
@pytest.mark.parametrize("raiser", [raise_before, raise_after_started])
async def test_strict_before_started(
strict: bool, raiser: Callable[[], Awaitable[None]]
) -> None:
with pytest.raises(BaseExceptionGroup if strict else ValueError) as exc:
await _start_raiser(raiser, strict)
if strict:
_check_exception(exc)


# it was only when run from `trio.run` that the double wrapping happened
@pytest.mark.parametrize("strict", [False, True])
@pytest.mark.parametrize(
"raiser", [raise_before, raise_after_started, raise_custom_exception_group_before]
)
def test_trio_run_strict_before_started(
strict: bool, raiser: Callable[[], Awaitable[None]]
) -> None:
expect_group = strict or raiser is raise_custom_exception_group_before
with pytest.raises(BaseExceptionGroup if expect_group else ValueError) as exc:
_core.run(_start_raiser, raiser, strict_exception_groups=strict)
if expect_group:
_check_exception(exc)

0 comments on commit c9e01f4

Please sign in to comment.