Skip to content

🐛 Fix support for StreamingResponses with dependencies with yield or UploadFiles, close after the response is done#14099

Merged
tiangolo merged 28 commits into
masterfrom
yield
Sep 29, 2025
Merged

🐛 Fix support for StreamingResponses with dependencies with yield or UploadFiles, close after the response is done#14099
tiangolo merged 28 commits into
masterfrom
yield

Conversation

@tiangolo

@tiangolo tiangolo commented Sep 21, 2025

Copy link
Copy Markdown
Member

🐛 Fix support for StreamingResponses with dependencies with yield or UploadFiles, close after the response is done


There's (again) an AsyncExitStack middleware, now used mainly to close objects after the response is done.

There's also a new wrapper duplicating some logic in Starlette but creating an AsyncExitStack for the exit code in dependencies with yield, for regular responses and WebSockets as well.

The exit code of dependencies with yield is now executed after the response is done.

tiangolo and others added 19 commits September 3, 2025 17:26
…tion`) in dependencies with `yield` in the exit code, do not support them in background tasks (#10831)"

This reverts commit a4aa79e.
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@tiangolo tiangolo added the bug Something isn't working label Sep 21, 2025
@github-actions

Copy link
Copy Markdown
Contributor

@tiangolo tiangolo marked this pull request as ready for review September 21, 2025 18:58
@tiangolo tiangolo changed the title 🐛 Fix support for streaming responses with dependencies with yield, close after the response is done 🐛 Fix support for StreamingResponses with dependencies with yield or UploadFiles, close after the response is done Sep 21, 2025
@github-actions

Copy link
Copy Markdown
Contributor

@github-actions

Copy link
Copy Markdown
Contributor

@tiangolo tiangolo merged commit e329d78 into master Sep 29, 2025
53 of 54 checks passed
@tiangolo tiangolo deleted the yield branch September 29, 2025 03:29
nilslindemann added a commit to nilslindemann/fastapi that referenced this pull request Oct 1, 2025
YuriiMotov added a commit that referenced this pull request Oct 1, 2025
* Sync German docs with #13917

Plus a typo fix in tutorial/security/oauth2-jwt.md line 89.

* Sync german docs with #14099

---------

Co-authored-by: Motov Yurii <109919500+YuriiMotov@users.noreply.github.com>
@anton-ryzhov

Copy link
Copy Markdown

This PR also fixes #12508

@anton-ryzhov

Copy link
Copy Markdown

But introduces another bug as described here

eumemic added a commit to eumemic/aios that referenced this pull request May 22, 2026
## Summary

- **Bug:** The four SSE route handlers open a dedicated asyncpg
connection via `open_listen_for_*` BEFORE constructing
`EventSourceResponse` (preserves #376 — preflight failures must surface
as 503 before 200 OK). The wrapped generator owns
`subscription.terminate()` in `finally`. But if the request task is
cancelled in FastAPI's dispatch gap — between `response = await
handler(request)` and the subsequent `await response(scope, receive,
send)` — the async generator is **never started**, and Python's
async-generator semantics guarantee that an unstarted generator's
`finally` block does NOT execute on `aclose` or GC. The asyncpg
connection, its `LISTEN`, and the SSE subscriber advisory lock leak
until TCP keepalive reaps the backend (~2h). Server restart with N
concurrent in-flight SSE setups leaks up to N postgres backends.
- **Reachability:** narrow (server shutdown / fast client disconnect
striking the µs-scale dispatch window) but real and persistent.
- **Fix:** `make_sse_response(subscription, content, *, ping=15)` in
`api/sse.py` constructs the `EventSourceResponse` and registers
`weakref.finalize(response, subscription.terminate)`. The finalizer
fires on response GC — including the un-invoked path. Migrated the 4 SSE
route sites to use the helper.
- **Idempotency:** `terminate()` is synchronous (see `db/listen.py`
docstring) and idempotent at asyncpg's transport layer, so the duplicate
call in the normal path (generator `finally` + finalizer) is a no-op.
- **Design alternative considered:** FastAPI `Depends`-with-yield (one
mechanism via `AsyncExitStack`). Rejected after code review: (a)
`Depends` teardown's cancellation guarantee only stabilized in
[fastapi#14099](fastapi/fastapi#14099) (Sep
2024), vs `weakref.finalize`'s stdlib-since-3.4 stability; (b) the
refactor would scatter `SSE_PREFLIGHT_EXCEPTIONS` handling and
per-stream `detail` payloads across 4 dependency functions; (c) the two
cleanup hooks here operate on **disjoint object-graph states**
(started-then-disconnect vs never-started), not duplicating a single
guard.

## Test plan

- [x] mypy — clean (160 source files)
- [x] ruff check + format — clean
- [x] Unit suite — 1470 passed (new
`test_make_sse_response_terminates_subscription_when_response_uninvoked`
is red without the finalize, green with — confirmed)
- [x] Code-reviewer independently re-verified discriminating power
(removed `weakref.finalize`, observed test failure on the right
assertion) plus `Depends(yield)` cancellation behavior
- [ ] CI runs e2e/integration suite

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants