Repository navigation
fix(router): hold Responses lifecycle events until output so a pre-output fallback announces one response - #43238
Conversation
…tput fallback announces one response
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if _responses_stream_holds_event(item, len(held_lifecycle_events)): | ||
| held_lifecycle_events = (*held_lifecycle_events, item) | ||
| continue |
There was a problem hiding this comment.
response.created but takes a long time to produce output, this holds the event while the proxy waits for its first chunk. With keepalive disabled, the client receives no SSE data during that gap and can time out despite the request still progressing
There was a problem hiding this comment.
The client's longest silence grows only by the provider's accept latency: before this PR it already waited time-to-first-output between in_progress and the first output event
There was a problem hiding this comment.
That’s true for the interval after response.in_progress: the provider’s time to first output is unchanged. The behavioral difference is that the client now receives no event during that interval—both response.created and response.in_progress are held—so a client-side first-event/read timeout can cover the entire accept-to-output period. Previously, those lifecycle events arrived before output and could reset or satisfy such a timeout.
So the delay itself is not added to provider TTFO, but suppressing the earlier events remains a compatibility tradeoff for clients with an event timeout.
There was a problem hiding this comment.
The silence becomes accept latency plus time-to-first-output instead of the larger of the two, and the /v1/messages wrapper already holds message_start the same way
There was a problem hiding this comment.
You're right — I misstated the timing. With lifecycle events held, silence from request start can span accept latency plus time-to-first-output; previously, the first lifecycle event arrived after accept latency, so the longest individual silence was the larger of those intervals.
The /v1/messages wrapper is a relevant precedent, and this is an explicitly documented compatibility tradeoff of associating lifecycle events with the response that ultimately completes. I withdraw the timeout finding; the remaining behavior is intentional and covered by the stated design.
| def _responses_stream_holds_event(item: object, held_event_count: int) -> bool: | ||
| from litellm.responses.streaming_iterator import PRE_OUTPUT_LIFECYCLE_EVENT_TYPES | ||
|
|
||
| if held_event_count >= MAX_HELD_PRE_OUTPUT_RESPONSES_EVENTS: |
There was a problem hiding this comment.
response.created then gives the client two response IDs. The cap test uses a fallback without that event, so it misses this case
There was a problem hiding this comment.
The cap is a memory bound: providers send two or three lifecycle events before output, and the Messages wrapper commits at its cap too
There was a problem hiding this comment.
That’s fair. The cap is a defensive memory bound, not an expected lifecycle threshold: a normal Responses stream emits only two or three pre-output lifecycle events, so reaching 200 would require pathological provider behavior. Given the existing /v1/messages precedent and the explicit caveat in the PR, I’m withdrawing this as a blocking finding; the cap behavior is acceptable here.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…nd test the hold helpers directly
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…m fails before its first event
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 11387cc. Configure here.
Upstream added 185 commits since the previous sync, including the owned `Client` pool refactor (BerriAI#43245), the Responses lifecycle-event hold (BerriAI#43238) and the driver preflight split (BerriAI#43259). One conflict, in the `get_litellm_params` import block in `litellm/main.py`: upstream added `InvalidControlOption`, `parse_control_options` and `with_control_options` where this branch added `REASONING_TRANSPORT_KWARGS_KEYS`. Both stay, in the order the import sorter wants. Reasoning normalization still runs ahead of the configured system-message ordering, and the net diff against upstream is the same 16 paths and 1005 added lines as before the sync. Verified on this merge: 1574 passed across the relocated hosted_vllm, openai chat and litellm_params files, 32 passed and 8 skipped in the router suite, 32 in caching, 308 in the Responses-to-Chat bridge, 337 in model management endpoints. ruff_strict_gate, type_discipline_gate and test_quality_gate pass with --base upstream/main.
2 participants
TLDR
Problem this solves:
/v1/responsesstreams tworesponse.createdevents with different idsHow it solves it:
response.created,response.in_progress, andresponse.queueduntil the first output event/v1/messagesfallback wrapperIntentional product change: on a native
/v1/responsesstream,response.createdandresponse.in_progressnow arrive together with the first output event instead of a few hundred milliseconds before it. Clients lose the early announcement, never the events, and time to first token is unchangedUser Flow
Before: a developer streaming
/v1/responsesthrough a proxy with fallbacks seesresponse.createdtwice, and the first id never completesprimaryandbackupwithrouter_settings.fallbacks: [{primary: [backup]}]and restarts the proxy"stream": truethrough openai-python and iterates the eventsprimarystreamsresponse.created(id A, seq 0) andresponse.in_progress(seq 1), then fails before any outputbackupand the client receives itsresponse.created(id B, seq 0 again),response.in_progress, the output events, andresponse.completed(id B)response.createdevents with two ids, sequence numbers that restart at 0, and an id A that never completesAfter: the same developer sees exactly one
response.created, the one whose id completesprimaryandbackupwithrouter_settings.fallbacks: [{primary: [backup]}]and restarts the proxy"stream": truethrough openai-python and iterates the eventsprimarystreamsresponse.createdandresponse.in_progress, then fails before any output; nothing has reached the client yetbackupand the client receives oneresponse.created(id B, seq 0), oneresponse.in_progress(seq 1), the output events, andresponse.completed(id B), with contiguous sequence numbersDesign decisions
The router's
/v1/responsesfallback wrapper now holds the primary stream's pre-output lifecycle events (response.created,response.in_progress,response.queued, the same boundarylitellm/responses/streaming_iterator.pyalready draws, so its_PRE_OUTPUT_LIFECYCLE_EVENT_TYPESconstant is made public and shared instead of copied) until the first output-bearing event and forwards them then. A mid-stream fallback error before that point drops them once the fallback stream yields its first event, so the client only ever sees the lifecycle events of the response that completes. A fallback that fails before yielding anything announced no response of its own, so the held pair is replayed ahead of the error and the failure reaches the client under the id it belongs to, the same wire as when no fallback was attempted (case 7 of the proof, raised by Bugbot on the first tip). Three alternatives were weighed and rejected: forwarding both pairs as today (two ids, sequence numbers restarting at 0); forwarding the primary's pair and suppressing the fallback's (the client then holds an id that never completes and cannot chain throughprevious_response_id); and rewriting the fallback's ids to the primary's (the completed id would not match the response the provider stored, soprevious_response_idandGET /v1/responses/{id}would break)The cost of the chosen option is the delay named under the TLDR. Every non-fallback exit keeps the wire unchanged: when no fallback lands (fallbacks disabled, every fallback failing, or a non-fallback error) the held events are flushed before the error propagates, a stream that ends without output flushes them at the end, and a stream that already produced output holds nothing, so a later error is never replayed (case 4 of the proof). The hold is capped at
MAX_HELD_PRE_OUTPUT_RESPONSES_EVENTS = 200, mirroringMAX_BUFFERED_PRE_CONTENT_ANTHROPIC_CHUNKSon the/v1/messagesfallback wrapper, which is the precedent this follows; past the cap the events commit and a later fallback replays a pair again, far above the three lifecycle events a provider sends. The fallback attempt moved into_aresponses_fallback_attempt, the same split the Anthropic wrapper already has, to keep_aresponses_streaming_iteratorunder the C901 budget, and the fallback stream is wrapped incontextlib.aclosingso its close is deterministic when the consumer stops early. The wrapper's two close guards catchExceptionrather thanBaseException, so a cancellation raised while a stream is being closed propagates instead of being logged and swallowed. No open PR was worth building on: #42958 and #35018 touch neither the router's responses fallback wrapper nor this symptomLinear ticket
Resolves LIT-8625
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@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
Both legs run the same rig and differ only in the commit the proxy was booted from. The upstream is a fault-injecting chunked SSE server (
fault_upstream.pyon 127.0.0.1:42838, raw asyncio, no framework) that picks its behavior from the request's model name:failed-eventsendsresponse.createdandresponse.in_progressand thenevent: response.failedwith aserver_error;drop-before-outputsends those two events and closes the socket mid chunked body;abort-before-outputsends them and then a TCP RST;drop-after-partialsends them plus two text deltas and then closes the socket mid body;healthystreams a full answer ending inresponse.completed;close-before-any-eventanswers 200 with SSE headers and closes the socket before its first frame.backupis a realopenai/gpt-5.4-minideployment, so every fallback that fires spends real money on OpenAI;backup-emptyis the zero-frame mode, standing in for a fallback deployment that dies before it announces anythingProxy config shared by both legs (
config.yaml):Each leg boots its own worktree and venv with
python litellm/proxy/proxy_cli.py --config config.yaml --port <random free port> --num_workers 2. Cases 1 to 5 and 7 run the customer's client, openai-python 2.33.0 (client_stream.py:OpenAI(base_url=<proxy>, api_key=$LITELLM_MASTER_KEY, max_retries=0, timeout=120).responses.create(model=<name>, input="Say hello in five words.", stream=True), printing every event with its sequence number, the response id on lifecycle events, and a finalRESULT:line saying how the iteration ended). Case 6 sends the same request over raw curl and pulls the event type, response id, and sequence number out of every SSE event. Case 7's before leg was captured on the/live-pr-riskrig, the same config with a request recorder in front ofbackup, which that case never reachesBefore (8b68c3c)
Proxy booted from
8b68c3cd09on port 20640 with--num_workers 21. primary-failed-event: provider sends response.failed before any output (the fallback path that predates PR #43133)
Run the client
Observed output
2. primary-drop: socket closed after response.in_progress, before any output
Run the client
Observed output
3. primary-abort: TCP reset after response.in_progress, before any output
Run the client
Observed output
4. primary-drop-after-partial: two text deltas, then the socket closes (guard: nothing is replayed once output started)
Run the client
Observed output
5. primary-healthy: full stream, no fallback (control)
Run the client
Observed output
6. primary-drop over raw curl: event types, response ids, and sequence numbers pulled out of the SSE body
Run the request
Observed output
7. primary-drop-to-empty-backup: socket closed before any output, and the fallback deployment closes its socket before its first event (guard: the failure is reported under the announced id)
Run the client
Observed output
After (11387cc)
Proxy booted from
11387ccfedon port 23595 with--num_workers 21. primary-failed-event: provider sends response.failed before any output (the fallback path that predates PR #43133)
Run the client
Observed output
2. primary-drop: socket closed after response.in_progress, before any output
Run the client
Observed output
3. primary-abort: TCP reset after response.in_progress, before any output
Run the client
Observed output
4. primary-drop-after-partial: two text deltas, then the socket closes (guard: nothing is replayed once output started)
Run the client
Observed output
5. primary-healthy: full stream, no fallback (control)
Run the client
Observed output
6. primary-drop over raw curl: event types, response ids, and sequence numbers pulled out of the SSE body
Run the request
Observed output
7. primary-drop-to-empty-backup: socket closed before any output, and the fallback deployment closes its socket before its first event (guard: the failure is reported under the announced id)
Run the client
Observed output
The same request at 3399cb8, the tip Bugbot reviewed, before 11387cc: the primary's announcement was dropped and the proxy synthesized a terminal event under a fresh id
Observations from the run:
response.failedon both legs; leaves aloneresponse.completednames the fallback's model, not the alias; pre-existing, leaves aloneresponse.failedunder the primary's id on both legs; at 3399cb8 it was a loneresponse.failedunder a synthesized id, fixed in 11387ccType
🐛 Bug Fix
Caveats (if any)
Low
Exceptioninstead ofBaseException(CodeQL), so a cancellation raised inside a stream'sacloseduring teardown propagates instead of being logged at debug; no live case exercises that pathlocal_testing_part1fails onlytest_get_model_info_bedrock_cross_region_capability_parity(job 2218680), the same single failure as main pipelines 90403 (job 2216868) and 90441 (job 2218778);langfuse_logging_unit_teststimes out at its Run tests step (job 2218658) as main's jobs 2216878 and 2218773 do;using_litellm_on_windowstimed out at Install Python (job 2218649) before any repo code ran, and main's runs time out on the same executor at Install Dependencies (job 2216869) and at the MAX_PATH guard step (job 2216487);unit(job 2218663) has 34 failures, 32 of them in main 90441's 36 (job 2218776), the other two being one more instance of the MCP conftest teardowntask_done() called too many timeserror main hits on three sibling tests andtest_the_repo_as_it_stands_has_every_shard_child_assigned, which CircleCI runs on the branch head that predates test: move tests/test_litellm core utils, routing, responses, caching and rust_bridge into tests/unit #43199's shard move and which the tip'smisc / Run testsGitHub Actions run on the merge commit passes;proxy_store_model_in_db_testsfails onlytest_e2e_langfuse_callbacks_in_db(job 2218668), the same single failure as main's jobs 2216860 and 2218794;build_and_testfails the same twotest_implemented_namespace_case_modules_remain_importableOCR parity cases (job 2218684) as main's job 2216862;integration-extensionsended in a CircleCIinfrastructure_failwith its Run owned integration contracts step canceled and no test run (job 2218632), the same shape as main's job 2216848Final Attestation
The tests check the right things, including the edge cases, and regressions in the respective real-world customer use-cases are not possible after this PR
11387cc passes /live-pr-risk