🐛 Set X-Accel-Buffering: no on JSONL streaming responses - #15813
torrresagus wants to merge 3 commits into
Conversation
|
Hello @torrresagus, thanks for the PR! I understand the point, but I'm not fully convinced yet. JSONL isn't 1 to 1 "SSE but in JSON", it's also used for plain bulk/streamed exports where caching can be legitimate. So I'd split the two headers:
In any case the picked behaviour should be clearly pointed out in the docs. |
|
Thanks for the careful review @luzzodev — that's a fair distinction, and you've convinced me. You're right that JSONL isn't semantically "SSE in JSON": it's also a transport for bulk/streamed exports where caching can be legitimate. And since
So I'll drop |
📝 Docs previewLast commit ac21385 at: https://153935d9.fastapitiangolo.pages.dev Modified Pages |
|
LGTM! |
|
Thanks for the review @luzzodev! |
mohamedtaqysalmi
left a comment
mohamedtaqysalmi
left a comment
There was a problem hiding this comment.
Mirroring the SSE anti-buffering behavior for JSONL is the right fix behind Nginx, and intentionally not imposing Cache-Control preserves legitimate export caching — the new docs section explains that well.
Small inconsistency: the PR title mentions both Cache-Control and X-Accel-Buffering, but
outing.py only sets X-Accel-Buffering. Either add the header for SSE parity or tighten the title/description so reviewers aren't looking for a header that isn't in the diff.
torrresagus
changed the title
X-Accel-Buffering: no on JSONL streaming responses
|
Good catch @mohamedtaqysalmi — the title and description were stale after I dropped |
|
@tiangolo Let me know if you need anything else from me. |
| # For Nginx proxies to not buffer the streamed response. | ||
| # Caching headers are intentionally left to the user, as JSONL | ||
| # is also used for bulk exports where caching can be legitimate. | ||
| response.headers["X-Accel-Buffering"] = "no" |
There was a problem hiding this comment.
since this is set before raw.extend, a user who sets X-Accel-Buffering on their own response ends up with two copies of the header instead of overriding it. intentional that the endpoint can't turn buffering back on?
There was a problem hiding this comment.
Good catch, you're right. raw.extend appends, so a user-set X-Accel-Buffering ended up as a second header instead of overriding it.
It wasn't a deliberate "you can't turn buffering back on" decision, I had just mirrored the SSE branch a few lines above, which does the same thing with Cache-Control and X-Accel-Buffering.
Fixed in fe13265 by setting the header after the extend:
response.headers.raw.extend(solved_result.response.headers.raw)
response.headers.setdefault("X-Accel-Buffering", "no")Now the default is still no, but a path operation that sets the header itself wins, and there's only one copy of it. Added a test for both cases in tests/test_stream_json_lines_headers.py and a line in the docs.
One note while testing it: the header has to be set from a dependency, not from inside the generator body, since the headers are already sent by the time the first item is yielded.
I left the SSE branch as is, changing its behavior feels out of scope for this PR, but happy to do it here too if a maintainer prefers to keep both consistent.
Fixes parity gap between SSE and JSONL streaming branches.
The SSE branch in get_request_handler() already sets:
response.headers["X-Accel-Buffering"] = "no"
to prevent Nginx (and other buffering proxies) from buffering the
event stream. Without this header, proxies configured with
`proxy_buffering on` (the Nginx default) will hold chunks until
their buffer fills before flushing to the client — causing large
latency spikes or no visible streaming at all.
The JSONL branch streams in an identical way (incremental line-by-line
yield, chunked transfer encoding), yet lacked this header. The result:
@app.get("/stream")
async def stream() -> AsyncIterable[Item]:
for item in items:
yield item
await asyncio.sleep(0.1)
Behind Nginx with proxy_buffering on:
• SSE: client sees events as they arrive ✔
• JSONL: client receives all lines in one flush ✘
Fix: apply the same header to the JSONL StreamingResponse.
Cache-Control is intentionally NOT set on JSONL — unlike SSE, JSONL
is also used for bulk/streamed file exports where caching may be
legitimate. Users who need no-cache on JSONL can set it via a
Response dependency parameter.
Also add a TODO comment in the SSE keepalive code noting that the two
branches (SSE + JSONL) now share the same proxy-buffering contract and
should be kept in sync.
Relates to: fastapi#15813
Relates to: fastapi#15794
This comment was marked as resolved.
This comment was marked as resolved.
github-actions
Bot
added
the
conflicts
Server-Sent Events responses set `Cache-Control: no-cache` and `X-Accel-Buffering: no` so proxies (e.g. Nginx) deliver events incrementally instead of buffering the whole response. JSONL streaming responses are incremental in the same way, but were missing these headers, so a buffering proxy would hold back the lines and defeat the streaming. Set the same headers on the JSONL response for consistency with SSE.
…g, document behavior Per review feedback: JSON Lines is not only a live event stream like SSE, it's also used for bulk/streamed exports where caching can be legitimate. `Cache-Control: no-cache` forces revalidation and would take away the `max-age` "serve from cache" option from those endpoints, so it shouldn't be imposed by default. Keep only `X-Accel-Buffering: no` (buffering defeats incremental streaming in every case) and leave caching headers to the user. Document the proxy buffering behavior in the JSON Lines tutorial.
fe13265 to
dcee75e
Compare
github-actions
Bot
removed
the
conflicts
|
I tried reproducing this locally with FastAPI behind Nginx on Windows. I configured proxy_buffering on and used a StreamingResponse producing JSONL chunks at 1-second intervals. The client still received each chunk incrementally rather than receiving them all at once. I also noticed that the FastAPI version I tested already sets X-Accel-Buffering: no for the response in routing.py. Because of this, I'm wondering if the behavior described in this PR has already been addressed in the version I tested. Could you clarify which FastAPI version/commit was used for the original reproduction? |
This branch has not been deployed
5 participants
What's wrong
FastAPI's Server-Sent Events responses set
X-Accel-Buffering: noso streaming survives a buffering proxy (theis_sse_streambranch infastapi/routing.py, added in #15030):The JSON Lines streaming responses (the
yield-basedapplication/jsonlstreaming added in #15022, theis_json_streambranch) stream items incrementally in the same way, but don't set it.Behind a proxy that buffers by default (e.g. Nginx with
proxy_buffering on), the JSONL lines are buffered and flushed together, which defeats the streaming: the client receives the200immediately, but the lines don't arrive until the proxy buffer fills. SSE isn't affected because it already sendsX-Accel-Buffering: no. The two features landed in separate PRs (SSE #15030, JSONL #15022), which is likely why only the SSE path got the header.Change
Set
X-Accel-Buffering: noon the JSONLStreamingResponse, so incremental streaming works through a buffering proxy the same way SSE does.Cache-Controlis intentionally not set: unlike SSE, JSON Lines is also used for bulk/streamed exports where caching can be legitimate, so the caching policy is left to the user (it can be set on theResponse). This is documented in the JSON Lines tutorial. (Thanks @luzzodev for the review that clarified this.)Tests
test_stream_disables_proxy_bufferingassertsx-accel-buffering: noacross the four JSONL streaming tutorial variants (parametrized like the existing SSE streaming tests).Context
Discussed first in #15794, where the same failure mode was confirmed independently, including a report behind Nginx in production.