fix: unwind exit stacks with in-flight exception details so generator dependencies can roll back - #259
Merged
Conversation
… 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).
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).
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #259 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 11 11
Lines 580 595 +15
Branches 69 72 +3
=========================================
+ Hits 580 595 +15
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #255
Problem
When a function decorated with
@injectableraises, generator dependencies never see the exception: exit stacks are always closed viaaclose()(i.e.__aexit__(None, None, None)), so a dependency'sexcept/rollback branch can never run — unlike a failing FastAPI request, which unwinds its exit stack with the exception details.There was also a deeper, silent layer to this: on fastapi>=0.121 generator dependencies are entered into the two inner request-scope stacks (
fastapi_inner_astack/fastapi_function_astack), and those were registered on the owning stack aspush_async_callback(stack.aclose)— stripping exception details even when the owning stack did unwind with them. This meantInjectableScope.__aexit__'s existing exception forwarding silently did nothing for generator dependencies.Fix
main.py— register the inner FastAPI stacks viapush_async_exitso exception details reach the stacks where generator teardowns actually live.util.py/async_exit_stack.py— new optionalexc=parameter oncleanup_exit_stack_of_func()/cleanup_all_exit_stacks()(plumbed throughAsyncExitStackManager): when provided, stacks unwind via__aexit__(type(exc), exc, exc.__traceback__), exactly as if the exception had propagated out of anasync withblock. A dependency re-raising the in-flight exception (except: rollback(); raise) is recognized as normal context-manager protocol, not reported as a cleanup failure; genuinely distinct teardown failures still surface asDependencyCleanupError.injectable_scope()— with (1) in place, an exception propagating out of theasync withblock now reaches generator dependencies automatically, giving full FastAPI parity with zero extra plumbing:Intentional semantic note
As in FastAPI, when an exception is delivered to a generator dependency, code placed after a bare
yield(notry/finally) does not run — teardown that must always run belongs infinally.test_exception_inside_scope_still_cleans_upwas updated to pin this contract (cleanup still always runs when written withfinally).Pre-existing bugs fixed along the way
concurrency.py): under the default"current"strategy, once the thread had no policy event loop set (e.g. after any pytest-asyncio test), everyrun_coroutine_synccall created a fresh throwaway loop without registering it. Dependencies were then resolved on loop A while cleanup ran on loop B, and the per-loop stack registry ([BUG] pytest hangs when using async_get_injected_obj #186) rightly refused to close A's stacks from B — silently skipping generator teardown. Reproducible onmain: running any async test file beforetest_integration.pybreaks 12+ sync integration tests. The created loop is now registered viaasyncio.set_event_loop(mirroring pre-3.12get_event_loopauto-create semantics), with a guard for closed policy loops.util.py):return valueinside the asyncgen loop now usescast("T2", value)— the previoustype: ignore[no-any-return]fails on the current mypy release (CI installs unpinned mypy).Verification
nox -s coverage: 100% statements + branchesnox -s mypy: clean;ruff check+ruff format --check: clean;nox -s docs-build: successtest/test_exit_stack_exception_unwind.py) cover: the issue's exact repro viacleanup_all_exit_stacks(exc=...)andcleanup_exit_stack_of_func(..., exc=...), automatic forwarding viainjectable_scope(), sync generator dependencies, the fully-sync entry point, a dependency that swallows the exception, and the unchanged no-exception commit path