nursery replaces custom ExceptionGroup subclasses with plain ExceptionGroups

[graingert]

import trio

class MyExceptionGroup(ExceptionGroup):
    pass


async def main():
    async with trio.open_nursery():
        raise MyExceptionGroup("hello", [ValueError("bad")])

trio.run(main)

output:

  + Exception Group Traceback (most recent call last):
  |   File "/home/graingert/projects/demo.py", line 11, in <module>
  |     trio.run(main)
  |   File "/home/graingert/.virtualenvs/anyio_fixture_methods/lib/python3.12/site-packages/trio/_core/_run.py", line 2306, in run
  |     raise runner.main_task_outcome.error
  |   File "/home/graingert/projects/demo.py", line 8, in main
  |     async with trio.open_nursery():
  |   File "/home/graingert/.virtualenvs/anyio_fixture_methods/lib/python3.12/site-packages/trio/_core/_run.py", line 959, in __aexit__
  |     raise combined_error_from_nursery
  | ExceptionGroup: Exceptions from Trio nursery (1 sub-exception)
  +-+---------------- 1 ----------------
    | Exception Group Traceback (most recent call last):
    |   File "/home/graingert/projects/demo.py", line 9, in main
    |     raise MyExceptionGroup("hello", [ValueError("bad")])
    | ExceptionGroup: hello (1 sub-exception)
    +-+---------------- 1 ----------------
      | ValueError: bad
      +------------------------------------

expected output:

  + Exception Group Traceback (most recent call last):
  |   File "/home/graingert/projects/demo.py", line 11, in <module>
  |     trio.run(main)
  |   File "/home/graingert/.virtualenvs/anyio_fixture_methods/lib/python3.12/site-packages/trio/_core/_run.py", line 2306, in run
  |     raise runner.main_task_outcome.error
  |   File "/home/graingert/projects/demo.py", line 8, in main
  |     async with trio.open_nursery():
  |   File "/home/graingert/.virtualenvs/anyio_fixture_methods/lib/python3.12/site-packages/trio/_core/_run.py", line 959, in __aexit__
  |     raise combined_error_from_nursery
  | ExceptionGroup: Exceptions from Trio nursery (1 sub-exception)
  +-+---------------- 1 ----------------
    | Exception Group Traceback (most recent call last):
    |   File "/home/graingert/projects/demo.py", line 9, in main
    |     raise MyExceptionGroup("hello", [ValueError("bad")])
    | MyExceptionGroup: hello (1 sub-exception)
    +-+---------------- 1 ----------------
      | ValueError: bad
      +------------------------------------

[graingert]

The problem is that splitting Cancelled from an ExceptionGroup tree converts custom EG subclasses into regular EGs:

trio/src/trio/_core/_run.py

Lines 641 to 648 in 3de7996

	elif isinstance(exc, BaseExceptionGroup):
	matched, exc = exc.split(Cancelled)
	if matched:
	self.cancelled_caught = True
	if exc:
	exc = collapse_exception_group(exc)

[A5rocks]

Do we need to provide our own .split?

>>> class ExampleEG(ExceptionGroup):
...     pass
...
>>> eg = ExampleEG("test", [ValueError("bad")])
>>> eg.split(TypeError)
(None, ExceptionGroup('test', [ValueError('bad')]))
>>> eg.split(ValueError)
(ExceptionGroup('test', [ValueError('bad')]), None)
>>> eg
ExampleEG('test', [ValueError('bad')])

Also, is this something we should report upstream to CPython?

---

I think I get the logic for why this doesn't return subtypes though:

>>> class ExampleEG(BaseExceptionGroup):
...     pass
...
>>> eg = ExampleEG("test", [ValueError("bad"), KeyboardInterrupt()])
>>> eg.split(ValueError)
(ExceptionGroup('test', [ValueError('bad')]), BaseExceptionGroup('test', [KeyboardInterrupt()]))

This way Python can preserve that ExceptionGroups are the norm unless someone passes in a not-Exception. I don't think this property is worth making every subclass make their own .split method though? IMO the left part of the tuple could be an ExceptionGroup/BaseExceptionGroup and the right part could be the original type -- though maybe with special casing for BaseExceptionGroup -> ExceptionGroup if possible.

[graingert]

I think the fix is using a custom split that doesn't recreate ExceptionGroups if the children aren't changing.

Probably it is worth reporting

[TeamSpen210]

For custom subclasses, you need to define derive(), so it knows how to create instances - it's not safe obviously to assume the constructor args are the same.

[A5rocks]

Should we warn about that and provide a sane alternative?

[jakkdl]

defining an ExceptionGroup subclass without a derive could maybe be a linter warning? (either flake8-async or flake8-bugbear)
I don't think trio should create a custom split here