-
Notifications
You must be signed in to change notification settings - Fork 146
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
Don't wrap exceptions in ExceptionGroup
if only one exception is raised on Python<3.11
#668
Comments
Not going to happen. This was a deliberate design decision, and asyncio already does the same with their |
I was afraid you were going to say that. :-) Does this mean that Python 3.11 should be considered the minimum version for anyio 4? This behavior is viral in unexpected ways. async with context_manager_of_3rd_party_library():
raise SomeException Then, later, as the author of the context manager, we decide that it's better to implement it by opening a task group in there. This means that suddenly, the exception will be propagated through a task group, which turns this exception into an For anyone coming across this issue, here is the related Trio issue: python-trio/trio#2785 |
Why? If you're targeting Python versions below 3.11, you just have to use the |
It's not "just". As a library author, it means asking end-users to use the If they had code like in the example above. If users suddenly see their exception being turned into another exception and they now require To be clear, I totally understand if this is not going to be changed. I only want to share how this complicates upgrading libraries to anyio 4 in these situations.
Yes, I do that as well. In the example however, we're talking about an exception from a user, and not one that was raised in a task, but rather in the body of a context manager that happens to open a task group internally. |
So you wrap the taskgroup and the Ten lines of boilerplate. Annoying, yes, but hardly viral. |
I'm trying with a from exceptiongroup import ExceptionGroup, catch
@asynccontextmanager
async def create_task_group() -> AsyncGenerator[anyio.abc.TaskGroup, None]:
def handler(exc: BaseException) -> None:
if isinstance(exc, ExceptionGroup) and len(exc.exceptions) == 1:
raise exc.exceptions[0]
raise exc
with catch({Exception: handler}):
async with anyio.create_task_group() as tg:
yield tg |
And what if the exception group contains another one? |
Don't forget, |
I know, it is somewhat fragile, but to me it looks feasible to write code where almost all exceptions are caught within the child tasks, and at most one exception will propagate. At least, that is what we currently have in a reasonable sized codebase (~200k loc). Do you think there will be situations where additional exceptions will be triggered, that can't be suppressed?
Right. It's a good point. Actually I only now realize that the star in |
Yeah because, if exception group collapsing was enabled (as in anyio 3), |
Not sure whether it's a feature or bug/regression.
I'm in the process of upgrading from anyio 3 to anyio 4.
So far we've explicitly designed our code in a way that no more than 1 exception will be raised in a task group. (By making sure only one code path will result in an exception.) This worked great, and prevented us from having to deal with exception groups. We still support Python 3.8 so, that's important to us.
However, after upgrading to anyio 4, even if there is only one exception raised as part of a task group, it will be wrapped in an
ExceptionGroup
. This means, we have to use thewith catch()
syntax which is quite cumbersome, and everywhere. That's a huge amount of work, and boilerplate to add. :'(.It would be nice if the code could be modified so that on Python<3.11, if there is only one exception, it won't be wrapped. The change is very simple, and should be backward compatible.
The text was updated successfully, but these errors were encountered: