From eac12735f9b278dfdcb8611ffa7b5f8c6807675e Mon Sep 17 00:00:00 2001 From: JasperSui Date: Sun, 16 Aug 2026 15:36:03 +0800 Subject: [PATCH 1/2] fix: unwind exit stacks with in-flight exception details so generator dependencies can roll back (#255) When a decorated function raises, generator dependencies never saw the exception: exit stacks were always closed via aclose(), and the inner FastAPI stacks (where generator teardowns actually live on fastapi>=0.121) were registered as aclose() callbacks that stripped exception details even when the owning stack unwound with them. - Register the inner FastAPI stacks via push_async_exit so exception details flow through to generator dependencies at their yield point. - Plumb an optional exc= through cleanup_exit_stack_of_func / cleanup_all_exit_stacks (and AsyncExitStackManager) to unwind stacks with __aexit__(type(exc), exc, tb), mirroring how FastAPI unwinds a failing request. A dependency re-raising the in-flight exception is treated as normal CM protocol, not a cleanup failure. - injectable_scope() now forwards the in-flight exception end to end, so except/rollback branches run with no extra plumbing. - Make _get_or_create_current_loop sticky (set_event_loop on create): consecutive synchronous calls previously landed on fresh throwaway loops, so the per-loop stack registry silently skipped teardown of stacks owned by an earlier loop. - Fix a latest-mypy failure in util.py (cast instead of a stale ignore). --- README.md | 43 ++++ src/fastapi_injectable/async_exit_stack.py | 51 ++++- src/fastapi_injectable/concurrency.py | 18 +- src/fastapi_injectable/main.py | 10 +- src/fastapi_injectable/scope.py | 6 + src/fastapi_injectable/util.py | 23 +- test/test_async_exit_stack.py | 78 +++++++ test/test_concurrency.py | 5 + test/test_exit_stack_exception_unwind.py | 253 +++++++++++++++++++++ test/test_scope.py | 19 +- test/test_util.py | 2 +- 11 files changed, 488 insertions(+), 20 deletions(-) create mode 100644 test/test_exit_stack_exception_unwind.py diff --git a/README.md b/README.md index 8d7e24e..7848b6c 100755 --- a/README.md +++ b/README.md @@ -407,6 +407,49 @@ run_coroutine_sync(cleanup_all_exit_stacks()) # accepts raise_exception=True too assert machine.db.closed is True ``` +#### Unwinding with an in-flight exception (rollback on error) + +In a FastAPI request, when your endpoint raises, generator dependencies receive the exception at their `yield` — so `except` branches (e.g. `rollback()`) run before `finally`. Outside a request you get the same behavior by passing the caught exception to the cleanup helpers via `exc=`, or by using `injectable_scope()`, which forwards it automatically: + +```python +from collections.abc import AsyncGenerator +from typing import Annotated + +from fastapi import Depends +from fastapi_injectable import cleanup_all_exit_stacks, injectable, injectable_scope + +async def get_connection() -> AsyncGenerator[Connection, None]: + conn = Connection() + try: + yield conn + except Exception: + conn.rollback() # runs when the decorated function raised + raise + else: + conn.commit() + finally: + conn.close() + +@injectable +async def process(conn: Annotated[Connection, Depends(get_connection)]) -> None: + raise ValueError("boom") + +# Option #1: pass the exception to the cleanup helpers explicitly +try: + await process() +except ValueError as exc: + # conn.rollback() and conn.close() run; without exc= only conn.commit()/close() would + await cleanup_all_exit_stacks(exc=exc) + # cleanup_exit_stack_of_func(process, exc=exc) also accepts it + +# Option #2: injectable_scope() forwards the exception automatically, +# exactly like a failing FastAPI request unwinding its exit stack +async with injectable_scope(): + await process() # raises -> conn.rollback() + conn.close() run during unwind +``` + +Just like in FastAPI, teardown that must **always** run belongs in `finally`: when an exception is delivered to the generator, code placed after a bare `yield` (with no `try`/`finally`) is skipped. + ### Async Support `fastapi-injectable` provides full support for both synchronous and asynchronous dependencies, allowing you to mix and match them as needed. You can freely use async dependencies in sync functions and vice versa. For cases where you need to run async code in a synchronous context, we provide the `run_coroutine_sync` utility function. diff --git a/src/fastapi_injectable/async_exit_stack.py b/src/fastapi_injectable/async_exit_stack.py index 29daad9..412d9af 100644 --- a/src/fastapi_injectable/async_exit_stack.py +++ b/src/fastapi_injectable/async_exit_stack.py @@ -80,7 +80,30 @@ async def get_stack(self, func: Callable[..., Any]) -> AsyncExitStack: per_loop[func] = stack return stack - async def _close_stack(self, loop: _Loop, stack: AsyncExitStack) -> None: + @staticmethod + async def _unwind_stack(stack: AsyncExitStack, exc: BaseException | None) -> None: + """Unwind one stack, mirroring how a context manager would exit. + + With ``exc`` the stack exits as if the exception propagated out of an + ``async with`` block: every teardown (generator ``except``/rollback + branches included) sees the original exception. Without it the stack is + closed normally (``aclose()``, i.e. ``__aexit__(None, None, None)``). + """ + if exc is None: + await stack.aclose() + return + try: + await stack.__aexit__(type(exc), exc, exc.__traceback__) + except BaseException as unwind_exc: + # A generator dependency that re-raises the in-flight exception + # ("except: rollback(); raise") follows the context-manager protocol + # (exception not suppressed) -- AsyncExitStack.__aexit__ then re-raises + # it here. The caller already caught and handled ``exc``, so only + # genuine teardown failures (a *different* exception) propagate. + if unwind_exc is not exc: + raise + + async def _close_stack(self, loop: _Loop, stack: AsyncExitStack, exc: BaseException | None = None) -> None: """Close one stack on its owning loop. - owning loop is the current running loop -> close it here directly; @@ -92,19 +115,29 @@ async def _close_stack(self, loop: _Loop, stack: AsyncExitStack) -> None: """ running = self._running_loop() if loop is running: - await stack.aclose() + await self._unwind_stack(stack, exc) return if loop.is_closed() or not loop.is_running(): return - future = asyncio.run_coroutine_threadsafe(stack.aclose(), loop) + future = asyncio.run_coroutine_threadsafe(self._unwind_stack(stack, exc), loop) await asyncio.wrap_future(future) - async def cleanup_stack(self, func: Callable[..., Any], *, raise_exception: bool = False) -> None: + async def cleanup_stack( + self, + func: Callable[..., Any], + *, + raise_exception: bool = False, + exc: BaseException | None = None, + ) -> None: """Clean up the stack(s) associated with the given function. Args: func: The function whose exit stack should be cleaned up raise_exception: If True, raises DependencyCleanupError when cleanup fails + exc: The in-flight exception, if any. When provided, each stack is unwound + with the exception details (``__aexit__(type(exc), exc, exc.__traceback__)``) + exactly as if it had propagated out of an ``async with`` block, so + generator dependencies can run their ``except``/rollback branches. Raises: DependencyCleanupError: When cleanup fails and raise_exception is True @@ -125,7 +158,7 @@ async def cleanup_stack(self, func: Callable[..., Any], *, raise_exception: bool exception_: Exception | None = None msg = "" try: - await asyncio.gather(*(self._close_stack(loop, stack) for loop, stack in entries)) + await asyncio.gather(*(self._close_stack(loop, stack, exc) for loop, stack in entries)) except RuntimeError as e: msg = f"Failed to cleanup stack for {func.__name__} during teardown: {e}" logger.warning(msg) @@ -138,11 +171,15 @@ async def cleanup_stack(self, func: Callable[..., Any], *, raise_exception: bool if exception_ is not None and raise_exception: raise DependencyCleanupError(msg) from exception_ - async def cleanup_all_stacks(self, *, raise_exception: bool = False) -> None: + async def cleanup_all_stacks(self, *, raise_exception: bool = False, exc: BaseException | None = None) -> None: """Clean up all stacks, each on its own owning loop. Args: raise_exception: If True, raises DependencyCleanupError when any cleanup fails + exc: The in-flight exception, if any. When provided, every stack is unwound + with the exception details (``__aexit__(type(exc), exc, exc.__traceback__)``) + exactly as if it had propagated out of an ``async with`` block, so + generator dependencies can run their ``except``/rollback branches. Raises: DependencyCleanupError: When any cleanup fails and raise_exception is True @@ -157,7 +194,7 @@ async def cleanup_all_stacks(self, *, raise_exception: bool = False) -> None: exception_: Exception | None = None msg = "" try: - await asyncio.gather(*(self._close_stack(loop, stack) for loop, stack in entries)) + await asyncio.gather(*(self._close_stack(loop, stack, exc) for loop, stack in entries)) except RuntimeError as e: msg = f"Failed to cleanup one or more dependency stacks during teardown: {e}" logger.warning(msg) diff --git a/src/fastapi_injectable/concurrency.py b/src/fastapi_injectable/concurrency.py index c40ba9f..362f38b 100644 --- a/src/fastapi_injectable/concurrency.py +++ b/src/fastapi_injectable/concurrency.py @@ -83,13 +83,27 @@ def _get_or_create_current_loop(self) -> asyncio.AbstractEventLoop: Compatible with Python 3.12+ and 3.14+. Attempts to get loop via policy, falls back to creating new loop if RuntimeError is raised. + A newly created loop is registered as the thread's current loop + (``set_event_loop``) so that consecutive synchronous calls keep landing on + the SAME loop. Without this, each call would run on a fresh throwaway loop: + dependencies would be resolved on loop A while their cleanup ran on loop B, + and the per-loop exit-stack registry (see ``AsyncExitStackManager``) would + rightly refuse to close A's stacks from B -- silently skipping generator + teardown. This mirrors the pre-3.12 auto-create semantics of + ``asyncio.get_event_loop()``. + Returns: Event loop instance. """ + policy = asyncio.get_event_loop_policy() try: - return asyncio.get_event_loop_policy().get_event_loop() + loop = policy.get_event_loop() except RuntimeError: - return asyncio.get_event_loop_policy().new_event_loop() + loop = None + if loop is None or loop.is_closed(): + loop = policy.new_event_loop() + asyncio.set_event_loop(loop) + return loop @property def loop_strategy(self) -> Literal["current", "isolated", "background_thread"]: diff --git a/src/fastapi_injectable/main.py b/src/fastapi_injectable/main.py index e871984..235bc9c 100644 --- a/src/fastapi_injectable/main.py +++ b/src/fastapi_injectable/main.py @@ -170,8 +170,14 @@ async def resolve_dependencies( fastapi_inner_astack = AsyncExitStack() fastapi_function_astack = AsyncExitStack() - async_exit_stack.push_async_callback(fastapi_inner_astack.aclose) - async_exit_stack.push_async_callback(fastapi_function_astack.aclose) + # Registered via push_async_exit (NOT push_async_callback(stack.aclose)) so that + # when the owning stack unwinds with an exception, the exception details reach + # these inner stacks. On fastapi>=0.121 generator dependencies are entered into + # them (see the fake_request_scope note below), and an ``aclose()`` callback + # would strip the exception -- generators would see a bare GeneratorExit and + # their ``except``/rollback branches could never run (issue #255). + async_exit_stack.push_async_exit(fastapi_inner_astack) + async_exit_stack.push_async_exit(fastapi_function_astack) fake_request_scope: dict[str, Any] = { "type": "http", diff --git a/src/fastapi_injectable/scope.py b/src/fastapi_injectable/scope.py index 47ed89e..41e4142 100644 --- a/src/fastapi_injectable/scope.py +++ b/src/fastapi_injectable/scope.py @@ -31,6 +31,12 @@ class InjectableScope: cached values into this scope's cache. Leaving the scope closes the exit stack (running all cleanup) and drops the cache. + When an exception propagates out of the ``async with`` block, it is forwarded + to the exit stack -- generator dependencies receive it at their ``yield`` and + can run their ``except``/rollback branches, exactly as FastAPI unwinds a + failing request (issue #255). As in FastAPI, teardown that must always run + belongs in a ``finally`` block. + The cache is partitioned by event loop. A scope object reused across event loops -- resolved on loop A then on loop B while both are alive -- must never serve a loop-A resource to loop B: a cached value commonly holds a loop-bound diff --git a/src/fastapi_injectable/util.py b/src/fastapi_injectable/util.py index 011d3d5..867e2ae 100644 --- a/src/fastapi_injectable/util.py +++ b/src/fastapi_injectable/util.py @@ -142,7 +142,7 @@ async def inner(dep: T2) -> T2: # If it's an async generator, get the first value if inspect.isasyncgen(dep): async for value in dep: # pragma: no cover - return value # type: ignore[no-any-return] + return cast("T2", value) return dep # Nice signature for docs/inspection @@ -446,13 +446,22 @@ async def get_db() -> AsyncGenerator[Database, None]: return await coro -async def cleanup_exit_stack_of_func(func: Callable[..., Any], *, raise_exception: bool = False) -> None: +async def cleanup_exit_stack_of_func( + func: Callable[..., Any], + *, + raise_exception: bool = False, + exc: BaseException | None = None, +) -> None: """Clean up the exit stack associated with a specific function. Args: func: The function whose exit stack should be cleaned up. raise_exception: Whether to raise exceptions during cleanup. If False, exceptions are logged as warnings. Defaults to False. + exc: The in-flight exception, if any. When provided, the exit stack is unwound + with the exception details -- exactly as FastAPI does when a request fails -- + so generator dependencies see the original exception and can run their + ``except``/rollback branches instead of a plain close. Notes: - This ensures that resources such as context managers or other async cleanup routines @@ -462,15 +471,19 @@ async def cleanup_exit_stack_of_func(func: Callable[..., Any], *, raise_exceptio DependencyCleanupError: When cleanup fails and raise_exception is True """ for wrapper in PROVIDER_TO_WRAPPER_FUNC_MAP.get(func, [func]): - await async_exit_stack_manager.cleanup_stack(wrapper, raise_exception=raise_exception) + await async_exit_stack_manager.cleanup_stack(wrapper, raise_exception=raise_exception, exc=exc) -async def cleanup_all_exit_stacks(*, raise_exception: bool = False) -> None: +async def cleanup_all_exit_stacks(*, raise_exception: bool = False, exc: BaseException | None = None) -> None: """Clean up all active exit stacks. Args: raise_exception: Whether to raise exceptions during cleanup. If False, exceptions are logged as warnings. Defaults to False. + exc: The in-flight exception, if any. When provided, every exit stack is unwound + with the exception details -- exactly as FastAPI does when a request fails -- + so generator dependencies see the original exception and can run their + ``except``/rollback branches instead of a plain close. Notes: - This method iterates through all registered exit stacks and ensures they are properly closed. @@ -479,7 +492,7 @@ async def cleanup_all_exit_stacks(*, raise_exception: bool = False) -> None: Raises: DependencyCleanupError: When cleanup fails and raise_exception is True """ - await async_exit_stack_manager.cleanup_all_stacks(raise_exception=raise_exception) + await async_exit_stack_manager.cleanup_all_stacks(raise_exception=raise_exception, exc=exc) async def clear_dependency_cache() -> None: diff --git a/test/test_async_exit_stack.py b/test/test_async_exit_stack.py index 6e0abea..c5777cb 100644 --- a/test/test_async_exit_stack.py +++ b/test/test_async_exit_stack.py @@ -186,6 +186,84 @@ async def test_cleanup_all_stacks_with_empty_stacks(manager: AsyncExitStackManag assert len(manager._stacks) == 0 +def _make_exc() -> ValueError: + try: + msg = "boom" + raise ValueError(msg) # noqa: TRY301 + except ValueError as e: + return e + + +async def test_cleanup_stack_with_exc_unwinds_with_exception_details( + manager: AsyncExitStackManager, mock_func: Mock, mock_stack: AsyncMock +) -> None: + """With ``exc``, the stack exits via __aexit__(type, exc, tb) instead of aclose().""" + _register(manager, mock_func, mock_stack) + exc = _make_exc() + + await manager.cleanup_stack(mock_func, exc=exc) + + mock_stack.__aexit__.assert_awaited_once_with(ValueError, exc, exc.__traceback__) + mock_stack.aclose.assert_not_awaited() + assert not _contains(manager, mock_func) + + +async def test_cleanup_all_stacks_with_exc_unwinds_with_exception_details( + manager: AsyncExitStackManager, mock_func: Mock, mock_stack: AsyncMock +) -> None: + """cleanup_all_stacks(exc=...) unwinds every stack with the exception details.""" + other_func = Mock() + other_func.__name__ = "other_func" + other_stack = AsyncMock(spec=AsyncExitStack) + _register(manager, mock_func, mock_stack) + _register(manager, other_func, other_stack) + exc = _make_exc() + + await manager.cleanup_all_stacks(exc=exc) + + mock_stack.__aexit__.assert_awaited_once_with(ValueError, exc, exc.__traceback__) + other_stack.__aexit__.assert_awaited_once_with(ValueError, exc, exc.__traceback__) + mock_stack.aclose.assert_not_awaited() + other_stack.aclose.assert_not_awaited() + assert len(manager._stacks) == 0 + + +async def test_cleanup_with_exc_swallows_reraised_in_flight_exception( + manager: AsyncExitStackManager, mock_func: Mock, mock_stack: AsyncMock +) -> None: + """A dependency re-raising the in-flight exception is CM protocol, not a cleanup failure. + + AsyncExitStack.__aexit__ re-raises the passed-in exception when a generator's + ``except: ...; raise`` branch doesn't suppress it. The caller already handled that + exception, so cleanup must not report it as a DependencyCleanupError. + """ + exc = _make_exc() + tb = exc.__traceback__ # re-raising below appends frames to exc.__traceback__ + mock_stack.__aexit__.side_effect = exc + _register(manager, mock_func, mock_stack) + + await manager.cleanup_stack(mock_func, raise_exception=True, exc=exc) + + mock_stack.__aexit__.assert_awaited_once_with(ValueError, exc, tb) + assert not _contains(manager, mock_func) + + +async def test_cleanup_with_exc_still_raises_on_distinct_teardown_failure( + manager: AsyncExitStackManager, mock_func: Mock, mock_stack: AsyncMock +) -> None: + """A teardown failure distinct from the in-flight exception still surfaces.""" + exc = _make_exc() + mock_stack.__aexit__.side_effect = RuntimeError("teardown boom") + _register(manager, mock_func, mock_stack) + + with pytest.raises(Exception, match="Failed to cleanup stack for mock_func") as exc_info: + await manager.cleanup_stack(mock_func, raise_exception=True, exc=exc) + + assert isinstance(exc_info.value, DependencyCleanupError) + assert isinstance(exc_info.value.__cause__, RuntimeError) + assert not _contains(manager, mock_func) + + async def test_cleanup_stack_runtime_error_names_func_not_loop( manager: AsyncExitStackManager, mock_func: Mock, mock_stack: AsyncMock ) -> None: diff --git a/test/test_concurrency.py b/test/test_concurrency.py index af5aae1..e1fc170 100644 --- a/test/test_concurrency.py +++ b/test/test_concurrency.py @@ -58,6 +58,7 @@ def test_get_loop_current_strategy(mock_get_running_loop: Mock, loop_manager_ins def test_get_loop_current_strategy_fallback(loop_manager_instance: LoopManager) -> None: """Test get_loop with 'current' strategy when no running loop exists (Python 3.14+ compatibility).""" mock_loop = Mock() + mock_loop.is_closed.return_value = False mock_policy = Mock() mock_policy.get_event_loop.return_value = mock_loop @@ -83,12 +84,16 @@ def test_get_loop_current_strategy_fallback_create_new(loop_manager_instance: Lo with ( patch("src.fastapi_injectable.concurrency.asyncio.get_running_loop", side_effect=RuntimeError), patch("src.fastapi_injectable.concurrency.asyncio.get_event_loop_policy", return_value=mock_policy), + patch("src.fastapi_injectable.concurrency.asyncio.set_event_loop") as mock_set_event_loop, ): loop_manager_instance.set_loop_strategy("current") result = loop_manager_instance.get_loop() assert result == mock_loop + # The freshly created loop is registered as the thread's current loop so that + # subsequent synchronous calls (resolution + cleanup) land on the SAME loop. + mock_set_event_loop.assert_called_once_with(mock_loop) mock_policy.get_event_loop.assert_called_once() mock_policy.new_event_loop.assert_called_once() diff --git a/test/test_exit_stack_exception_unwind.py b/test/test_exit_stack_exception_unwind.py new file mode 100644 index 0000000..27bd305 --- /dev/null +++ b/test/test_exit_stack_exception_unwind.py @@ -0,0 +1,253 @@ +"""End-to-end regression tests for https://github.com/JasperSui/fastapi-injectable/issues/255. + +When a decorated function raises, generator dependencies must be able to see the +in-flight exception during teardown -- exactly as FastAPI unwinds a request's exit +stack -- so their ``except``/rollback branches run. Two paths provide this: + +- ``cleanup_exit_stack_of_func(func, exc=...)`` / ``cleanup_all_exit_stacks(exc=...)`` + for the global, function-keyed stacks; +- ``async with injectable_scope():`` which forwards the exception automatically. +""" + +from collections.abc import AsyncGenerator, Callable, Generator +from typing import Annotated + +import pytest +from fastapi import Depends + +from fastapi_injectable.concurrency import run_coroutine_sync +from fastapi_injectable.decorator import injectable +from fastapi_injectable.scope import injectable_scope +from fastapi_injectable.util import cleanup_all_exit_stacks, cleanup_exit_stack_of_func + + +class FakeConnection: + """Tracks which teardown branch of its provider generator ran.""" + + def __init__(self) -> None: + self.committed = False + self.rolled_back = False + self.closed = False + self.seen_exception: BaseException | None = None + + +def _make_async_provider(conn: FakeConnection) -> Callable[[], AsyncGenerator[FakeConnection, None]]: + async def get_connection() -> AsyncGenerator[FakeConnection, None]: + try: + yield conn + except Exception as exc: + conn.rolled_back = True + conn.seen_exception = exc + raise + else: + conn.committed = True + finally: + conn.closed = True + + return get_connection + + +def _make_sync_provider(conn: FakeConnection) -> Callable[[], Generator[FakeConnection, None, None]]: + def get_connection() -> Generator[FakeConnection, None, None]: + try: + yield conn + except Exception as exc: + conn.rolled_back = True + conn.seen_exception = exc + raise + else: + conn.committed = True + finally: + conn.closed = True + + return get_connection + + +@pytest.fixture(autouse=True) +async def _clean_global_stacks() -> AsyncGenerator[None, None]: + await cleanup_all_exit_stacks() + yield + await cleanup_all_exit_stacks() + + +async def test_cleanup_all_exit_stacks_with_exc_runs_rollback_branch() -> None: + """The issue's exact repro: rollback (not commit) runs when cleanup gets the exception.""" + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + caught: BaseException | None = None + try: + await do_work() # type: ignore[call-arg] + except ValueError as exc: + caught = exc + finally: + await cleanup_all_exit_stacks(exc=caught, raise_exception=True) + + assert caught is not None + assert conn.rolled_back is True + assert conn.committed is False + assert conn.closed is True + assert conn.seen_exception is caught + + +async def test_cleanup_exit_stack_of_func_with_exc_runs_rollback_branch() -> None: + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + caught: BaseException | None = None + try: + await do_work() # type: ignore[call-arg] + except ValueError as exc: + caught = exc + + await cleanup_exit_stack_of_func(do_work, exc=caught, raise_exception=True) + + assert conn.rolled_back is True + assert conn.committed is False + assert conn.closed is True + assert conn.seen_exception is caught + + +async def test_cleanup_without_exc_runs_commit_branch() -> None: + """Existing behavior is unchanged: no exception -> commit branch runs on cleanup.""" + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> FakeConnection: + return connection + + result = await do_work() # type: ignore[call-arg] + await cleanup_all_exit_stacks(raise_exception=True) + + assert result is conn + assert conn.committed is True + assert conn.rolled_back is False + assert conn.closed is True + assert conn.seen_exception is None + + +async def test_injectable_scope_forwards_exception_to_generator_dependencies() -> None: + """Inside ``injectable_scope`` the exception reaches dependencies with no extra plumbing.""" + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + with pytest.raises(ValueError, match="boom") as exc_info: + async with injectable_scope(): + await do_work() # type: ignore[call-arg] + + assert conn.rolled_back is True + assert conn.committed is False + assert conn.closed is True + assert conn.seen_exception is exc_info.value + + +async def test_injectable_scope_without_exception_runs_commit_branch() -> None: + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> FakeConnection: + return connection + + async with injectable_scope(): + result = await do_work() # type: ignore[call-arg] + + assert result is conn + assert conn.committed is True + assert conn.rolled_back is False + assert conn.closed is True + + +async def test_exception_reaches_sync_generator_dependency() -> None: + """Sync generator dependencies (run through FastAPI's threadpool CM) also see the exception.""" + conn = FakeConnection() + get_connection = _make_sync_provider(conn) + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + caught: BaseException | None = None + try: + await do_work() # type: ignore[call-arg] + except ValueError as exc: + caught = exc + + await cleanup_all_exit_stacks(exc=caught, raise_exception=True) + + assert conn.rolled_back is True + assert conn.committed is False + assert conn.closed is True + assert conn.seen_exception is caught + + +def test_sync_entrypoint_cleanup_with_exc_runs_rollback_branch() -> None: + """The whole flow from sync code: sync wrapper + run_coroutine_sync cleanup.""" + conn = FakeConnection() + get_connection = _make_async_provider(conn) + + @injectable + def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + caught: BaseException | None = None + try: + do_work() # type: ignore[call-arg] + except ValueError as exc: + caught = exc + + run_coroutine_sync(cleanup_all_exit_stacks(exc=caught, raise_exception=True)) + + assert conn.rolled_back is True + assert conn.committed is False + assert conn.closed is True + assert conn.seen_exception is caught + + +async def test_dependency_that_swallows_exception_is_supported() -> None: + """A generator that handles the exception without re-raising still cleans up fine.""" + conn = FakeConnection() + + async def get_connection() -> AsyncGenerator[FakeConnection, None]: + try: + yield conn + except Exception as exc: # noqa: BLE001 + conn.rolled_back = True + conn.seen_exception = exc + finally: + conn.closed = True + + @injectable + async def do_work(connection: Annotated[FakeConnection, Depends(get_connection)]) -> None: + msg = "boom" + raise ValueError(msg) + + caught: BaseException | None = None + try: + await do_work() # type: ignore[call-arg] + except ValueError as exc: + caught = exc + + await cleanup_all_exit_stacks(exc=caught, raise_exception=True) + + assert conn.rolled_back is True + assert conn.closed is True + assert conn.seen_exception is caught diff --git a/test/test_scope.py b/test/test_scope.py index b8fc940..89d8ab0 100644 --- a/test/test_scope.py +++ b/test/test_scope.py @@ -217,13 +217,25 @@ def test_public_api_exports_scope_symbols() -> None: assert exported_factory is injectable_scope -async def test_exception_inside_scope_still_cleans_up() -> None: +async def test_exception_inside_scope_still_cleans_up_and_reaches_dependency() -> None: + """FastAPI parity (issue #255): the in-flight exception reaches generator dependencies. + + The exception is thrown into each generator at its ``yield``, so ``except``/rollback + branches run. Teardown that must always run belongs in ``finally`` -- the same + contract FastAPI documents for ``Depends`` with ``yield`` in a failing request. + """ captured: dict[str, Mayor] = {} + seen: dict[str, BaseException] = {} async def get_mayor() -> AsyncGenerator[Mayor, None]: mayor = Mayor() - yield mayor - mayor.cleanup() + try: + yield mayor + except ValueError as exc: + seen["exc"] = exc + raise + finally: + mayor.cleanup() with pytest.raises(ValueError, match="boom"): # noqa: PT012 async with injectable_scope(): @@ -233,6 +245,7 @@ async def get_mayor() -> AsyncGenerator[Mayor, None]: raise ValueError(msg) assert captured["m"]._is_cleaned_up is True + assert isinstance(seen["exc"], ValueError) assert _current_scope.get() is None diff --git a/test/test_util.py b/test/test_util.py index c06ca37..d6bd46b 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -57,7 +57,7 @@ def func() -> None: return None await cleanup_exit_stack_of_func(func) - mock_async_exit_stack_manager.cleanup_stack.assert_awaited_once_with(func, raise_exception=False) + mock_async_exit_stack_manager.cleanup_stack.assert_awaited_once_with(func, raise_exception=False, exc=None) async def test_cleanup_all_exit_stacks(mock_async_exit_stack_manager: Mock) -> None: From ec6334d64edc008bcb9d81cc576a8c07410a246b Mon Sep 17 00:00:00 2001 From: JasperSui Date: Sun, 16 Aug 2026 15:39:47 +0800 Subject: [PATCH 2/2] fix: assign nox.options.sessions as a list for latest nox type stubs Latest nox annotates nox.options.sessions as 'list[str] | None'; the tuple assignment fails the noxfile mypy step in CI (nox is installed unpinned via pipx there). --- noxfile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/noxfile.py b/noxfile.py index 3c439c6..9e2aee6 100755 --- a/noxfile.py +++ b/noxfile.py @@ -12,12 +12,12 @@ python_versions_without_free_threaded = [version for version in python_versions if not version.endswith("t")] latest_python_version = python_versions_without_free_threaded[-1] nox.needs_version = ">= 2025.05.01" -nox.options.sessions = ( +nox.options.sessions = [ "pre-commit", "mypy", "tests", "docs-build", -) +] nox.options.default_venv_backend = "uv"