mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
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.
This commit is contained in:
parent
edde95197a
commit
8b89c909a9
7 changed files with 230 additions and 15 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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."""
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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")
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue