diff --git a/litellm/proxy/common_utils/openai_error_payload.py b/litellm/proxy/common_utils/openai_error_payload.py index 2f01dbe11f7..3f852ca7fec 100644 --- a/litellm/proxy/common_utils/openai_error_payload.py +++ b/litellm/proxy/common_utils/openai_error_payload.py @@ -45,6 +45,12 @@ def openai_error_type(exc: object, status_code: int) -> str: return openai_error_type_for_status(status_code) +def caller_error_type(exc: object, status_code: int) -> str: + if status_code == status.HTTP_408_REQUEST_TIMEOUT or status_code >= status.HTTP_500_INTERNAL_SERVER_ERROR: + return "internal_server_error" + return openai_error_type(exc, status_code) + + def openai_error_type_for_status(status_code: int) -> str: mapped: Final = _OPENAI_ERROR_TYPE_BY_STATUS.get(status_code) if mapped is not None: diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 824b310a1dd..1f3a882469f 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -7225,12 +7225,7 @@ async def list_keys( except Exception as e: verbose_proxy_logger.exception("Error in list_keys: %s", e) if isinstance(e, HTTPException): - raise ProxyException( - message=getattr(e, "detail", f"error({e})"), - type=ProxyErrorTypes.internal_server_error, - param=getattr(e, "param", "None"), - code=getattr(e, "status_code", fastapi.status.HTTP_500_INTERNAL_SERVER_ERROR), - ) + raise handle_exception_on_proxy(e) elif isinstance(e, ProxyException): raise e raise ProxyException( diff --git a/litellm/proxy/utils.py b/litellm/proxy/utils.py index 28be11509ae..0af73ca3f98 100644 --- a/litellm/proxy/utils.py +++ b/litellm/proxy/utils.py @@ -84,6 +84,8 @@ from litellm.proxy._types import ( from litellm.proxy.bug_report_config import build_proxy_bug_report from litellm.proxy.common_utils.openai_error_payload import ( attribute_of, + caller_error_type, + error_status_code, litellm_call_id_headers, openai_error_param, with_litellm_call_id, @@ -132,7 +134,7 @@ from litellm import ( Router, ) from litellm._internal_context import service_target -from litellm._logging import redact_string, verbose_proxy_logger +from litellm._logging import redact_internal_details_from_client_message, redact_string, verbose_proxy_logger from litellm._service_logger import ServiceLogging, ServiceTypes from litellm.caching.caching import DualCache, RedisCache from litellm.caching.dual_cache import LimitedSizeOrderedDict @@ -9049,21 +9051,22 @@ def handle_exception_on_proxy(e: Exception, litellm_call_id: str | None = None) headers: Final = litellm_call_id_headers(litellm_call_id) 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=caller_error_type(e, http_status_code), param=openai_error_param(e), headers=headers, - code=getattr(e, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR), + code=http_status_code, ) elif isinstance(e, ProxyException): return with_litellm_call_id(e, litellm_call_id) - _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) if should_report_bug(e): verbose_proxy_logger.error(bug_report_notice(build_proxy_bug_report(e))) return ProxyException( - message=strip_bug_report_notice(str(e)), - type=ProxyErrorTypes.internal_server_error, + message=redact_internal_details_from_client_message(strip_bug_report_notice(str(e))), + type=caller_error_type(e, _status_code), param=openai_error_param(e), headers=headers, code=_status_code, diff --git a/tests/integration/authorization/test_guardrail_submissions.py b/tests/integration/authorization/test_guardrail_submissions.py index f120028f46d..07d9a96c979 100644 --- a/tests/integration/authorization/test_guardrail_submissions.py +++ b/tests/integration/authorization/test_guardrail_submissions.py @@ -432,7 +432,7 @@ def test_team_guardrail_submissions_require_admin_approval( f"Guardrail '{first_name}' not found. " "Please ensure the guardrail is configured in your LiteLLM proxy." ), - "type": "internal_server_error", + "type": "invalid_request_error", "param": None, "code": "404", } @@ -481,7 +481,7 @@ def test_team_guardrail_submissions_require_admin_approval( f"Guardrail '{second_name}' not found. " "Please ensure the guardrail is configured in your LiteLLM proxy." ), - "type": "internal_server_error", + "type": "invalid_request_error", "param": None, "code": "404", } diff --git a/tests/integration/authorization/test_user_self_update_guard.py b/tests/integration/authorization/test_user_self_update_guard.py index d83960e1e85..722046ca6bd 100644 --- a/tests/integration/authorization/test_user_self_update_guard.py +++ b/tests/integration/authorization/test_user_self_update_guard.py @@ -279,7 +279,7 @@ def test_new_user_with_admin_role_requires_proxy_admin(gateway: Gateway, role: s assert refused.json() == { "error": { "message": f"Only proxy admins can create administrative users (proxy_admin, proxy_admin_viewer). Attempted to create user with role: {role}. Your role: internal_user", - "type": "internal_server_error", + "type": "permission_error", "param": None, "code": "403", } diff --git a/tests/integration/management/test_user_delete_revokes_keys.py b/tests/integration/management/test_user_delete_revokes_keys.py index 324963a0da9..edbbf550871 100644 --- a/tests/integration/management/test_user_delete_revokes_keys.py +++ b/tests/integration/management/test_user_delete_revokes_keys.py @@ -170,7 +170,7 @@ def test_user_delete_revokes_keys_and_memberships(gateway: Gateway, peer: Gatewa assert info.json() == { "error": { "message": f"User {user_id} not found", - "type": "internal_server_error", + "type": "invalid_request_error", "param": None, "code": "404", } @@ -337,7 +337,7 @@ def test_scim_user_delete_revokes_keys(gateway: Gateway, peer: Gateway, monkeypa expected_info: Final = { "error": { "message": f"User {user_id} not found", - "type": "internal_server_error", + "type": "invalid_request_error", "param": None, "code": "404", } diff --git a/tests/integration/observability/test_apply_guardrail_endpoint.py b/tests/integration/observability/test_apply_guardrail_endpoint.py index 9fa7d1a74db..f50679bd54e 100644 --- a/tests/integration/observability/test_apply_guardrail_endpoint.py +++ b/tests/integration/observability/test_apply_guardrail_endpoint.py @@ -156,7 +156,7 @@ def test_apply_guardrail_returns_a_block_error_for_generic_guardrails(gateway: G assert error.model_dump() == { "error": { "message": _BLOCK_REASON, - "type": "internal_server_error", + "type": "invalid_request_error", "param": None, "code": 400, } diff --git a/tests/unit/proxy/credential_endpoints/test_endpoints.py b/tests/unit/proxy/credential_endpoints/test_endpoints.py index c905d38dd41..3c5343eb7e7 100644 --- a/tests/unit/proxy/credential_endpoints/test_endpoints.py +++ b/tests/unit/proxy/credential_endpoints/test_endpoints.py @@ -3014,3 +3014,20 @@ class TestCredentialDisplayName: assert (by_name["display_name"], by_name["source"]) == ("Prod OpenAI", "db") assert config_by_name["source"] == "config" assert listed["openai-prod"]["credential_values"]["api_key"] != "sk-old" + + +def test_looking_up_a_missing_credential_is_labelled_a_client_error_not_a_gateway_failure( + restore_credential_list: None, +): + """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: Final = _call_as("GET", "/credentials/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/unit/proxy/management_endpoints/test_key_management_endpoints.py b/tests/unit/proxy/management_endpoints/test_key_management_endpoints.py index d6ee9b1d29e..cafb96735c5 100644 --- a/tests/unit/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/unit/proxy/management_endpoints/test_key_management_endpoints.py @@ -6540,7 +6540,7 @@ async def test_list_keys_with_invalid_status(): ) # Verify ProxyException properties - assert exc_info.value.code == "400" + assert (exc_info.value.code, exc_info.value.type) == ("400", "invalid_request_error") assert "Invalid status value" in str(exc_info.value.message) assert "deleted" in str(exc_info.value.message) @@ -18733,7 +18733,7 @@ async def test_list_keys_rejects_invalid_expires(): expires="expred", ) - assert exc_info.value.code == "400" + assert (exc_info.value.code, exc_info.value.type) == ("400", "invalid_request_error") assert "Invalid expires value" in str(exc_info.value.message) diff --git a/tests/unit/proxy/utils/helpers/test_error_helpers.py b/tests/unit/proxy/utils/helpers/test_error_helpers.py index df399c8b1d2..a6667f7a46d 100644 --- a/tests/unit/proxy/utils/helpers/test_error_helpers.py +++ b/tests/unit/proxy/utils/helpers/test_error_helpers.py @@ -1,11 +1,15 @@ import asyncio import json +from typing import Final from unittest.mock import AsyncMock, MagicMock +import httpx +import openai import pytest from fastapi import HTTPException from prisma.errors import DataError +import litellm from litellm.proxy._types import ProxyErrorTypes, ProxyException from litellm.proxy.utils import get_error_message_str, handle_exception_on_proxy @@ -96,7 +100,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", } @@ -156,7 +160,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", } @@ -176,6 +180,112 @@ def test_handle_exception_on_proxy_error_path_none_input_wraps_as_500(): } +@pytest.mark.parametrize( + "status_code, expected_type", + [ + (400, "invalid_request_error"), + (401, "authentication_error"), + (403, "permission_error"), + (404, "invalid_request_error"), + (408, ProxyErrorTypes.internal_server_error.value), + (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: Final = 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 | None) -> 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"), + (408, ProxyErrorTypes.internal_server_error.value), + (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: Final = 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: Final = handle_exception_on_proxy( + litellm.RateLimitError(message="slow down", llm_provider="openai", model="gpt-4o") + ) + + assert (result.code, result.type) == ("429", "throttling_error") + + +@pytest.mark.parametrize( + "exc, expected_code", + [ + ( + openai.InternalServerError( + message="upstream down", + response=httpx.Response(500, request=httpx.Request("GET", "https://api.openai.com/v1/batches/b")), + body={"type": "server_error"}, + ), + "500", + ), + (litellm.Timeout(message="took too long", model="gpt-6.1-sol", llm_provider="openai"), "408"), + ], + ids=["provider_5xx_naming_its_own_type", "provider_timeout"], +) +def test_handle_exception_on_proxy_keeps_internal_server_error_for_failures_the_caller_cannot_fix( + exc: Exception, expected_code: str +): + """Batch and fine-tuning routes pass provider errors straight through, and a retry loop + that treats internal_server_error as transient must keep retrying a provider outage or a + timeout instead of reading the provider's own type or a fix-your-request label.""" + result: Final = handle_exception_on_proxy(exc) + + assert (result.code, result.type) == (expected_code, ProxyErrorTypes.internal_server_error.value) + + +def test_handle_exception_on_proxy_redacts_the_internal_host_an_unreachable_upstream_names(): + """A guardrail or provider the gateway cannot reach names its private address in the + error, and any virtual key can call those routes, so the host must not reach the caller.""" + result: Final = handle_exception_on_proxy(ConnectionError("Cannot connect to host 10.12.4.7:8443")) + + assert result.code == "500" + assert "Cannot connect to host" in result.message + assert "10.12.4.7" not in result.message + + +def test_handle_exception_on_proxy_answers_500_for_an_exception_whose_status_is_none(): + """A guardrail whose upstream refuses the connection raises with status_code=None, and + passing that through as the code crashed the proxy's error handler, which then blamed the + auth database with a 503 instead of naming the unreachable guardrail.""" + result: Final = handle_exception_on_proxy(_exception_carrying_status(None)) + + assert (result.code, result.type, result.message) == ("500", ProxyErrorTypes.internal_server_error.value, "boom") + + @pytest.mark.parametrize( "exc", [