gh-135736: Fix interaction between TaskGroup and aclose()

[msullivan]

Currently if you have a generator like:

async def test():
    async with asyncio.TaskGroup() as tg:
        async for x in whatever():
            yield x

If aclose() is called on the generator and GeneratorExit is raised,
the TaskGroup's __aexit__ will raise a BaseExceptionGroup, and that
will be raised from the aclose() call instead of it being swallowed.

Fix this by raising GeneratorExit from aexit when:

1. The body of task group raised it
2. No subtasks raised exceptions

This makes GeneratorExit work without swallowing other exceptions
(like it would if we just added it to _is_base_error).

• Issue: GeneratorExit and asyncio.TaskGroup - missing from _is_base_error? #135736

[msullivan]

A slightly different approach that @1st1 and I discussed would be to add a class:

class TaskGroupGeneratorExit(GeneratorExit, BaseExceptionGroup):
    pass

and raise that, wrapping the original GeneratorExit, instead of just raising the original GeneratorExit.

This has the advantage that the behavior changes less: a BaseExceptionGroup still gets raised, it's just also now a GeneratorExit. I have an implementation of this and it works fine, but the idea of having to documen this exception for end users seemed unfortunate to me, so I didn't do it for this PR.

Any thoughts on this?

[read-the-docs-community]

Documentation build overview

📚 cpython-previews | 🛠️ Build #33749787 | 📁 Comparing 147f0f4 against main (998fc4a)

  🔍 Preview build  

2 files changed

± library/asyncio-task.html
± whatsnew/changelog.html

[kumaraditya303]

This PR seems to do the same as #154538

[msullivan]

This PR seems to do the same as #154538

Huh, very funny that the bug sits around for a year and attracts two competing PRs in the same week.

Anyway, I think the approaches and behaviors of the two PRs are quite different, though.

I think this approach is preferable, because it doesn't drop exceptions on a GeneratorExit, which is not a particularly exceptional case. (Unlike SystemExit and KeyboardInterrupt)

[1st1]

because it doesn't drop exceptions on a GeneratorExit,

Yeah, that's quite important.

@kumaraditya303 Yeah, this PR has a better approach. Basically,

• A generator has a task group, all tasks completed, a GeneratorExit is thrown in -- we propagate just GeneratorExit, aclose() swallows it, no information lost.
• A generator has a task group, some tasks errored out, a GeneratorExit is thrown in -- we propagate an ExceptionGroup with GeneratorExit in it. aclose() would re-raise, no information is lost again. GeneratorExit ideally shouldn't be part of the propagated ExceptionGroup, but the fact that it's there isn't a big deal (removing it would require much more involved fix).

IMO this PR is fine.

We should salvage the logging part of https://github.com/python/cpython/pull/154538/changes in a separate PR though.

[1st1]

@msullivan I've approved the PR but I'll let it sit for some time before hitting the button in case Kumar or others have more comments.

[kumaraditya303]

Suggested change

	with self.assertRaises(BaseExceptionGroup):
	await fn()
	with self.assertRaises(BaseExceptionGroup) as cm:
	await fn()
	self.assertEqual(get_error_types(cm.exception), {GeneratorExit})

[kumaraditya303]

I think we are backporting this, in that case it doesn't make sense to keep a version changed entry for 3.16