Repository navigation
fix(utils): isolate callback errors in async_post_call_success_deployment_hook - #42535
Conversation
…ment_hook A callback that raises inside async_post_call_success_deployment_hook no longer fails the completed request. The exception is logged with the callback class and call_type, the response stays as it was, and later callbacks still run. Guardrail callbacks are exempt because raising is how a post-call guardrail blocks 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>
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
bugbot run |
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
…sing hook Parametrize the unit regression over video, embedding, responses, image, rerank, transcription, chat and anthropic messages responses and assert the failure log names the callback and call type. Run the integration test through a real proxy for /v1/chat/completions, /v1/embeddings, /v1/responses and /v1/videos Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
bugbot run |
| @@ -0,0 +1,171 @@ | |||
| import base64 | |||
There was a problem hiding this comment.
This bug fix creates a separate integration test file instead of extending the existing mapped callback test file. Repository policy requires bug fixes to extend the existing mapped test file, so this requirement must be satisfied before merging.
Context Used: AGENTS.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…callback delivery file 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 c8e9756. Configure here.
…e failure-hook regression (#42646) * test(utils): raise the post-success hook error from a guardrail in the failure-hook regression Since #42535 a plain logger raising inside async_post_call_success_deployment_hook is logged and the completed request returns, so the regression added by #36657 for "a post-success error never reaches async_post_call_failure_deployment_hook" failed with DID NOT RAISE on every main run once #42603 revived the misc unit shard. The raising callback is now a CustomGuardrail, the one kind of callback whose post-success raise still propagates, which keeps the original assertions intact * test(utils): type the guardrail's success-hook request_data as a Mapping --------- Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
TLDR
Problem this solves:
video_generationthrough the async success deployment hookresponse.choicesraises onVideoObjectHow it solves it:
async_post_call_success_deployment_hookawait is wrapped in try/exceptcall_type, then skippedCustomGuardrailcallbacks re-raise, since raising is how a post-call guardrail blocksUser Flow
Before: an operator with a chat-oriented custom callback gets a 400 back for every video job, even though the provider accepted it
{"model": "sora-2", "prompt": "a cat", "seconds": "4", "size": "720x1280"}{"error": {"message": "Invalid request format: 'VideoObject' object has no attribute 'choices'", ...}}After: the same request returns the queued video job
{"id": "video_...", "object": "video", "status": "queued", ...}Relevant issues
Regression introduced by #42354 (merged as 12f7930)
Affected release
Linear ticket
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
Live proxy on each side (
python litellm/proxy/proxy_cli.py --config config.yaml --port <port>), real OpenAI key, same config on both sides:hooks.pyonPYTHONPATH:chat_shapedis aCustomLoggerwhoseasync_post_call_success_deployment_hookreadsresponse.choices[0].message.contentand returns None,recordingis aCustomLoggerthat logs the response type and returns None,BlockingGuardrailis aCustomGuardrailwhoseasync_post_call_success_hookraisesHTTPException(400, {"error": "blocked by risk guardrail"})Before (9d299d0)
Video job with the chat-shaped hook
curl -s http://localhost:4302/v1/videos -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model":"sora-2","prompt":"a cat","seconds":"4","size":"720x1280"}' -w '\nHTTP %{http_code}\n'{"error":{"message":"Invalid request format: 'VideoObject' object has no attribute 'choices'","type":"invalid_request_error","param":null,"code":"400"}}HTTP 400RISK chat_shaped saw VideoObject call_type=CallTypes.avideo_generationthree times (three upstream jobs),recordingnever ranChat completion with the same hooks
curl -s http://localhost:4302/v1/chat/completions -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model":"gpt-5.6","messages":[{"role":"user","content":"say hi"}]}' -w '\nHTTP %{http_code}\n'{"id":"chatcmpl-...","model":"gpt-5.6","object":"chat.completion","choices":[{"finish_reason":"stop","index":0,"message":{"content":"Hi!","role":"assistant",...}}],...}HTTP 200, both hooks loggedChat completion with a per-request post_call guardrail that raises
"guardrails":["risk-blocker"]added to the body{"error":{"message":"blocked by risk guardrail","type":"invalid_request_error","param":null,"code":"400","provider_specific_fields":{"error":"blocked by risk guardrail","guardrail_name":"risk-blocker","guardrail_mode":"post_call"}}}HTTP 400After (c8e9756, production diff identical to 6c03b71; the last commit only moves the integration cases into test_callback_delivery.py)
Video job with the chat-shaped hook
curl -s http://localhost:4301/v1/videos -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model":"sora-2","prompt":"a cat","seconds":"4","size":"720x1280"}' -w '\nHTTP %{http_code}\n'{"id":"video_bGl0ZWxsbTpjdXN0b21fbGxtX3Byb3ZpZGVyOm9wZW5haTttb2RlbF9pZDpzb3JhLTI7dmlkZW9faWQ6dmlkZW9fNmFiMmY2N2E0NWM4ODE5MTk3MzI4YWE3NjdkNjRhNDIwN2M2Yzg0YTlhMTY2NTQw","object":"video","status":"queued",...,"seconds":"4","size":"720x1280","model":"sora-2","usage":{"duration_seconds":4.0}}HTTP 200RISK chat_shaped saw VideoObjectonce, thenLiteLLM:ERROR: utils.py:1452 - async_post_call_success_deployment_hook error in ChatShapedHook for call_type=CallTypes.avideo_generation, thenRISK recording saw VideoObject call_type=CallTypes.avideo_generationChat completion with the same hooks
Chat completion with a per-request post_call guardrail that raises
"guardrails":["risk-blocker"]against port 4301blocked by risk guardrailbody, so guardrail blocking still worksAdmin UI, same request on both sides
The screen recording of this walkthrough (base 9d299d0 on port 4102, an earlier tip addc9bb on port 4101, real sora-2 call on each; the production change is unchanged since) was handed to the requester with the session report. To reproduce: start each proxy with the config above and its own
DATABASE_URL, send the video curl, open http://127.0.0.1:4101/ui/?page=logs and http://localhost:4301/ui/?page=logs (different origins, so the two dashboards keep separate sessions), log in withadminand the master key, search the Logs page by the returned request id and open the row. Base showsFailurewithMessage: 'VideoObject' object has no attribute 'choices'in the detail panel. The PR tip showsSuccess, call typeavideo_generation, cost $0.40 andRetries: NoneLive audit matrix (head 6c03b71 vs base 9d299d0, production code identical to c8e9756)
Two proxies, two uvicorn workers each, own Postgres per side, real OpenAI and Anthropic keys, no mocks. Callbacks registered: a chat-shaped hook that reads
response.choices, a hook that always raisesValueError, and a recording hook that returns the response, plus apost_callCustomGuardrail(default_on: false) that raises 400 when named inguardrailsRerun at the tip 6c03b71: head returns 200 on every non-streaming call type with two of three hooks raising:
/v1/chat/completions(curlchatcmpl-ER2YXFB4..., openai sync, openai async),/v1/responses(openai sync and async),/v1/messages(anthropic syncmsg_011CfKCzGUEk...and async),/v1/embeddings, and a realsora-2/v1/videosjob (video_bGl0ZWxs...MTY2NTQw, queued). The proxy log carries oneasync_post_call_success_deployment_hook error in <Hook> for call_type=CallTypes.<type>per raising hook and onerecording saw ...line per request, so the third hook ran every time. Base on the same cells returns 500audit: always raising hookfor chat, 500'dict' object has no attribute 'choices'for messages, 400'ResponsesAPIResponse' object has no attribute 'choices'for responses, 400'EmbeddingResponse' ...for embeddings and 400'VideoObject' ...for videos. Streaming chat, responses and messages return 200 on both sides because streaming provider responses do not pass through this hookguardrails: ["audit-blocker"]returns 400blocked by audit guardrailon head for chat, videos and embeddings; on base the ordinary hook exception fired first (500 on chat, 400 choices error on the other two) and masked the guardrail. Bad model name (400), invalid provider key (401), no api key (401),/key/healthand/health/readiness(200) are identical on both sides.guardrailsas an integer gives 500'int' object is not iterableandguardrailsas a bare string is iterated per character on both sides: pre-existing, not touched hereThree identical chat requests produce three distinct spend rows with one row each. A 32-request async burst across chat, responses, embeddings and streaming chat lands 24/24 spend rows, no duplicates. Killing one worker with SIGKILL mid burst drops only the 3 in-flight connections on that worker, a replacement worker spawns, all 27 completed ids have exactly one success row, and the next 8 requests are 200. SIGHUP restarts the workers and drops the requests in flight on both head and base
Full call-type matrix (head c8e9756 vs base 9d299d0)
Same two proxies, one ordinary
CustomLoggerthat always raises plus a recording hook, real keys for OpenAI, Anthropic, Gemini, Cohere, Mistral, Tavily and Azure OpenAI, no mocks. One real request per call type that reaches the hook. Head returns 200 and logs the isolation line and the recording line, base returns 500audit: always raising hook, on all of: chat, text completions, embeddings, moderations, anthropic messages, responses, responses input items, gemini generateContent, rerank, search, vector store search, ocr, azure pass-through, image generation, image edit, speech, transcription, file create/retrieve/list/delete, batch create/retrieve, fine-tuning job create/retrieve/cancel, container create/list/retrieve/delete, skill create, rag ingest and query, and a completed sora-2 video with list, content, remix, extension, edit, create character and get character. MCPtools/callover the real/mcpstreamable-http transport returns the tool result on head and a tool error carrying the hook message on base. Through the SDK, file content, container file upload and list, vector store search, gemini, azure pass-through and A2Aasend_message(completion bridge) all return on head and raise the hook'sValueErroron baseNot entering the hook on either side, unchanged by this PR: the proxy handlers for file content, container file upload and list, and the
/mcp-rest/tools/callfacade (they bypass the@clientwrapper), every streaming call (gemini streamGenerateContent plus streaming chat, responses and messages),arealtimeandaresponses_websocket, and functions absent fromCallTypes(video status, list batches, skills list/get/delete, vector store CRUD, evals, interactions)Not driven: the sandbox family (
acreate_sandbox,arun_code,adelete_sandbox,acode_interpreter_tool), whose only providers are E2B (no key available) and a self-hosted OpenSandbox server. Same wrapper and dispatcher. The mock_response converted stream path and the async pre-call hook are also not exercised (out of scope, see Caveats). Per-cell commands, status codes, response ids and log lines are in the matrix report handed to the requester (matrix_report.md)Type
🐛 Bug Fix
Caveats (if any)
Severe
CustomGuardrailMedium
post_call_success_deployment_hookdispatcher; the only other caller isresponses/streaming_iterator.py, which already wraps the hook in a baretry/exceptand drops the exception without logging. Unchanged hereasync_pre_call_deployment_hookstill propagates hook exceptions; unchanged here, and pre-call hooks may legitimately reject a requestisinstance(callback, CustomGuardrail), so a guardrail built on plainCustomLoggerwould now be swallowedLow
rust-wheelfails the same seventests/test_litellm_rusttests on main 05d7fb2 (run 35772779792),proxy-infra / Run testsfails on the merge base 9d299d0 too, andcodecov/patch(37.5%) is below target because themiscunit shard that ownstests/test_litellm/test_utils.pycollected[0 items]in CI and the workflow treats pytest exit 5 as a pass, so the new branches inlitellm/utils.pyget no upload even though the mapped regression is green locally. None of the three touch the changed linestests/test_litellm/test_utils.pyhas two failures unrelated to this PR that also fail on main (test_generate_azure_ad_redis_token,test_aaamodel_prices_and_context_window_json_is_valid)Final Attestation
Tests
tests/test_litellm/test_utils.py::test_success_deployment_hook_raising_keeps_response_and_runs_later_hooksis parametrized overVideoObject,EmbeddingResponse,ResponsesAPIResponse,ImageResponse,RerankResponse,TranscriptionResponseandModelResponse(chat and anthropic messages call types) and asserts the response is returned unchanged, the later hook still runs and exactly oneverbose_logger.exceptionrecord names the callback class and call type. A second test checks a hook that raises after an earlier hook rewrote the response keeps the rewrite. The guardrail test checks a raisingCustomGuardrailstill aborts. Mutation: replacing the catch with a bareraisefails 9 of the 13 mapped teststests/integration/observability/test_callback_delivery.py::test_response_survives_raising_success_deployment_hook (added to the existing callback file, four parametrized cases)starts a real proxy with an always-raisingCustomLoggercallback and a scripted upstream, and sends/v1/chat/completions,/v1/embeddings,/v1/responsesand/v1/videos. All four are 500hook rejected <Type> for CallTypes.<type>on base 9d299d0 and 200 with the upstream payload intact at the tipran /live-pr-risk at 6c03b71 (production diff unchanged since the first run and unchanged by the test-only move to c8e9756, live matrix rerun on that code) and found no regressions/backward incompatible risks beyond the approved one above
Link to Devin session: https://app.devin.ai/sessions/d954ca781d894d068319a115b73cf73b
Open in Devin Desktop: https://app.devin.ai/desktop/session/d954ca781d894d068319a115b73cf73b?variant=devin
Requested by: @yucheng-berri