GeneratorExit and asyncio.TaskGroup - missing from _is_base_error?

[ghuser1234523]

Bug report

Bug description:

Consider the following code:

import asyncio
  
async def TestGen():
	yield 1
	
async def TestFn():
	async with asyncio.TaskGroup() as tg:
		async for n in TestGen():
			yield n
			
async def Run():
	g = TestFn()
	await g.asend( None )
	await g.aclose()
	
asyncio.run( Run() )

which unexpectedly yields the following output:

  + Exception Group Traceback (most recent call last):
  |   File "<python-input-7>", line 16, in <module>
  |     asyncio.run( Run() )
  |     ~~~~~~~~~~~^^^^^^^^^
  |   File "/usr/lib/python3.13/asyncio/runners.py", line 195, in run
  |     return runner.run(main)
  |            ~~~~~~~~~~^^^^^^
  |   File "/usr/lib/python3.13/asyncio/runners.py", line 118, in run
  |     return self._loop.run_until_complete(task)
  |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  |   File "/usr/lib/python3.13/asyncio/base_events.py", line 719, in run_until_complete
  |     return future.result()
  |            ~~~~~~~~~~~~~^^
  |   File "<python-input-7>", line 14, in Run
  |     await g.aclose()
  |   File "<python-input-7>", line 7, in TestFn
  |     async with asyncio.TaskGroup() as tg:
  |                ~~~~~~~~~~~~~~~~~^^
  |   File "/usr/lib/python3.13/asyncio/taskgroups.py", line 71, in __aexit__
  |     return await self._aexit(et, exc)
  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^
  |   File "/usr/lib/python3.13/asyncio/taskgroups.py", line 173, in _aexit
  |     raise BaseExceptionGroup(
  |     ...<2 lines>...
  |     ) from None
  | BaseExceptionGroup: unhandled errors in a TaskGroup (1 sub-exception)
  +-+---------------- 1 ----------------
    | Traceback (most recent call last):
    |   File "<python-input-7>", line 9, in TestFn
    |     yield n
    | GeneratorExit
    +------------------------------------

Expectation: no "unhandled errors in a TaskGroup" error.

Quick look through taskgroups.py reveals that GeneratorExit might be missing from _is_base_error() method:

    # Since Python 3.8 Tasks propagate all exceptions correctly,
    # except for KeyboardInterrupt and SystemExit which are
    # still considered special.

    def _is_base_error(self, exc: BaseException) -> bool:
        assert isinstance(exc, BaseException)
        return isinstance(exc, (SystemExit, KeyboardInterrupt))

Adding GeneratorExit to _is_base_error seems to solve issue:

import asyncio

class TaskGroupWithGeneratorExit(asyncio.TaskGroup):
	def _is_base_error(self, exc: BaseException) -> bool:
		if super()._is_base_error( exc ):
			return True
		return isinstance(exc, GeneratorExit)
  
async def TestGen():
	yield 1
	
async def TestFn():
	async with TaskGroupWithGeneratorExit() as tg:
		async for n in TestGen():
			yield n
			
async def Run():
	g = TestFn()
	await g.asend( None )
	await g.aclose()
	
asyncio.run( Run() )

The above yields no output, as expected.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs

• gh-135736: Fix asyncio.TaskGroup swallowing errors on GeneratorExit #154538
• gh-135736: Fix interaction between TaskGroup and aclose() #154649
• [3.15] gh-135736: Fix interaction between TaskGroup and aclose() (GH-154649) #154900

[serhiy-storchaka]

I wonder, should not it be defined as not isinstance(exc, Exception)? Since SystemExit, KeyboardInterrupt, and GeneratorExit are the only standard exceptions which are not derived from Exception, it is almost the same as isinstance(exc, (SystemExit, KeyboardInterrupt, GeneratorExit)). But it would also work with third-party exceptions not derived from Exception.

cc @gvanrossum, @1st1, @asvetlov.

[gvanrossum]

I'm sure @1st1 did this intentionally. I recall talking about it with him. But I don't recall the exact motivation, only that it struck me as odd at the time, though I think maybe the point was to allow all exceptions to be caught, including BaseExceptions, except for KeyboardError and SystemExit (the comment pretty much says so). If you add GeneratorExit to the list, do all the tests still pass? (Are there enough tests for GeneratorExit?) If you make it not isinstance(exc, Exception) do any tests fail?

Sorry I have no better insight. :-(

[luctowers]

@serhiy-storchaka @gvanrossum It should be noted that TaskGroup._is_base_error is used in two locations.

1. in TaskGroup.__aexit__ where exception in the with ends up (this is the issue in the example above)
2. in TaskGroup._on_task_done where the exceptions from the tasks ends up

The current implementation of _is_base_error makes sense to me for the second case, a task raising a GeneratorExit should be treated as a normal exception. For the first case though, it creates this unexpected behaviour in the bug report.

[gvanrossum-ms]

I've pinged Yury privately. Maybe he has an opinion.

[1st1]

I've pinged Yury privately. Maybe he has an opinion.

Thanks for pinging me!

Let's start with covering the current logic of task groups and why _is_base_error even exists.

TaskGroup wraps all exceptions propagated within it in an exception group. But if a SystemExit error is raised and would be wrapped in a group, it then wouldn't be intercepted by except SystemExit or even perform the intended action which is stopping the application ASAP. That's why KeyboardInterrupt and SystemExit are so special - we can't wrap them.

The logic is as follows:

• TaskGroup runs tasks concurrently
• If any task fails the TaskGroup will cancel all of tasks that are still running at that point, and wait for all tasks to finish their cancellation
• If any task raises KeyboardInterrupt or SystemExit we memorize that in self._base_error, but proceed business as usual
• When everything is about to be done, in TaskGroup.__aexit__, just before propagating all exceptions wrapped in an ExceptionGroup, we check if self._base_error is not None and raise that if it is. If not - raise all collected exceptions in a group.

Is this ideal? No, but this is how we could ship task groups without breaking previous asyncio behavior. Besides, it's unlikely that you'll need to do much with the remainder of the task errors if a SystemExit or KeyboardInterrupt is happening.

I wonder, should not it be defined as not isinstance(exc, Exception)?

asyncio.CancelledError is BaseException (link) and we don't want cancellation to break everything like SystemExit, so no, we can't do that.

Adding GeneratorExit to _is_base_error seems to solve issue:

I think... this might be the only option we have here, but it is problematic in its own way. The problem is that all trapped exceptions in the TaskGroup won't propagate (and be lost), only GeneratorExit would.

A few thoughts in no particular order:

• We've lived this long without noticing this because typically generators are either aborted by CancelledError or run to completion. It's rare that users manually close generators mid-flight.
• GeneratorExit captured in a TaskGroup is a problem, we can't do that.
• The main complication is that we "silence" all exceptions from the group if aclose() is called. To mitigate this:
  • We should call loop.call_exception_handler() on all silenced errors, so that they are at least logged.
  • We could make SystemExit, KeyboardInterrupt, and GeneratorExit inherit from BaseExceptionGroup, and then we could make AsyncGenerator.aclose() implementation to re-raise the captured "bag" of errors once the generator is closed. This is harder to implement for all obvious reasons but is probably the best option long term.

---

Summary: let's special case GeneratorExit in _is_base_error, but add code that calls loop.call_exception_handler() on every error before propagating GeneratorExit. Potentially we should do that when we raise SystemExit and KeyboardInterrupt too.

[achimnol]

Please have a look at my own solution:

• aiotools.as_complated_safe()
  • source code
  • I found that this function also need to re-raise GeneratorExit transparently. But there was nothing to do special in TaskScope about it.
• aiotools.TaskScope
  • source code
  • This provides an explicit option to use loop.call_exception_handler() or set a custom exception handler callback for the subtasks spawned by the scope.
  • Along with gh-101581: Add asyncio.TaskScope and let asyncio.TaskGroup subclass it #105011 and aiotools, I'm thinking about whether we should provide a way to switch between callbacks and ExceptionGroup by user choice.