From 8b89c909a9448a66a0f60cc3c9de7e236271b838 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Tue, 8 Sep 2026 12:16:08 -0700 Subject: [PATCH] fix(proxy): keep a ProxyException's status and label 408s in the OpenAI error payload error_status_code only read status_code, so a ProxyException raised before routing (which stores its status as the string code) answered 500 with its 4xx type through the rerank, images, realtime, files, and pass-through tails. It now falls back to a decimal code. A 408 maps to timeout_error instead of invalid_request_error. Tail regressions for rerank, images, realtime calls, and the chat pass-through fail at the merge base with ('None', 'None'); the new files-test helpers are fully typed. --- .../common_utils/openai_error_payload.py | 9 ++- .../common_utils/test_openai_error_payload.py | 28 ++++++++++ .../proxy/image_endpoints/test_endpoints.py | 48 +++++++++++++++- .../test_files_endpoint.py | 24 +++++--- .../test_pass_through_endpoints.py | 38 ++++++++++++- .../test_realtime_webrtc_endpoints.py | 42 ++++++++++++++ .../proxy/rerank_endpoints/test_endpoints.py | 56 ++++++++++++++++++- 7 files changed, 230 insertions(+), 15 deletions(-) diff --git a/litellm/proxy/common_utils/openai_error_payload.py b/litellm/proxy/common_utils/openai_error_payload.py index 180ec152094..2d589871fea 100644 --- a/litellm/proxy/common_utils/openai_error_payload.py +++ b/litellm/proxy/common_utils/openai_error_payload.py @@ -12,6 +12,7 @@ _OPENAI_ERROR_TYPE_BY_STATUS: Final[Mapping[int, str]] = MappingProxyType( { status.HTTP_401_UNAUTHORIZED: "authentication_error", status.HTTP_403_FORBIDDEN: "permission_error", + status.HTTP_408_REQUEST_TIMEOUT: "timeout_error", status.HTTP_429_TOO_MANY_REQUESTS: "rate_limit_error", } ) @@ -22,9 +23,13 @@ def attribute_of(value: object, name: str, default: object = None) -> object: def error_status_code(exc: object, default: int) -> int: - """The HTTP status an exception carries, or ``default`` when it carries none.""" + """The HTTP status an exception carries as ``status_code`` or, the way ``ProxyException`` + stores it, as a stringified ``code``; ``default`` when it carries neither.""" carried: Final = attribute_of(exc, "status_code") - return carried if isinstance(carried, int) and not isinstance(carried, bool) else default + if isinstance(carried, int) and not isinstance(carried, bool): + return carried + stringified: Final = attribute_of(exc, "code") + return int(stringified) if isinstance(stringified, str) and stringified.isdecimal() else default def openai_error_type(exc: object, status_code: int) -> str: diff --git a/tests/test_litellm/proxy/common_utils/test_openai_error_payload.py b/tests/test_litellm/proxy/common_utils/test_openai_error_payload.py index 3b39ea706fe..3145be2d522 100644 --- a/tests/test_litellm/proxy/common_utils/test_openai_error_payload.py +++ b/tests/test_litellm/proxy/common_utils/test_openai_error_payload.py @@ -18,6 +18,7 @@ from litellm.proxy.common_utils.openai_error_payload import ( (401, "authentication_error"), (403, "permission_error"), (404, "invalid_request_error"), + (408, "timeout_error"), (422, "invalid_request_error"), (429, "rate_limit_error"), (499, "invalid_request_error"), @@ -109,6 +110,33 @@ def test_a_non_int_carried_status_falls_back_to_the_default(carried_status: obje assert error_status_code(_Carrier("boom"), 500) == 500 +def test_a_proxy_exception_keeps_the_status_it_was_raised_with(): + """ProxyException stores its status as the string ``code`` rather than ``status_code``, + so a route tail that rewraps one used to answer a 4xx rejection as a 500.""" + rejection = ProxyException(message="session_id is required", type="bad_request_error", param="session_id", code=400) + + assert error_status_code(rejection, 500) == 400 + + +@pytest.mark.parametrize("carried_code", [None, "None", "", "rate_limited", "4xx", 404]) +def test_a_code_that_is_not_a_decimal_string_falls_back_to_the_default(carried_code: object): + """Only ProxyException's stringified status is a status; ``code`` on anything else + (OpenAI's ``invalid_api_key``, a stray int) says nothing about the HTTP answer.""" + + class _Carrier(Exception): + code = carried_code + + assert error_status_code(_Carrier("boom"), 500) == 500 + + +def test_a_status_code_wins_over_a_stringified_code(): + class _Carrier(Exception): + status_code = 429 + code = "400" + + assert error_status_code(_Carrier("boom"), 500) == 429 + + def test_a_status_carried_by_an_exception_drives_the_type_it_reports(): """The two helpers compose at every call site: the status the exception carries is what names its type, not the default the route would have used.""" diff --git a/tests/test_litellm/proxy/image_endpoints/test_endpoints.py b/tests/test_litellm/proxy/image_endpoints/test_endpoints.py index 203391aadad..d8b3eef98bd 100644 --- a/tests/test_litellm/proxy/image_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/image_endpoints/test_endpoints.py @@ -5,12 +5,12 @@ from typing import Any, Dict import orjson import pytest -from fastapi import FastAPI +from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient from starlette.requests import Request from starlette.responses import Response -from litellm.proxy._types import UserAPIKeyAuth +from litellm.proxy._types import ProxyException, UserAPIKeyAuth from litellm.proxy.auth.user_api_key_auth import user_api_key_auth from litellm.proxy.image_endpoints import endpoints @@ -167,3 +167,47 @@ def test_image_edit_multipart_n_that_is_not_a_number_is_left_alone(monkeypatch): assert response.status_code == 200 assert captured["n"] == "two" + + +@pytest.mark.asyncio +async def test_a_model_the_router_cannot_serve_answers_an_openai_typed_error(monkeypatch: pytest.MonkeyPatch): + """A bare HTTPException carries no type or param, so the tail used to ship the + literal string "None" in both fields.""" + + async def fake_add_litellm_data_to_request(**kwargs: object) -> object: + return kwargs["data"] + + async def fake_pre_call_hook(*, user_api_key_dict: UserAPIKeyAuth, data: dict[str, object], call_type: str) -> dict[str, object]: + return data + + async def fake_post_call_failure_hook(**_: object) -> None: + return None + + async def failing_route_request(**_: object) -> None: + raise HTTPException( + status_code=404, detail={"error": "image_generation: Invalid model name passed in model=dall-e-3"} + ) + + monkeypatch.setattr("litellm.proxy.proxy_server.add_litellm_data_to_request", fake_add_litellm_data_to_request) + monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {}) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", None) + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_config", {}) + monkeypatch.setattr( + "litellm.proxy.proxy_server.proxy_logging_obj", + SimpleNamespace(pre_call_hook=fake_pre_call_hook, post_call_failure_hook=fake_post_call_failure_hook), + ) + monkeypatch.setattr("litellm.proxy.proxy_server.user_model", None) + monkeypatch.setattr("litellm.proxy.proxy_server.version", "test-version") + monkeypatch.setattr("litellm.proxy.image_endpoints.endpoints.route_request", failing_route_request) + + body = orjson.dumps({"model": "dall-e-3", "prompt": "a lighthouse at dusk"}) + + async def receive() -> dict[str, object]: + return {"type": "http.request", "body": body, "more_body": False} + + request = Request({"type": "http", "method": "POST", "path": "/v1/images/generations", "headers": []}, receive) + + with pytest.raises(ProxyException) as raised: + await endpoints.image_generation(request=request, fastapi_response=Response(), user_api_key_dict=UserAPIKeyAuth()) + + assert (raised.value.type, raised.value.param, raised.value.code) == ("invalid_request_error", None, "404") diff --git a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py index 2257ae1ab29..132b53792b0 100644 --- a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py +++ b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py @@ -4668,7 +4668,9 @@ def test_create_file_path_traversal_filename_rejected_before_forwarding(monkeypa assert forwarded_calls == [] -def _setup_managed_file_route_answering_404(mocker: MockerFixture, monkeypatch, llm_router: Router): +def _setup_managed_file_route_answering_404( + mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch, llm_router: Router +) -> None: """Wire the single-file routes to a managed file store that knows no file, the way the managed files hook answers once a file has been deleted or was never the caller's.""" import litellm.proxy.proxy_server as ps @@ -4676,7 +4678,7 @@ def _setup_managed_file_route_answering_404(mocker: MockerFixture, monkeypatch, from litellm.llms.base_llm.files.transformation import BaseFileEndpoints from litellm.proxy._types import LitellmUserRoles - async def _file_not_found(file_id: str, **kwargs): + async def _file_not_found(file_id: str, **kwargs: object) -> None: raise HTTPException(status_code=404, detail=f"File not found: {file_id}") proxy_logging_obj = setup_proxy_logging_object(monkeypatch, llm_router) @@ -4694,7 +4696,7 @@ def _setup_managed_file_route_answering_404(mocker: MockerFixture, monkeypatch, ) -def _call_managed_file_route(method: str, path: str): +def _call_managed_file_route(method: str, path: str) -> httpx.Response: try: return client.request(method, path, headers={"Authorization": "Bearer test-key"}) finally: @@ -4703,7 +4705,7 @@ def _call_managed_file_route(method: str, path: str): app.dependency_overrides.pop(ps.user_api_key_auth, None) -def _missing_managed_file_error(file_id: str) -> dict: +def _missing_managed_file_error(file_id: str) -> dict[str, dict[str, str | None]]: return { "error": { "message": f"File not found: {file_id}", @@ -4714,7 +4716,9 @@ def _missing_managed_file_error(file_id: str) -> dict: } -def test_create_file_reports_a_half_specified_expires_after_as_a_400(monkeypatch, llm_router: Router): +def test_create_file_reports_a_half_specified_expires_after_as_a_400( + monkeypatch: pytest.MonkeyPatch, llm_router: Router +): """A 400 raised inside the route answers with the type a 400 stands for and a JSON null param, not the literal string "None" in both fields, so a client can classify it.""" setup_proxy_logging_object(monkeypatch, llm_router) @@ -4735,7 +4739,9 @@ def test_create_file_reports_a_half_specified_expires_after_as_a_400(monkeypatch assert error["code"] == "400" -def test_get_file_reports_a_missing_managed_file_as_a_404(mocker: MockerFixture, monkeypatch, llm_router: Router): +def test_get_file_reports_a_missing_managed_file_as_a_404( + mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch, llm_router: Router +): _setup_managed_file_route_answering_404(mocker, monkeypatch, llm_router) file_id = _unified_managed_file_id() @@ -4745,7 +4751,9 @@ def test_get_file_reports_a_missing_managed_file_as_a_404(mocker: MockerFixture, assert response.json() == _missing_managed_file_error(file_id) -def test_delete_file_reports_a_missing_managed_file_as_a_404(mocker: MockerFixture, monkeypatch, llm_router: Router): +def test_delete_file_reports_a_missing_managed_file_as_a_404( + mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch, llm_router: Router +): _setup_managed_file_route_answering_404(mocker, monkeypatch, llm_router) file_id = _unified_managed_file_id() @@ -4756,7 +4764,7 @@ def test_delete_file_reports_a_missing_managed_file_as_a_404(mocker: MockerFixtu def test_get_file_content_reports_a_missing_managed_file_as_a_404( - mocker: MockerFixture, monkeypatch, llm_router: Router + mocker: MockerFixture, monkeypatch: pytest.MonkeyPatch, llm_router: Router ): _setup_managed_file_route_answering_404(mocker, monkeypatch, llm_router) file_id = _unified_managed_file_id() diff --git a/tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py b/tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py index fb4ed3db4d2..d57bed430c1 100644 --- a/tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py +++ b/tests/test_litellm/proxy/pass_through_endpoints/test_pass_through_endpoints.py @@ -11,7 +11,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import httpx import pytest -from fastapi import Request, UploadFile +from fastapi import Request, Response, UploadFile from starlette.datastructures import FormData, Headers, QueryParams from starlette.datastructures import UploadFile as StarletteUploadFile @@ -22,6 +22,7 @@ from litellm.proxy.pass_through_endpoints.pass_through_endpoints import ( InitPassThroughEndpointHelpers, LITELLM_PASS_THROUGH_CUSTOM_BODY_STATE_KEY, _registered_pass_through_routes, + chat_completion_pass_through_endpoint, create_pass_through_route, initialize_pass_through_endpoints, pass_through_request, @@ -5837,3 +5838,38 @@ def test_passthrough_client_cannot_forge_session_id_omission(client_metadata_key ) == "per-call-random-trace-id" ) + + +@pytest.mark.asyncio +async def test_chat_completion_pass_through_endpoint_answers_an_openai_typed_error_for_an_unknown_model( + monkeypatch: pytest.MonkeyPatch, +): + """A bare HTTPException carries no type or param, so the tail used to ship the + literal string "None" in both fields.""" + proxy_logging = MagicMock() + proxy_logging.pre_call_hook = AsyncMock(side_effect=lambda **kwargs: kwargs["data"]) + proxy_logging.post_call_failure_hook = AsyncMock() + + async def fake_add_litellm_data_to_request(**kwargs: object) -> object: + return kwargs["data"] + + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", proxy_logging) + monkeypatch.setattr("litellm.proxy.proxy_server.add_litellm_data_to_request", fake_add_litellm_data_to_request) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", None) + monkeypatch.setattr("litellm.proxy.proxy_server.user_model", None) + monkeypatch.setattr("litellm.proxy.proxy_server.general_settings", {}) + + request = MagicMock(spec=Request) + request.body = AsyncMock( + return_value=json.dumps({"model": "unknown-model", "messages": [{"role": "user", "content": "hi"}]}).encode() + ) + + with pytest.raises(ProxyException) as raised: + await chat_completion_pass_through_endpoint( + fastapi_response=Response(), + request=request, + adapter_id="anthropic", + user_api_key_dict=UserAPIKeyAuth(api_key="sk-test"), + ) + + assert (raised.value.type, raised.value.param, raised.value.code) == ("invalid_request_error", None, "400") diff --git a/tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py b/tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py index 66eeb3cef34..8cc5994dc81 100644 --- a/tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py +++ b/tests/test_litellm/proxy/realtime_endpoints/test_realtime_webrtc_endpoints.py @@ -6,10 +6,12 @@ Tests for LiteLLM proxy realtime WebRTC HTTP endpoints: import json import time +from collections.abc import Awaitable, Callable from unittest.mock import AsyncMock, MagicMock, patch import httpx import pytest +from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient @@ -1199,3 +1201,43 @@ async def test_transcription_sessions_wraps_route_exception( assert "Model not allowed" in response.text finally: proxy_app.dependency_overrides.pop(user_api_key_auth, None) + + +def test_realtime_calls_upstream_rejection_answers_an_openai_typed_error( + proxy_app: FastAPI, + mock_add_litellm_data: Callable[..., Awaitable[object]], + mock_pre_call_hook: Callable[..., Awaitable[object]], + monkeypatch: pytest.MonkeyPatch, +): + """A bare HTTPException carries no type or param, so the tail used to ship the + literal string "None" in both fields of the error the browser client reads.""" + token_payload = _encode_realtime_token_payload( + ephemeral_key="fake_upstream_epk", + model_id="gpt-4o-realtime-preview", + user_id=None, + team_id=None, + expires_at=int(time.time()) + 3600, + ) + encrypted_token = encrypt_value_helper(token_payload) + + async def failing_route_request(*args: object, **kwargs: object) -> None: + raise HTTPException( + status_code=404, + detail={"error": "realtime: Invalid model name passed in model=gpt-4o-realtime-preview"}, + ) + + proxy_logging = MagicMock() + proxy_logging.pre_call_hook = AsyncMock(side_effect=mock_pre_call_hook) + proxy_logging.post_call_failure_hook = AsyncMock() + monkeypatch.setattr("litellm.proxy.proxy_server.route_request", failing_route_request) + monkeypatch.setattr("litellm.proxy.proxy_server.add_litellm_data_to_request", mock_add_litellm_data) + monkeypatch.setattr("litellm.proxy.proxy_server.proxy_logging_obj", proxy_logging) + + response = TestClient(proxy_app).post( + "/v1/realtime/calls", + headers={"Authorization": f"Bearer {encrypted_token}"}, + content=b"v=0\r\no=- 0 0 IN IP4 0.0.0.0\r\ns=-\r\n", + ) + + assert response.status_code == 404 + assert (response.json()["error"]["type"], response.json()["error"]["param"]) == ("invalid_request_error", None) diff --git a/tests/test_litellm/proxy/rerank_endpoints/test_endpoints.py b/tests/test_litellm/proxy/rerank_endpoints/test_endpoints.py index 9f11ff6f20d..ea858e04e0f 100644 --- a/tests/test_litellm/proxy/rerank_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/rerank_endpoints/test_endpoints.py @@ -6,11 +6,11 @@ import json from unittest.mock import AsyncMock, MagicMock, patch import pytest -from fastapi import Request, Response +from fastapi import HTTPException, Request, Response import litellm.proxy.common_request_processing as common_request_processing_mod import litellm.proxy.proxy_server as proxy_server_mod -from litellm.proxy._types import UserAPIKeyAuth +from litellm.proxy._types import ProxyException, UserAPIKeyAuth from litellm.proxy.rerank_endpoints.endpoints import rerank from litellm.types.utils import RerankResponse @@ -118,3 +118,55 @@ async def test_rerank_omits_detailed_timing_headers_when_disabled(): fastapi_response = await _call_rerank() assert "x-litellm-timing-llm-api-ms" not in fastapi_response.headers + + +async def _rerank_failure( + failure: Exception, *, raised_before_routing: bool, monkeypatch: pytest.MonkeyPatch +) -> ProxyException: + proxy_logging_obj = MagicMock() + proxy_logging_obj.pre_call_hook = AsyncMock( + side_effect=failure if raised_before_routing else lambda **kwargs: kwargs["data"] + ) + proxy_logging_obj.post_call_failure_hook = AsyncMock() + + async def fake_add_litellm_data_to_request(**kwargs: object) -> object: + return kwargs["data"] + + async def failing_route_request(**kwargs: object) -> None: + raise failure + + monkeypatch.setattr(proxy_server_mod, "add_litellm_data_to_request", fake_add_litellm_data_to_request) + monkeypatch.setattr(proxy_server_mod, "route_request", failing_route_request) + monkeypatch.setattr(proxy_server_mod, "proxy_logging_obj", proxy_logging_obj) + monkeypatch.setattr(proxy_server_mod, "llm_router", MagicMock()) + monkeypatch.setattr(proxy_server_mod, "version", "1.2.3") + + with pytest.raises(ProxyException) as raised: + await rerank( + request=_build_request(), + fastapi_response=Response(), + user_api_key_dict=UserAPIKeyAuth(api_key="sk-test"), + ) + return raised.value + + +@pytest.mark.asyncio +async def test_a_model_the_router_cannot_serve_answers_an_openai_typed_error(monkeypatch: pytest.MonkeyPatch): + """A bare HTTPException carries no type or param, so the tail used to ship the + literal string "None" in both fields.""" + failure = HTTPException(status_code=404, detail={"error": "rerank: Invalid model name passed in model=rerank-model"}) + + error = await _rerank_failure(failure, raised_before_routing=False, monkeypatch=monkeypatch) + + assert (error.type, error.param, error.code) == ("invalid_request_error", None, "404") + + +@pytest.mark.asyncio +async def test_a_rejection_raised_before_routing_keeps_its_own_status(monkeypatch: pytest.MonkeyPatch): + """A ProxyException stores its status as the string ``code``, which the tail used to + miss and rewrap as a 500 while keeping the 4xx type and param.""" + rejection = ProxyException(message="session_id is required", type="bad_request_error", param="session_id", code=400) + + error = await _rerank_failure(rejection, raised_before_routing=True, monkeypatch=monkeypatch) + + assert (error.type, error.param, error.code) == ("bad_request_error", "session_id", "400")