avoid collapsing exception groups from user code #2830
Conversation
| @@ -258,7 +259,7 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: | |||
| except OSError: | |||
| raise ClientDisconnect() | |||
| else: | |||
| async with anyio.create_task_group() as task_group: | |||
| async with create_collapsing_task_group() as task_group: | |||
There was a problem hiding this comment.
Do we also have an issue with StreamingResponse standalone?
With my comment on the other PR I was trying to avoid the changes here. 🤔
There was a problem hiding this comment.
Yeah it's the same sort of issue, but it's wrapped in two TaskGroups before the user gets the exception. Collapsing is ok here because if wait_for_disconnect raises an exception it is always 'catastrophic' eg won't be caught
|
I have encountered this issue with starlette. I would love to have this PR merged tbh :) |
| cause = exc.__cause__ | ||
| sc = exc.__suppress_context__ | ||
| try: | ||
| raise exc |
There was a problem hiding this comment.
| raise exc | |
| raise exc from exc.__cause__ or exc.__context__ |
This will prevent weird traceback repetition as in pydantic/logfire#1279 (comment) and also means that existing __context__ won't be lost from the traceback and replaced with the exception group as the context instead, although it will become __cause__ instead.
Summary
If user code is raising ExceptionGroups eg from TaskGroups or otherwise, they should get ExceptionGroups raised. There could also be important notes attached.
Checklist
No docs changes needed, probably should have made an issue for this.