(Regression on master) strict_exception_groups is not respected

[gschaffner]

strict_exception_groups is documented as making the exception structure that comes out of a nursery aexit not depend on whether == 1 or > 1 tasks raised, by having a strict nursery always wrap all exception(s) in a new group:

If [strict_exception_groups is] true, nurseries will always wrap even a single raised exception in an exception group.

but on master this isn't happening:

minimized reproducer 1

import trio


async def eg() -> None:
    async def raiser() -> None:
        raise RuntimeError("boom")

    async def f() -> None:
        async with trio.open_nursery(strict_exception_groups=True) as inner:
            inner.start_soon(raiser)

    async with (
        # A similar bug also occurs if there are more strict nurseries open here:
        # trio.open_nursery(strict_exception_groups=True),
        trio.open_nursery(strict_exception_groups=True) as nursery,
    ):
        n = 2
        # try uncommenting me! it completely changes the exception structure.
        # n = 1
        for _ in range(n):
            nursery.start_soon(f)


trio.run(eg)

expected: the same exception structure with n = 1 and n > 1, i.e. two levels of exception groups (or three if the third nursery is uncommented).

e.g. with n = 1:

  + Exception Group Traceback (most recent call last):
  |   ...
  | trio.NonBaseMultiError: <MultiError: RuntimeError('boom')>
  +-+---------------- 1 ----------------
    | Exception Group Traceback (most recent call last):
    |   ...
    | trio.NonBaseMultiError: RuntimeError('boom')
    +-+---------------- 1 ----------------
      | Traceback (most recent call last):
      |   ...
      | RuntimeError: boom
      +------------------------------------

this is the behavior on v0.22.2.

observed: (w/ trio @ 6c50dc2, CPython 3.11.6.) different exception structure depending on n = 1 vs. n > 1.

e.g. with n = 1:

  + Exception Group Traceback (most recent call last):
  |   ...
  | trio.NonBaseMultiError: RuntimeError('boom')
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   ...
    | RuntimeError: boom
    +------------------------------------

the task's crash is in only one one exception group, even though it occurred two strict nurseries down the stack. (similarly, if you uncomment the extra nursery, it's still only one when it should be three.)

while this new behavior will not break except* usage (because except* recurses), it is still a problem. for example:

minimized reproducer 2

import trio


async def eg() -> None:
    async def raiser() -> None:
        exc_group_from_some_task = ExceptionGroup(
            "some error message", (RuntimeError("boom"),)
        )
        exc_group_from_some_task.add_note("some note")
        raise exc_group_from_some_task

    try:
        async with trio.open_nursery(strict_exception_groups=True) as nursery:
            nursery.start_soon(raiser)
    except BaseExceptionGroup as nursery_exc_group:
        raise BaseExceptionGroup(
            "a useful error message instead of just the (necessarily) vague MultiError repr",
            nursery_exc_group.exceptions,
        ) from None
        # Note: `strict_exception_groups=True` guarantees that the `nursery_exc_group` we caught
        # here is a brand new one that the nursery created, and NOT an exception group
        # that came out of one of `nursery`'s tasks.


trio.run(eg)

expected: two exception groups, and trio doesn't cause your exception group messages/notes to be hidden:

  + Exception Group Traceback (most recent call last):
  |   ...
  | ExceptionGroup: a useful error message instead of just the (necessarily) vague MultiError repr (1 sub-exception)
  +-+---------------- 1 ----------------
    | ...
    | ExceptionGroup: some error message (1 sub-exception)
    | some note
    +-+---------------- 1 ----------------
      | RuntimeError: boom
      +------------------------------------

observed: one exception group, and trio causes some exception group messages/notes to be hidden:

  + Exception Group Traceback (most recent call last):
  |   ...
  | ExceptionGroup: a useful error message instead of just the (necessarily) vague MultiError repr (1 sub-exception)
  +-+---------------- 1 ----------------
    | RuntimeError: boom
    +------------------------------------

details

i encountered this regression while testing a downstream work against trio master in preparation for the upcoming v0.23 release.

this bisects to #2826 (CC @jakkdl). from its newsfragment, tests, and surrounding discussions, my understanding is that #2826's primary intent was to hide the implementation-detail nursery of start (i.e. to resolve #2611)—but that it did not intend to make trio violate its strict_exception_groups docs & also hide third-party exception messages from tracebacks.

[jakkdl]

Thank you for finding it! Will look at the pull request