Repository navigation
fix(passthrough): relay upstream error streams without waiting for the log preview and keep passthrough normalization - #43056
devin-ai-integration[bot] wants to merge 33 commits into
Conversation
…is collected Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…sconnects mid-relay Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
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! |
…r instead of awaiting it in the relay Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…tes, disconnect burst and unchanged llm endpoint normalization Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…end so a delivered error always lands its row Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…gs separately for the upstream abort cell Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review latest head |
|
bugbot run |
… report tasks Once the preview budget is crossed and the report is dispatched, an upstream httpx.HTTPError is re-raised so the client still sees the truncated framing instead of a clean terminator. Report tasks now live in a module registry behind a semaphore and are drained with a timeout during proxy shutdown, and a client disconnect before upstream completion logs the preview byte count Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
@greptileai review latest head |
|
bugbot run |
…pend flushes Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…e error relay is abandoned before the response Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…ed upstream fails Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
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 5beef60. Configure here.
TLDR
Problem this solves:
normalized_errorto429_BUDGET_EXCEEDEDHow it solves it:
normalize_errorredact_secretsbefore logs, hooks and spend rowsStreamingResponseexists, which also closes the upstream connection and logs "abandoned before reaching the client" rather than a client disconnect, and a failing upstream close is logged instead of masking the exception that abandoned the relay,5beef60327); the response'sBackgroundTaskand lifespan shutdown (10 s wait) only await it-----BEGIN PRIVATE KEY-----mention in an error message stays readableUser Flow
Before: a developer streaming through a passthrough route gets nothing while the provider holds a 429 open, and the log misfiles it as a budget error
429_BUDGET_EXCEEDEDAfter: the 429 and its first frame arrive at once and the log entry stays a passthrough error
500_UPSTREAM_PASSTHROUGHwith key-shaped fragments maskedBehavior changes
Streamed
post_call_failure_hookfor an upstream error now runs afterpost_call_response_headers_hookand after the last body byte. A response header derived from failure-hook state is no longer present on streamed error responses (base:x-failure-for-this-request: 500, tip:none), pinned bytest_passthrough_streamed_error_headers_do_not_carry_failure_hook_stateand the ASGI order test intest_pass_through_endpoints.py. Non-streamed errors are unchangedRelevant issues
Follow-up to #42695
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)Screenshots / Proof of Fix
Two proxies on real Postgres (
proxy_batch_write_at: 1), base at the merge basef6882246d4and tip at58d02d5f84(6a4a17f013and3da1a16060only change tests), forwarding/ab/...to a scripted upstream./sse-hanganswers 429text/event-stream, sends one frame and holds 12 s./json-429answers 429{"error": {"message": "You exceeded your current quota ... Budget for Key=sk-<40 chars> is spent"}}./abort-Nstreams N bytes of a 500 then closes abnormally/sse-hang{"stream":true}code=429 ttfb=12.076 total=12.076code=429 ttfb=0.008 total=12.110/sse-hang{}(SSE detected from content-type)ttfb=12.063ttfb=0.006/sse-hangwith--max-time 5curl: (28) ... 0 bytes received,code=000curl: (28) ... 32 bytes received,code=429, body is the frame/json-429, thenselect normalized_error from "LiteLLM_SpendLogs" where request_id=<call id>429_BUDGET_EXCEEDED500_UPSTREAM_PASSTHROUGH, error_message hasKey=REDACTED/sse-hangwith--max-time 2(disconnect after first frame)500_UPSTREAM_PASSTHROUGH, error_code 429/abort-5000(past the 4 KB preview)size=5000,curl: (18)size=5000,curl: (18)/abort-3000(inside the preview)size=3000, exit 0size=3000, exit 0uvicorn --timeout-graceful-shutdown 3, SIGTERM once the hook is parkedstarlette/background.pysse_keepalive_ping_interval_seconds: 0.2, 2 s failure hook, 500 SSE body: pingframesx-failure-for-this-requestheader set from failure-hook state500none(see Behavior changes)Byte-identical on both legs: 6 KB streamed 500 in 512 B chunks, non-streaming 404, 200 SSE success, gzip 500,
turn_off_message_logging: trueAudit at the tip
5beef60327(merge base14f4c34c61), two proxies with two workers each on real Postgres and Redis, scripted upstream only at the edge, base leg run id95652898eaa848a38519886505721871, head leg run id92a5193b18b6415f86b8159fd9c6a8c1. The head'stests/integration/observability/test_passthrough_upstream_error_visibility.pyandtest_passthrough_upstream_error_chaos.py(54 nodes) ran against the merge base: 19 red, 34 green, 1 skipped. The same files at the tip ran twice inside the fullextensionsgroup with--hypothesis-seed=42 --integration-order-seed=42: 53 green and 1 skipped both times, identical collected and passed selections for every passthrough nodetests/integration/observability/)test_gemini_passthrough_streaming_429_first_frame_reaches_client_while_upstream_holds,..._async_streaming_429_first_frame...,test_provider_passthrough_streaming_429_first_frame_reaches_client_while_upstream_holds[anthropic,azure,cohere,mistral,openai,config-route]test_gemini_passthrough_quota_wording_in_upstream_body_keeps_passthrough_normalized_error,..._async_quota_wording_keeps_passthrough_normalized_errortest_gemini_passthrough_streaming_429_client_disconnect_still_logs_failure,..._streaming_429_upstream_abort_after_first_frame_still_logs_once,test_upstream_abort_after_preview_budget_reaches_client_as_truncated,test_passthrough_keepalive_pings_never_follow_the_upstream_error_body,test_passthrough_streamed_error_headers_do_not_carry_failure_hook_state,..._streaming_error_with_keepalive_and_slow_headers_logs_failure_oncetest_passthrough_disconnect_burst_logs_every_failure_once,test_passthrough_sigterm_drains_reports_parked_on_a_slow_failure_hook,test_passthrough_sigterm_with_graceful_timeout_exits_and_flushes_spend,test_passthrough_abort_after_budget_with_disconnect_during_hook_logs_once,test_passthrough_sigterm_exits_when_late_relay_background_never_returnstest_llm_endpoint_upstream_quota_429_normalized_error_unchanged[*](6),test_budget_rejected_call_keeps_budget_normalized_error, 200 SSE, gzip, 4096/4097, control chars, empty body, streaming 500, 404, sync and async SDK errors, message logging off, repeated errors, 64 KB near miss,test_passthrough_upstream_outage_mid_burst_still_logs_errors_once,test_passthrough_error_report_survives_sigterm_after_full_response[hypercorn,uvicorn],..._sigterm_graceful_window_reports_dispatched_at_preview_budget,..._sigterm_during_headers_hook_still_dispatches_the_report,test_passthrough_buffered_error_body_redaction_stays_boundedtest_passthrough_worker_sigkill_leaves_sibling_serving_and_loggingBUG:, see REVIEWER MUST KNOW)Two cells outside this PR fail in the same
extensionsgroup on the merge base and the tip alike or flake between the two tip runs:test_redis_outage_keeps_serving_in_memory_hits(the box's Redis 6.0.16 rejects theset-proc-titledirective the owned-redis helper passes) and onetest_presidio_streaming_outputcell per run (a periodicGET /v1/modelspoll hitting the scripted wire). Neither touches this diff and both are green on this PR's CircleCI. Unit tests in the mappedtests/test_litellmfiles cover the 64/65 boundary, exactly-once report, disconnect warning, post-budget abort reporting before the re-raise, theBackgroundTaskhandoff, the hook order over a real ASGI call and the abandon close failure; fifteen mutation checks each turn exactly their guard redType
🐛 Bug Fix
Caveats (if any)
Medium
MINIMUM_CUSTOM_KEY_LENGTH) still pass the redactor, same as the base and every other error logLow
Final Attestation
REVIEWER MUST KNOW BEFORE APPROVING
test_passthrough_worker_sigkill_leaves_sibling_serving_and_logginggreen at the base, 3/3 red at the tip, nowpytest.skip("BUG: ...")pertests/integration/AGENTS.md. Accepted as-is by yucheng-berri; the fix would put the hook back on the body tail, which is the keepalive-ping regressiontest_passthrough_sigterm_exits_when_late_relay_background_never_returnsREDACTEDfrom its last word boundary, and text past the cut never enters the output even when an earlier redaction shortens the head. A private key whose END marker falls past the window is cut toREDACTEDfrom its BEGIN header only when base64-shaped key material follows it: after optional whitespace or JSON-escaped\nliterals, one 40+ char base64 token with no spaces, two or more consecutive 16+ char base64 tokens (a key wrapped into short lines), a 16+ char token that ends the head, or the header itself ending the head. Prose that merely quotes the header (Vertex "must start with -----BEGIN PRIVATE KEY-----", "header is missing", a URL) reaches the log unchanged; a key wrapped into lines shorter than 16 chars would not be caught (7267f32954,0005428a76,b84ae93341,2cace25930)httpx.HTTPErrorre-raise (the ASGI call raises, so no background would run). With keepalive pings enabled, a slow hook there can emit pings before the abnormal closehttpx.HTTPErroron an error stream logs oneread failedwarningpost_call_response_headers_hookon streamed errorsLink to Devin session: https://app.devin.ai/sessions/84cc1558c71a4a789c9bd0dfcef7ff23
Open in Devin Desktop: https://app.devin.ai/desktop/session/84cc1558c71a4a789c9bd0dfcef7ff23?variant=devin
Requested by: @yucheng-berri
Note
Medium Risk
Changes core proxy passthrough error streaming, failure-hook timing relative to headers/body, and shutdown/report lifecycle, which affects observability and spend rows under client disconnect and graceful shutdown.
Overview
Fixes passthrough routes that blocked the client while reading up to 4 KB of an upstream error before relaying, and misclassified quota-style upstream bodies as budget errors.
Streaming relay: Upstream 4xx/5xx bodies are forwarded chunk-by-chunk immediately via
_PreviewReportingStream. A separate named background report collects a preview for warnings,post_call_failure_hook, and spend metadata—after the stream ends, on preview budget, on disconnect, or via abandon if building the response fails.StreamingResponsegets aBackgroundTask; proxy shutdown waits up to 10s for in-flight report tasks.Safety and classification: Log/hook text is built from a 64 KiB scan with bounded
redact_secrets, PEM dangling-key masking, and truncation markers.normalize_errortreats an "upstream passthrough request failed" prefix as500_UPSTREAM_PASSTHROUGHbefore quota/budget heuristics. Keepalive late-relay teardown now runs response background work with a bounded wait.Integration chaos tests and unit coverage exercise disconnect bursts, SIGTERM, slow hooks, and large error bodies; one worker-SIGKILL spend-row test is skipped as a known gap.
Reviewed by Cursor Bugbot for commit 5beef60. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Medium Risk
Changes core proxy passthrough error streaming, failure-hook timing relative to headers/body, and shutdown/report lifecycle, which affects observability and spend rows under client disconnect and graceful shutdown.
Overview
Fixes passthrough routes that blocked the client while reading up to 4 KB of an upstream error before relaying, and misclassified quota-style upstream bodies as budget errors.
Streaming relay: Upstream 4xx/5xx bodies are forwarded chunk-by-chunk immediately via
_PreviewReportingStream. A separate named background report collects a preview for warnings,post_call_failure_hook, and spend metadata—after the stream ends, on preview budget, on disconnect, or via abandon if building the response fails.StreamingResponsegets aBackgroundTask; proxy shutdown waits up to 10s for in-flight report tasks.Safety and classification: Log/hook text is built from a 64 KiB scan with bounded
redact_secrets, PEM dangling-key masking, and truncation markers.normalize_errortreats an "upstream passthrough request failed" prefix as500_UPSTREAM_PASSTHROUGHbefore quota/budget heuristics. Keepalive late-relay teardown now runs response background work with a bounded wait.Integration chaos tests and unit coverage exercise disconnect bursts, SIGTERM, slow hooks, and large error bodies; one worker-SIGKILL spend-row test is skipped as a known gap.
Reviewed by Cursor Bugbot for commit 5beef60. Bugbot is set up for automated code reviews on this repo. Configure here.