Repository navigation
fix(azure): propagate asyncio.CancelledError instead of raising a 500 - #35330
devin-ai-integration[bot] wants to merge 7 commits into
Conversation
…OpenAIError(500) Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
|
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| async def cancelled_create(**kwargs): | ||
| raise asyncio.CancelledError() | ||
|
|
||
| client.chat.completions.with_raw_response.create = cancelled_create |
There was a problem hiding this comment.
Directly replacing with_raw_response.create on a real SDK client violates the repository's test convention and couples this regression test to the SDK wrapper object's attribute behavior. Use the established dependency-injection or mock-patching pattern so SDK implementation changes do not turn this isolated test into a network request.
Context Used: CLAUDE.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!
There was a problem hiding this comment.
The handler type-checks for a real AsyncAzureOpenAI, so the swap is on an instance the test owns, never a class, matching the sibling streaming test.
There was a problem hiding this comment.
You're right. This is an instance-local replacement on a client owned by the test, not a class-level monkeypatch, and it follows the existing _FakeRawCompletions injection pattern in the sibling streaming test in this file. Since litellm.acompletion receives that client explicitly and the fake prevents any network call while exercising the real Azure handler path, this is appropriate. I withdraw the comment.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Customer report (Pylon #8893) confirms this also trips false router cooldowns on healthy Azure deployments. Tracked in LIT-8248 |
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…ancelled Live proxy with cancel_on_disconnect, two-deployment group, generic allowed_fails=0, red at the pre-fix handler and green with the bare raise Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…e Azure deployment
…ncellederror_35329
…hub.com/BerriAI/litellm into devin_ai_fix_azure_cancellederror_35329 # Conflicts: # tests/e2e/router/reliability_support.py # tests/e2e/router/test_reliability_cancel_on_disconnect_e2e.py # tests/test_litellm/llms/azure/test_azure.py
|
bugbot run |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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 70be37a. Configure here.
|
Superseded by #42295, the same tip moved to a litellm_ branch so CircleCI runs the legacy provider suites on it |
This branch was successfully deployed
3 participants
TLDR
Problem this solves:
cancel_on_disconnect, one client hang-up became a deployment failureAzureException APIError500s Azure never returnedHow it solves it:
asyncio.CancelledErrorinstead ofAzureOpenAIError(500)Transport.abandon, which closes the socket mid-answerUser Flow
Before: a platform team running
cancel_on_disconnect: truein front of an Azure model group watches one impatient client take a healthy Azure deployment out of rotation"model": "azure-group"and a prompt whose answer takes about a minuteAzureException APIErrorwith an empty error body, booked against the Azure deployment that was serving it"model": "azure-group", comes back 200 with anx-litellm-model-idheader naming a different deployment: the one the client hung up on is in cooldownAfter: the same hang-up is recorded as the client's doing and the Azure deployment keeps serving
"model": "azure-group"and a prompt whose answer takes about a minuteClient disconnected the requestand is not counted against any deployment"model": "azure-group", comes back 200 withx-litellm-model-idnaming the same Azure deployment the client hung up onRelevant issues
Fixes #35329
Fixes #42222
Affected release
Linear ticket
Resolves LIT-8248
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/llms/azure/test_azure.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
Shared setup: a local proxy on a random port with
general_settings.cancel_on_disconnect: trueandstore_model_in_db: true,router_settingsonsimple-shufflewithnum_retries: 3, real Azure OpenAI (azure/gpt-5.4-nano,api_version 2024-10-21) and OpenAI credentials from the environment. Every case registers a fresh two-deployment group through POST /model/new: the Azure deployment withweight: 1,max_retries: 0,cooldown_time: 300andmodel_info.allowed_fails: 0, plus aweight: 0OpenAI backup (openai/gpt-5.5) that simple-shuffle can only reach once the Azure deployment is benched. Before ran the proxy with the merge base's Azure handler and this PR's tests, After ran the proxy at this PR's tip. Real Azure and OpenAI calls, no mocksBefore (a83773c)
Live e2e cell: a client hanging up mid-answer never benches the deployment
tests/e2e:LITELLM_PROXY_URL=http://127.0.0.1:38037 pytest router/test_reliability_cancel_on_disconnect_e2e.py -p no:cacheprovider -q --no-header -rA(the cell readscancel_on_disconnectback from the proxy, warms the key up with one cheap call, asks for a 3000-word essay with retries off, hangs up 8s in, settles 3s, then sends six cheap calls)curl: hang up 2s into a long answer, then ask the group three times
POST /model/newtwice, bodies as in the QA runbook below) and wait for the group to show onGET /v1/models:curl -s --max-time 2 -X POST http://127.0.0.1:38037/chat/completions -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model": "azure-disconnect-before3", "messages": [{"role": "user", "content": "Write a 3000 word essay on the history of the telegraph."}], "max_tokens": 4096}'curl -s -D - -o /dev/null -X POST http://127.0.0.1:38037/chat/completions -H "Authorization: Bearer $KEY" -H 'Content-Type: application/json' -d '{"model": "azure-disconnect-before3", "messages": [{"role": "user", "content": "say hi"}]}' | grep -i x-litellm-model-idEvery follow-up is answered by the zero-weight backup: the Azure deployment is benched
Proxy log
grep -n 'Attempting to add\|client disconnected' litellm.logaround each hang-up:The cancelled call is booked as a bodiless 500
APIErrorand the deployment goes to cooldown before the 499 reaches the clientAfter (70be37a)
Live e2e cell: a client hanging up mid-answer never benches the deployment
curl: hang up 2s into a long answer, then ask the group three times
Every follow-up is answered by the Azure deployment the client hung up on
Proxy log
grep -c 'Attempting to add' litellm.logis 0: each hang-up is logged as a client disconnect and nothing is benchedPlain
litellm.acompletionagainst a stalled Azure endpoint, cancelled mid-flight, from the original run of this PR:Type
🐛 Bug Fix
✅ Test
Caveats (if any)
Low
cancel_on_disconnect: trueon the proxy: the stage mirror CI config sets it, and the Buildkite ephemeral stack values get it in the companion PR BerriAI/project-releaser#255, sobuildkite/e2e-testshere stays red until that landsCancelledError; streaming was never affectedacompletiontask now seeCancelledError, the asyncio contract, instead ofAPIErrorlitellm.APIError, which no per-type allowed-fails policy covers, so the test benches on genericallowed_fails: 0gpt-5.4-nanoanswer takes longer than the 8s hang-up window; if Azure ever answers first the cell fails loudly instead of passing vacuouslyQA runbook
general_settings.cancel_on_disconnect: trueandstore_model_in_db: true, withAZURE_API_KEY,AZURE_API_BASE(a resource servinggpt-5.4-nano) andOPENAI_API_KEYin the environment{"model_name": "grp", "litellm_params": {"model": "azure/gpt-5.4-nano", "api_key": "os.environ/AZURE_API_KEY", "api_base": "os.environ/AZURE_API_BASE", "api_version": "2024-10-21", "max_retries": 0, "weight": 1, "cooldown_time": 300}, "model_info": {"allowed_fails": 0}}and note the returnedmodel_info.id{"model_name": "grp", "litellm_params": {"model": "openai/gpt-5.5", "api_key": "os.environ/OPENAI_API_KEY", "weight": 0}}curl -s -D - -o /dev/null -X POST http://localhost:4000/chat/completions -H "Authorization: Bearer sk-1234" -d '{"model": "grp", "messages": [{"role": "user", "content": "say hi"}]}'and expect 200 withx-litellm-model-idequal to the Azure id (a cold virtual key can spend a couple of seconds in auth, and a hang-up that lands before the provider call is in flight cancels nothing)curl -m 8 -X POST http://localhost:4000/chat/completions -H "Authorization: Bearer sk-1234" -d '{"model": "grp", "messages": [{"role": "user", "content": "Write a 3000 word essay on the history of the telegraph."}], "max_tokens": 4096, "stream": false}'and expect curl to exit 28 with no response (a 4096-token answer takes about a minute, so 8s lands well inside it)x-litellm-model-idequal to the Azure id every time; before the fix the first one already names the backup idclient disconnected, upstream LLM request cancelledfor the hang-up and noAttempting to add <id> to cooldown listlinecancel_on_disconnectback from the proxy so it cannot pass vacuously, and fails loudly if Azure answers before the hang-up) or potentially flakyFinal Attestation
Link to Devin session: https://app.devin.ai/sessions/879961a9c5504feda31adfde569811db
Open in Devin Desktop: https://app.devin.ai/desktop/session/879961a9c5504feda31adfde569811db?variant=devin
Note
Medium Risk
Changes Azure async completion error handling and cancellation propagation, which affects router cooldown behavior under cancel_on_disconnect; scope is narrow but touches reliability-critical routing paths.
Overview
Fixes a bug where Azure async chat turned
asyncio.CancelledError(e.g. fromcancel_on_disconnectwhen the client closes the socket) into a fake 500AzureOpenAIError, so the router treated a hang-up as a deployment failure and could cooldown a healthy Azure deployment.The handler still runs
post_calllogging, then re-raisesCancelledErrorinstead of mapping it to HTTP 500. SDK callers cancelling an Azureacompletionnow get the normal asyncio cancellation contract, notAPIError.Tests and e2e harness: a unit test asserts
litellm.acompletionpropagatesCancelledError; a live e2e case hangs up mid-answer via newabandon/Transport.abandon, checkscancel_on_disconnectviaGET /config/list, and verifies follow-up calls still hit the same Azure deployment. CI mirror config enablescancel_on_disconnect: true; coverage registry addsreliability.cooldown.client_disconnect.stays_healthy.Reviewed by Cursor Bugbot for commit 70be37a. Bugbot is set up for automated code reviews on this repo. Configure here.