fix(responses): surface upstream error status on get instead of 500 - #32287
Conversation
Greptile SummaryThis PR fixes
Confidence Score: 5/5Safe to merge — changes are narrowly scoped to four GET call sites and one Azure exception-mapping branch, with mock-transport tests covering each modified path. The fix correctly applies No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/llm_http_handler.py | Adds response.raise_for_status() to all four GET call sites (sync/async get_responses and list_responses_input_items), matching the existing post/delete pattern; also correctly demotes the async_get_responses log from exception to debug. |
| litellm/litellm_core_utils/exception_mapping_utils.py | Gates the "invalid_request_error" string-match branch to status_code in (None, 400) and adds a dedicated 404 branch in the Azure status-code ladder, fixing Azure 404s being incorrectly downgraded to BadRequestError. |
| tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py | Adds mock-transport tests covering all four changed GET paths (async and sync get_responses, async and sync list_input_items); uses httpx.MockTransport to avoid real network calls, consistent with repo test rules. |
| tests/test_litellm/litellm_core_utils/test_exception_mapping_utils.py | Adds a regression test pinning Azure 404 + invalid_request_error body to NotFoundError, exercising the updated _map_azure_exception branch directly. |
Reviews (3): Last reviewed commit: "fix(responses): demote expected 4xx get ..." | Re-trigger Greptile
Greptile SummaryThis PR fixes
Confidence Score: 4/5The change is narrow, well-tested, and fixes a real regression without touching any auth or critical-path logic. Both changes are straightforward: adding
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/llm_http_handler.py | Adds response.raise_for_status() to four GET call sites so upstream HTTP errors route through _handle_error instead of reaching the pydantic transformer and exploding as ValidationError; async get_responses will now also emit a verbose_logger.exception stack trace for expected 4xx errors. |
| litellm/litellm_core_utils/exception_mapping_utils.py | Adds a 404 branch to the Azure status-code ladder and gates the "invalid_request_error" string match to status 400/None, preventing Azure 404s (which carry "type": "invalid_request_error") from being downgraded to BadRequestError. |
| tests/test_litellm/llms/custom_httpx/test_llm_http_handler.py | New mock-transport tests verify async GET, sync GET, and async list-input-items surface NotFoundError on 404; sync list_responses_input_items path is untested. |
| tests/test_litellm/litellm_core_utils/test_exception_mapping_utils.py | Adds a focused unit test pinning Azure 404 + invalid_request_error body to NotFoundError with correct status code; straightforward and correct. |
Reviews (2): Last reviewed commit: "fix(responses): surface upstream error s..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
1aa2a1d to
ab79f82
Compare
ab79f82 to
4e0a514
Compare
Relevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays 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 against a real Azure OpenAI deployment (
azure/gpt-4o, api_version 2025-03-01-preview), run with.venv/bin/python litellm/proxy/proxy_cli.py --config config.yaml --port 29764 --detailed_debugBefore, on
litellm_internal_staging@7f991481cc069d7a069a8a50c140bcfaec9a4e6c. Create a non-retrievable response, then GET itSame on a deleted id: POST with
"store": true, DELETE it (200response.deleted), then GET the deleted id returns the identical 500 pydantic ValidationError. Control: POST stored + GET returns 200After, on
fd9d0cf3f6ff9e718a5e172c17ad485f536a1fa2. Same two error curls now surface the real upstream status and messageGET on a deleted id likewise returns 404 with Azure's "Response with id ... not found." message. The stored control still returns 200 with the full response, and DELETE still returns 200
Type
🐛 Bug Fix
Changes
GET /v1/responses/{id}returned a 500APIConnectionErrorwhenever the upstream provider returned an error (response created with"store": false, or id already deleted). Root cause:HTTPHandler.get/AsyncHTTPHandler.getnever callraise_for_status()(unlikepostanddelete), so the upstream error JSON flowed intotransform_get_response_api_response, which force-parsed it into theResponsesAPIResponsepydantic model and blew up withValidationError: 3 validation errors ... id/created_at/output Field requiredllm_http_handler.py: the four responses GET call sites (get_responses,async_get_responses,list_responses_input_items,async_list_responses_input_items) now callresponse.raise_for_status()inside the existing try, so upstream errors route through_handle_errorand the provider error class with their real status and body, matching what create/cancel (POST) and delete already doexception_mapping_utils.py: the azure mapper had no 404 branch in its status ladder, and its"invalid_request_error" in error_strstring match ran before any status check, downgrading Azure's 404 (which carries"type": "invalid_request_error") to a 400. Added the 404 branch raisingNotFoundErrorand gated the string match to status 400 or no status, so real statuses survive the mappingRegression tests in the mapped test files:
tests/test_litellm/llms/custom_httpx/test_llm_http_handler.pycovers async GET, sync GET, and input items surfacing a 404NotFoundError(dependency-injected client backed byhttpx.MockTransport), andtests/test_litellm/litellm_core_utils/test_exception_mapping_utils.pypins azure 404 +invalid_request_errormapping toNotFoundError. All four tests fail on the pre-fix code and pass with the fix