Skip to content

Comments

Add session cleanup middleware to FAB FastAPI app#61480

Merged
shahar1 merged 2 commits intoapache:mainfrom
yulit0738:fix/fab-fastapi-session-cleanup
Feb 10, 2026
Merged

Add session cleanup middleware to FAB FastAPI app#61480
shahar1 merged 2 commits intoapache:mainfrom
yulit0738:fix/fab-fastapi-session-cleanup

Conversation

@yulit0738
Copy link
Contributor

Summary

Fix PendingRollbackError on FAB admin pages (/auth/users/list/, /auth/roles/list/) by adding session cleanup middleware to the FAB auth manager's FastAPI app.

Problem

FAB auth manager's FastAPI app has this route structure:

  • /token, /logout → FastAPI routes
  • /users/*, /roles/* → FastAPI API routes
  • /* (catch-all) → WSGIMiddleware → Flask App (FAB views)
    The Flask AppBuilder views (e.g., /users/list/, /roles/list/) use settings.Session (SQLAlchemy scoped_session) for database access. In a native Flask app, teardown_appcontext automatically calls Session.remove() after each request. However, when Flask is mounted via WSGIMiddleware inside FastAPI, Flask's teardown hooks do not trigger.
    This causes sessions to remain in "idle in transaction" state. When the database connection times out (e.g., PostgreSQL's idle_in_transaction_session_timeout), subsequent requests that reuse the invalidated session raise PendingRollbackError (500 error).

Steps to reproduce

  1. Deploy Airflow 3.x with FAB auth manager and PostgreSQL
  2. Access /auth/users/list/ or /auth/roles/list/
  3. Wait for PostgreSQL to timeout idle transactions
  4. Refresh the page → 500 PendingRollbackError

Solution

Add a FastAPI HTTP middleware in get_fastapi_app() that calls Session.remove() in the finally block after every request. This ensures session cleanup for all requests, including those forwarded to Flask via WSGI — closing the gap left by the missing teardown_appcontext.

Why this is safe

  • settings.Session is a scoped_session (thread-local) — remove() only affects the current thread
  • The finally block runs after the response is fully constructed, so it doesn't interfere with request processing
  • All 585 existing FAB provider tests pass with this change

Testing

  • Added TestFabAuthManagerSessionCleanup with 2 tests:
    • test_session_cleanup_middleware_on_wsgi_route: Verifies Session.remove() is called after WSGI (Flask) route requests — the exact scenario that caused the bug
    • test_session_cleanup_middleware_on_fastapi_route: Verifies cleanup also runs after FastAPI route requests
  • Verified fix resolves the issue in a production environment with PostgreSQL (idle transactions drop to 0 after patch)

AI Disclosure

This PR was developed with AI assistance (Claude).

@yulit0738 yulit0738 requested a review from vincbeck as a code owner February 5, 2026 03:53
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 5, 2026

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contributors' Guide (https://github.com/apache/airflow/blob/main/contributing-docs/README.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our prek-hooks will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
  • Always keep your Pull Requests rebased, otherwise your build might fail due to changes not related to your commits.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Add HTTP middleware to get_fastapi_app() that calls Session.remove()
after each request. This prevents PendingRollbackError that can occur
when database connections time out while transactions are in
'idle in transaction' state.

Unlike Flask's teardown_appcontext, FastAPI doesn't automatically
clean up SQLAlchemy scoped sessions, which can leave transactions
open and eventually cause session invalidation errors.
@shahar1 shahar1 force-pushed the fix/fab-fastapi-session-cleanup branch from ed041a1 to d89c8be Compare February 10, 2026 16:39
@shahar1 shahar1 changed the title fix(providers/fab): add session cleanup middleware to FAB FastAPI app Add session cleanup middleware to FAB FastAPI app Feb 10, 2026
@shahar1 shahar1 merged commit b2b089c into apache:main Feb 10, 2026
86 checks passed
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 10, 2026

Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions.

@potiuk
Copy link
Member

potiuk commented Feb 15, 2026

NIce one ! Thanks @yulit0738 !

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.

4 participants