diff --git a/litellm/proxy/utils.py b/litellm/proxy/utils.py index 5c0fbd64744..ff5ce75a8c8 100644 --- a/litellm/proxy/utils.py +++ b/litellm/proxy/utils.py @@ -37,7 +37,11 @@ from litellm.proxy._types import ( SpendLogsMetadata, SpendLogsPayload, ) -from litellm.proxy.common_utils.openai_error_payload import openai_error_param +from litellm.proxy.common_utils.openai_error_payload import ( + error_status_code, + openai_error_param, + openai_error_type, +) from litellm.proxy.spend_tracking.spend_log_error_logger import spend_log_error from litellm.types.guardrails import GuardrailEventHooks from litellm.types.proxy.model_listing import ModelInfoResponse @@ -7096,18 +7100,19 @@ def handle_exception_on_proxy(e: Exception) -> ProxyException: verbose_proxy_logger.exception("Exception: %s", e) if isinstance(e, HTTPException): + http_status_code: Final = error_status_code(e, status.HTTP_500_INTERNAL_SERVER_ERROR) return ProxyException( message=getattr(e, "detail", f"error({e})"), - type=ProxyErrorTypes.internal_server_error, + type=openai_error_type(e, http_status_code), param=openai_error_param(e), - code=getattr(e, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR), + code=http_status_code, ) elif isinstance(e, ProxyException): return e - _status_code: Final = getattr(e, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR) + _status_code: Final = error_status_code(e, status.HTTP_500_INTERNAL_SERVER_ERROR) return ProxyException( message=str(e), - type=ProxyErrorTypes.internal_server_error, + type=openai_error_type(e, _status_code), param=openai_error_param(e), code=_status_code, ) diff --git a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py index dcd8e6881bd..fd38c22398a 100644 --- a/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py +++ b/tests/test_litellm/proxy/credential_endpoints/test_endpoints.py @@ -86,3 +86,31 @@ def test_update_credential_still_answers_200_on_a_successful_write(): assert response.status_code == 200, response.text assert response.json()["success"] is True + + +def _get_credential_by_name(name: str): + missing = object() + previous_override = app.dependency_overrides.get(user_api_key_auth, missing) + app.dependency_overrides[user_api_key_auth] = _as_admin + try: + return client.get(f"/credentials/by_name/{name}", headers={"Authorization": "Bearer test-key"}) + finally: + if previous_override is missing: + app.dependency_overrides.pop(user_api_key_auth, None) + else: + app.dependency_overrides[user_api_key_auth] = previous_override + + +def test_looking_up_a_missing_credential_is_labelled_a_client_error_not_a_gateway_failure(): + """Naming a credential that does not exist is the caller's own mistake, and the shared + handler used to label every one of these internal_server_error, so an SDK branching on + the type read a 404 it can never retry past as a gateway outage and backed off forever.""" + response = _get_credential_by_name("definitely-not-there") + + assert response.status_code == 404, f"lookup answered {response.status_code}: {response.text}" + assert response.json()["error"] == { + "message": "Credential not found. Got credential name: definitely-not-there", + "type": "invalid_request_error", + "param": None, + "code": "404", + } diff --git a/tests/test_litellm/proxy/utils/helpers/test_error_helpers.py b/tests/test_litellm/proxy/utils/helpers/test_error_helpers.py index dc30798df55..3333bcc80e5 100644 --- a/tests/test_litellm/proxy/utils/helpers/test_error_helpers.py +++ b/tests/test_litellm/proxy/utils/helpers/test_error_helpers.py @@ -3,6 +3,7 @@ import json import pytest from fastapi import HTTPException +import litellm from litellm.proxy._types import ProxyErrorTypes, ProxyException from litellm.proxy.utils import get_error_message_str, handle_exception_on_proxy @@ -93,7 +94,7 @@ def test_handle_exception_on_proxy_happy_path_http_exception(): assert snapshot == { "is_proxy_exception": True, "message": "forbidden", - "type": ProxyErrorTypes.internal_server_error.value, + "type": "permission_error", "code": "403", } @@ -153,7 +154,7 @@ def test_handle_exception_on_proxy_uses_attached_status_code_when_present(): assert snapshot == { "code": "418", "message": "teapot", - "type": ProxyErrorTypes.internal_server_error.value, + "type": "invalid_request_error", } @@ -171,3 +172,62 @@ def test_handle_exception_on_proxy_error_path_none_input_wraps_as_500(): "code": "500", "type": ProxyErrorTypes.internal_server_error.value, } + + +@pytest.mark.parametrize( + "status_code, expected_type", + [ + (400, "invalid_request_error"), + (401, "authentication_error"), + (403, "permission_error"), + (404, "invalid_request_error"), + (409, "invalid_request_error"), + (422, "invalid_request_error"), + (429, "rate_limit_error"), + (500, ProxyErrorTypes.internal_server_error.value), + (503, ProxyErrorTypes.internal_server_error.value), + ], +) +def test_handle_exception_on_proxy_labels_an_http_exception_by_the_status_it_answers( + status_code: int, expected_type: str +): + """A management route that rejects a caller's own argument answers 400, and calling that + internal_server_error tells the caller's retry loop the gateway is down when nothing but + their request is wrong.""" + result = handle_exception_on_proxy(HTTPException(status_code=status_code, detail="boom")) + + assert (result.code, result.type) == (str(status_code), expected_type) + + +def _exception_carrying_status(carried: int) -> Exception: + class _CarriedStatus(Exception): + status_code = carried + + return _CarriedStatus("boom") + + +@pytest.mark.parametrize( + "status_code, expected_type", + [ + (400, "invalid_request_error"), + (404, "invalid_request_error"), + (429, "rate_limit_error"), + (500, ProxyErrorTypes.internal_server_error.value), + ], +) +def test_handle_exception_on_proxy_labels_a_carried_status_the_same_way(status_code: int, expected_type: str): + """Routes also reach this handler with plain exceptions carrying their own status, and + those must be named by that status rather than all landing on the 500 label.""" + result = handle_exception_on_proxy(_exception_carrying_status(status_code)) + + assert (result.code, result.type) == (str(status_code), expected_type) + + +def test_handle_exception_on_proxy_keeps_the_type_the_exception_already_names(): + """A rate limit litellm raised already names itself, and overwriting that with the status + map's guess would drop what the client branches on.""" + result = handle_exception_on_proxy( + litellm.RateLimitError(message="slow down", llm_provider="openai", model="gpt-4o") + ) + + assert (result.code, result.type) == ("429", "throttling_error")