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

Re-raising a CancelledError after catching it in an ExceptionGroup confuses asyncio backend #634

Closed
2 tasks done
DaGenix opened this issue Nov 17, 2023 · 6 comments · Fixed by #639
Closed
2 tasks done
Labels
bug Something isn't working

Comments

@DaGenix
Copy link

DaGenix commented Nov 17, 2023

Things to check first

  • I have searched the existing issues and didn't find my bug already reported there

  • I have checked that my bug is still present in the latest release

AnyIO version

4.0.0

Python version

3.11

What happened?

If you catch a CancelledError as part of an ExceptionGroup and then re-raise it when using the asyncio backend, the surrounding CacnelScope won't recognize it as a cancellation exception anymore. The end result, is that instead of cancellation working as expected and the exception being swallowed by the CancelScope, its instead re-raised.

This issue only occurs when using the asyncio backend. It does not occur on the trio backend.

I think the issue is that trio internally will unwrap cancellation exception inside of Exception Groups: https://github.com/python-trio/trio/blob/d5c0fb83bba6b56c35956c65092411cc9a2be8c3/src/trio/_core/_run.py#L595-L604. However, I don't think the asyncio backend in anyio does the same thing.

How can we reproduce the bug?

Run the following code, which exits with an exception. Change the backend to "trio" and it exits without an exception.

import anyio


async def main():
    async with anyio.create_task_group() as tg:
        tg.cancel_scope.cancel()
        try:
            await anyio.sleep_forever()
        except* anyio.get_cancelled_exc_class():
            raise


if __name__ == "__main__":
    anyio.run(main, backend="asyncio")
@DaGenix DaGenix added the bug Something isn't working label Nov 17, 2023
@agronholm
Copy link
Owner

Your assessment is correct: the asyncio version of AnyIO's task group does not recognize cancellation errors from the host coming wrapped in an exception group.

@DaGenix
Copy link
Author

DaGenix commented Nov 17, 2023

@agronholm Is that the intended behavior?

A slight more involved, motivating example:

In Anyio 3.7.1, this code does what I'd expect - it either prints out "Inner task failed" or "Inner task was cancelled":

import anyio
import random


class WorkFailed(Exception):
    pass


async def do_work():
    async with anyio.create_task_group():
        if random.random() < 0.5:
            raise WorkFailed()
        else:
            await anyio.sleep_forever()


async def main():
    with anyio.move_on_after(1):
        try:
            await do_work()
        except WorkFailed:
            print("Inner task failed")
        except anyio.get_cancelled_exc_class():
            print("Inner task was cancelled")
            raise


if __name__ == "__main__":
    anyio.run(main, backend="asyncio")

In Anyio 4.0.0, however, that code now doesn't catch the WorkFailed Exception and log it - since its wrapped in an ExceptionGroup.

So, if I change the code so that it catches the WorkFailed exception type using except* syntax, now I can catch the WorkFailed events, but in the cancel case, re-raising the cancelation exception doesn't work as I'd expect since the asyncio TaskGroup backend doesn't recognize the re-raised cancelled exception anymore since its now wrapped in a BaseExceptionGroup by Python due to the except* syntax:

async def main():
    with anyio.move_on_after(1):
        try:
            await do_work()
        except* WorkFailed:
            print("Inner task failed")
        except* anyio.get_cancelled_exc_class():
            print("Inner task was cancelled")
            raise

I know that there are a few ways to solve that. The first thing that comes to mind is:

async def main():
    with anyio.move_on_after(1):
        try:
            await do_work()
        except BaseException as ex:
            if isinstance(ex, BaseExceptionGroup):
                if ex.subgroup(WorkFailed):
                    print("Inner task failed")
                else:
                    raise
            elif isinstance(ex, anyio.get_cancelled_exc_class()):
                print("Inner task was cancelled")
                raise
            else:
                raise

And that works - but it seems to be a bit less pleasant to write than using except* syntax.

Anyway, anyio is awesome and I just wanted to include a less contrived example of what motivated me to open up this issue. Thanks!

@agronholm
Copy link
Owner

I aim for the asyncio and trio backends to have the same semantics to the extent possible. I think it's reasonable to try to make the asyncio backend work the same way as trio in this case.

@agronholm
Copy link
Owner

I'm pretty confused now, as this test passes on all Python versions:

async def test_reraise_cancelled_in_excgroup() -> None:
    def handler(excgrp: BaseExceptionGroup) -> None:
        raise

    async with anyio.create_task_group() as tg:
        tg.cancel_scope.cancel()
        with catch({get_cancelled_exc_class(): handler}):
            await anyio.sleep_forever()

But this 3.11+ syntax test does not pass on asyncio:

async def test_reraise_cancelled_in_excgroup() -> None:
    async with anyio.create_task_group() as tg:
        tg.cancel_scope.cancel()
        try:
            await anyio.sleep_forever()
        except* get_cancelled_exc_class():
            raise

I verified that the handler is called.

@agronholm
Copy link
Owner

Ok, this seems like a subtle bug in exceptiongroup, as if it catches a naked exception and the handler does raise, the original one bubbles out of the context manager without being wrapped in an exception group.

@agronholm
Copy link
Owner

With exceptiongroup 1.2.0, the test now fails as expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants