-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
avoid collapsing exception groups from user code #2830
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
base: main
Are you sure you want to change the base?
Changes from all commits
ae86e1d
a4687d7
ffacdb2
02c221f
c7c6206
802409a
5b43511
6500d2f
5e4a262
e44b254
e24198f
e780650
6ad6d7c
f30ef9f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -4,7 +4,9 @@ | |||||
| import functools | ||||||
| import sys | ||||||
| import typing | ||||||
| from contextlib import contextmanager | ||||||
| from contextlib import asynccontextmanager | ||||||
|
|
||||||
| import anyio.abc | ||||||
|
|
||||||
| from starlette.types import Scope | ||||||
|
|
||||||
|
|
@@ -13,12 +15,14 @@ | |||||
| else: # pragma: no cover | ||||||
| from typing_extensions import TypeGuard | ||||||
|
|
||||||
| has_exceptiongroups = True | ||||||
| if sys.version_info < (3, 11): # pragma: no cover | ||||||
| try: | ||||||
| from exceptiongroup import BaseExceptionGroup # type: ignore[unused-ignore,import-not-found] | ||||||
| from exceptiongroup import BaseExceptionGroup | ||||||
| except ImportError: | ||||||
| has_exceptiongroups = False | ||||||
|
|
||||||
| class BaseExceptionGroup(BaseException): # type: ignore[no-redef] | ||||||
| pass | ||||||
|
|
||||||
|
|
||||||
| T = typing.TypeVar("T") | ||||||
| AwaitableCallable = typing.Callable[..., typing.Awaitable[T]] | ||||||
|
|
@@ -70,16 +74,28 @@ async def __aexit__(self, *args: typing.Any) -> None | bool: | |||||
| return None | ||||||
|
|
||||||
|
|
||||||
| @contextmanager | ||||||
| def collapse_excgroups() -> typing.Generator[None, None, None]: | ||||||
| @asynccontextmanager | ||||||
| async def create_collapsing_task_group() -> typing.AsyncGenerator[anyio.abc.TaskGroup, None]: | ||||||
| try: | ||||||
| yield | ||||||
| except BaseException as exc: | ||||||
| if has_exceptiongroups: # pragma: no cover | ||||||
| while isinstance(exc, BaseExceptionGroup) and len(exc.exceptions) == 1: | ||||||
| exc = exc.exceptions[0] | ||||||
|
|
||||||
| raise exc | ||||||
| async with anyio.create_task_group() as tg: | ||||||
| yield tg | ||||||
| except BaseExceptionGroup as excs: | ||||||
| if len(excs.exceptions) != 1: | ||||||
| raise | ||||||
|
|
||||||
| exc = excs.exceptions[0] | ||||||
| context = exc.__context__ | ||||||
| tb = exc.__traceback__ | ||||||
| cause = exc.__cause__ | ||||||
| sc = exc.__suppress_context__ | ||||||
| try: | ||||||
| raise exc | ||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This will prevent weird traceback repetition as in pydantic/logfire#1279 (comment) and also means that existing |
||||||
| finally: | ||||||
| exc.__traceback__ = tb | ||||||
| exc.__context__ = context | ||||||
| exc.__cause__ = cause | ||||||
| exc.__suppress_context__ = sc | ||||||
| del exc, cause, tb, context | ||||||
|
|
||||||
|
|
||||||
| def get_route_path(scope: Scope) -> str: | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
| import anyio | ||
| import anyio.to_thread | ||
|
|
||
| from starlette._utils import create_collapsing_task_group | ||
| from starlette.background import BackgroundTask | ||
| from starlette.concurrency import iterate_in_threadpool | ||
| from starlette.datastructures import URL, Headers, MutableHeaders | ||
|
|
@@ -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: | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we also have an issue with With my comment on the other PR I was trying to avoid the changes here. 🤔
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
|
||
| async def wrap(func: typing.Callable[[], typing.Awaitable[None]]) -> None: | ||
| await func() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.