fix(proxy): deterministically close upstream streams for /v1/responses and /v1/messages - #36272
Conversation
…s and /v1/messages The proxy's streaming cleanup closes the upstream response only when the stream object exposes aclose. LiteLLMCompletionStreamingIterator and BaseResponsesAPIStreamingIterator had none, and the /v1/messages passthrough generator never closed its httpx response, so releasing the provider connection depended on GC timing instead of an explicit close. The chat completions route got the explicit contract in BerriAI#30245; this extends it to the two remaining streaming routes BaseResponsesAPIStreamingIterator gains a null-safe aclose (some subclasses bypass its constructor and carry no response), the bridge iterator delegates to its CustomStreamWrapper, and chunk_processor closes the upstream response in its finally after spend logging is scheduled Fixes BerriAI#36123
Greptile SummaryThis PR adds deterministic asynchronous cleanup for Responses API and Anthropic passthrough streams, including the previously reported MCP wrapper path. One MCP lifecycle gap remains:
Confidence Score: 4/5The PR is not yet safe to merge because MCP follow-up rounds can discard an upstream stream without explicitly closing it. MCP auto-execution replaces base_iterator immediately after a completed event, while aclose only reaches the replacement, so an earlier round's upstream response can remain open until garbage collection. Files Needing Attention: litellm/responses/mcp/mcp_streaming_iterator.py
|
| Filename | Overview |
|---|---|
| litellm/responses/mcp/mcp_streaming_iterator.py | Adds current-iterator cleanup, but follow-up rounds overwrite the previous iterator without closing it. |
| litellm/responses/streaming_iterator.py | Adds null-safe closure of the underlying httpx response. |
| litellm/responses/litellm_completion_transformation/streaming_iterator.py | Delegates cleanup to the chat-completion stream wrapper. |
| litellm/proxy/pass_through_endpoints/streaming_handler.py | Closes passthrough upstream responses in generator teardown without masking cleanup. |
| tests/test_litellm/responses/mcp/test_mcp_streaming_iterator.py | Covers closing the currently stored MCP iterator but not replacement across follow-up rounds. |
| tests/test_litellm/proxy/pass_through_endpoints/test_streaming_handler.py | Covers disconnect, natural completion, and Anthropic wrapper cleanup. |
| tests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_streaming_iterator.py | Verifies closure reaches the wrapped completion stream. |
| tests/test_litellm/responses/test_streaming_iterator.py | Verifies underlying httpx response closure and the response-less no-op path. |
Comments Outside Diff (1)
-
litellm/responses/mcp/mcp_streaming_iterator.py, line 815 (link)Previous MCP stream remains open
When MCP auto-execution starts a follow-up request after
response.completed, this assignment replaces the previousbase_iteratorwithout closing it. A later disconnect only closes the replacement iterator, leaving the previous upstream connection open until garbage collection.
Reviews (2): Last reviewed commit: "fix(responses): propagate aclose through..." | Re-trigger Greptile
… and type test helpers Greptile review: the MCP wrapper stores its upstream-owning iterator in base_iterator, so the inherited response-based aclose was a no-op there. Delegate to the base iterator's aclose when it has one. Also add the missing parameter and return annotations on the new passthrough test helpers
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
On the follow-up round note: base_iterator is only replaced after StopAsyncIteration, so the discarded stream is already exhausted and its connection released |
TLDR
Problem this solves:
How it solves it:
User Flow
Before: a developer streaming against a self-hosted backend cancels a request, but the backend keeps generating until the response is complete
"stream": truefor a model served by their vLLM boxAfter: cancelling frees the backend within about a second
"stream": trueThe same before and after applies to POST http://localhost:4000/v1/responses with
"stream": true. Chat completions was already covered by #30245 and is unchanged hereRelevant issues
Fixes #36123
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Setup: live proxy (
python litellm/proxy/proxy_cli.py --config <config> --port 4000) in front of a local SSE upstream that emits one chunk per second for 60 seconds and logs the moment its client socket dies. The upstream stands in for a self-hosted backend because this run had no provider credentials to spend; the observed signal, the upstream socket closing, is exactly what #36123 reports vLLM never seeing. No LiteLLM code is mocked, the requests below are what a customer sendsAfter the fix (commit a4ce0f2), both endpoints, curl killed 3 seconds in:
Upstream log, close arrives about one second after each curl dies instead of 60 chunks later:
Honest caveat on the before run (commit e24a914): in this small local repro the connection also closed within about a second, because with nothing else holding references CPython immediately collects the abandoned stream objects and the transport closes on finalization. The leak in #36123 shows up when that collection does not happen promptly, for example when the response object is retained for the request's lifetime under load. What this PR changes is that the close no longer depends on collection timing at all: cleanup now invokes an explicit close on both routes, which is the same guarantee #30245 gave chat completions. With the fix reverted, the new regression tests fail with
AttributeError: 'LiteLLMCompletionStreamingIterator' object has no attribute 'aclose'and with the upstream response left open in the passthrough generatorFor an operator with a vLLM or sglang backend, the two curl commands above (pointed at a real model, cancelled mid-stream) plus the backend's abort log or nvidia-smi are the full verification
Type
🐛 Bug Fix
Changes
BaseResponsesAPIStreamingIteratorgains a null-safeaclosethat closes the httpx response it holds; null-safe because two subclasses bypass its constructor and carry no response. The bridge iterator used when /v1/responses is served by a chat-completions model overrides it to close its stream wrapper, which releases the provider connection through the #30245 machinery. The /v1/messages passthrough generator now closes its upstream response in itsfinally, after spend logging is scheduled, so partial-usage billing on disconnect is untouched. The MCP-enhanced wrapper keeps its upstream-owning iterator in a separate field, so it overrides the shared close to delegate there (flagged by Greptile). The proxy's existing streaming cleanup and the router's fallback cleanup both already probe foraclose; they now find one on these routesTests: regression coverage in
tests/test_litellm/proxy/pass_through_endpoints/test_streaming_handler.py(new, mirrors its module),tests/test_litellm/responses/test_streaming_iterator.py(extended), andtests/test_litellm/responses/litellm_completion_transformation/test_litellm_completion_streaming_iterator.py(new; descriptive name because the directory is not a package and the mirrored name collides with an existing file). Disconnect and natural-completion paths are both pinned, plus the no-response subclass case and the MCP wrapper delegation intests/test_litellm/responses/mcp/test_mcp_streaming_iterator.pyFinal Attestation