Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace usage of strict_exception_groups=False in Nursery.start #2938

Merged
merged 3 commits into from
Jan 31, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 23 additions & 13 deletions src/trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -1231,19 +1231,29 @@ async def async_fn(arg1, arg2, *, task_status=trio.TASK_STATUS_IGNORED):
raise RuntimeError("Nursery is closed to new arrivals")
try:
self._pending_starts += 1
# `strict_exception_groups=False` prevents the implementation-detail
# nursery from inheriting `strict_exception_groups=True` from the
# `run` option, which would cause it to wrap a pre-started()
# exception in an extra ExceptionGroup. See #2611.
async with open_nursery(strict_exception_groups=False) as old_nursery:
task_status: _TaskStatus[Any] = _TaskStatus(old_nursery, self)
thunk = functools.partial(async_fn, task_status=task_status)
task = GLOBAL_RUN_CONTEXT.runner.spawn_impl(
thunk, args, old_nursery, name
)
task._eventual_parent_nursery = self
# Wait for either TaskStatus.started or an exception to
# cancel this nursery:
# wrap internal nursery in try-except to unroll any exceptiongroups
# to avoid wrapping pre-started() exceptions in an extra ExceptionGroup.
# See #2611.
try:
# set strict_exception_groups = True to make sure we always unwrap
# *this* nursery's exceptiongroup
async with open_nursery(strict_exception_groups=True) as old_nursery:
task_status: _TaskStatus[Any] = _TaskStatus(old_nursery, self)
thunk = functools.partial(async_fn, task_status=task_status)
task = GLOBAL_RUN_CONTEXT.runner.spawn_impl(
thunk, args, old_nursery, name
)
task._eventual_parent_nursery = self
# Wait for either TaskStatus.started or an exception to
# cancel this nursery:
except BaseExceptionGroup as exc:
if len(exc.exceptions) == 1:
raise exc.exceptions[0] from None
# TODO: give a deprecationwarning instead?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think making this an internal error immediately is fine - it's possible to do this, but it's never been something we documented or that we think is reasonable to have done for some reason.

raise TrioInternalError(
"internal nursery should not have multiple tasks"
) from exc
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I liked how one of the linked PRs proposed warning in .start_soon for that internal nursery (re: todo)


# If we get here, then the child either got reparented or exited
# normally. The complicated logic is all in TaskStatus.started().
# (Any exceptions propagate directly out of the above.)
Expand Down
16 changes: 16 additions & 0 deletions src/trio/_core/_tests/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -2700,3 +2700,19 @@ async def start_raiser() -> None:
assert type(should_be_raiser_exc) == type(raiser_exc)
assert should_be_raiser_exc.message == raiser_exc.message
assert should_be_raiser_exc.exceptions == raiser_exc.exceptions


async def test_internal_error_old_nursery_multiple_tasks() -> None:
async def error_func() -> None:
raise ValueError

async def spawn_tasks_in_old_nursery(task_status: _core.TaskStatus[None]) -> None:
old_nursery = _core.current_task().parent_nursery
assert old_nursery is not None
old_nursery.start_soon(error_func)
old_nursery.start_soon(error_func)

async with _core.open_nursery() as nursery:
with pytest.raises(_core.TrioInternalError) as excinfo:
await nursery.start(spawn_tasks_in_old_nursery)
assert RaisesGroup(ValueError, ValueError).matches(excinfo.value.__cause__)
Loading