Skip to content

Conversation

@cofin
Copy link
Member

@cofin cofin commented Dec 20, 2025

Summary

Fixes #647

This PR resolves the session lifecycle timing conflict between Starlette middleware and FastAPI's generator-based dependency cleanup (provide_service()). The issue was causing asyncpg connections to not be properly returned to the pool, resulting in:

  • InvalidRequestError: An invalid request was made.
  • SAWarning: The garbage collector is trying to clean up non-checked-in connection...

Root Cause

The middleware was closing sessions before FastAPI's generator dependency cleanup could run:

1. Request arrives
2. FastAPI resolves `provide_service()` dependency (generator)
3. Session created and stored in `request.state`
4. Generator yields service, suspends
5. Route handler executes
6. Response is generated
7. Middleware runs → session_handler() CLOSES SESSION ← Problem
8. FastAPI dependency cleanup resumes generator
9. Generator's `async with` tries to exit with closed session
10. asyncpg connection not properly returned → GC warning

Solution

  • Mark sessions as "generator-managed" when using provide_service()
  • Middleware stores response status but skips cleanup for generator-managed sessions
  • Generator handles commit/rollback/close in its finally block with access to the stored response status

Changes

File Changes
advanced_alchemy/extensions/fastapi/providers.py Add _should_commit_for_status() helper, update provide_service() to handle lifecycle
advanced_alchemy/extensions/starlette/config.py Update middleware dispatch to skip generator-managed sessions
tests/unit/test_extensions/test_fastapi/test_session_lifecycle.py 50 new tests covering session lifecycle scenarios

Test Plan

  • All 176 FastAPI unit tests pass
  • 50 new session lifecycle tests covering:
    • _should_commit_for_status() helper function
    • Commit strategies with autocommit, autocommit_include_redirect, and manual modes
    • Response status codes (2xx, 3xx, 4xx, 5xx)
    • Middleware skipping generator-managed sessions
    • Non-generator sessions still use middleware cleanup
    • Multiple configs with different session keys
  • Linting passes

@codecov-commenter
Copy link

codecov-commenter commented Dec 20, 2025

Codecov Report

❌ Patch coverage is 80.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.45%. Comparing base (947df81) to head (524dcdc).

Files with missing lines Patch % Lines
advanced_alchemy/extensions/fastapi/providers.py 80.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #648      +/-   ##
==========================================
+ Coverage   80.44%   80.45%   +0.01%     
==========================================
  Files          87       87              
  Lines        6453     6462       +9     
  Branches      838      841       +3     
==========================================
+ Hits         5191     5199       +8     
- Misses        998      999       +1     
  Partials      264      264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…ncies

Fixes litestar-org#647

The middleware was closing sessions before FastAPI's generator dependency
cleanup could run, causing asyncpg connections to not be properly returned
to the pool. This resulted in SAWarning about garbage collection of
non-checked-in connections.

Changes:
- Add _should_commit_for_status() helper function for commit logic
- Mark sessions as generator-managed when using provide_service()
- Skip middleware cleanup for generator-managed sessions
- Handle commit/rollback/close in generator finally block
- Store response status in request.state for generator access
@cofin cofin force-pushed the fix/fastapi-session-lifecycle-647 branch from 8c750b0 to 524dcdc Compare December 20, 2025 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: FastAPI session lifecycle bug causes asyncpg connection pool warnings

2 participants