mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-08 03:08:45 +00:00
fix(proxy): stop putting the literal string "None" in error payloads
A blocked guardrail (and any other HTTP error the proxy converts) came back with "type": "None" and "param": "None", because the converters passed the string "None" as the getattr default instead of None. OpenAI types error.type as a required string and error.param as nullable, so type now falls back to the type its status code stands for and param serializes as JSON null. Covers the non-streaming body, the SSE error frame, the client-disconnect frame, and the unclassified-exception path, so every unified LLM endpoint and the anthropic endpoints return the same shape.
This commit is contained in:
mateo-berri
parent
658f50663d
commit
1b4d2e25db
2 changed files with 174 additions and 22 deletions
|
|
@ -467,6 +467,42 @@ def _getattr_object(value: object, name: str, default: object = None) -> object:
|
|||
return getattr(value, name, default)
|
||||
|
||||
|
||||
_OPENAI_ERROR_TYPE_BY_STATUS: Final[Mapping[int, str]] = MappingProxyType(
|
||||
{
|
||||
status.HTTP_401_UNAUTHORIZED: "authentication_error",
|
||||
status.HTTP_403_FORBIDDEN: "permission_error",
|
||||
status.HTTP_429_TOO_MANY_REQUESTS: "rate_limit_error",
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
def _error_status_code(exc: object, default: int) -> int:
|
||||
"""The HTTP status an exception carries, or ``default`` when it carries none."""
|
||||
carried: Final = _getattr_object(exc, "status_code")
|
||||
return carried if isinstance(carried, int) and not isinstance(carried, bool) else default
|
||||
|
||||
|
||||
def _openai_error_type(exc: object, status_code: int) -> str:
|
||||
"""OpenAI types ``error.type`` as a required string, so an exception carrying none
|
||||
falls back to the type its status code stands for."""
|
||||
carried: Final = _getattr_object(exc, "type")
|
||||
if isinstance(carried, str):
|
||||
return carried
|
||||
mapped: Final = _OPENAI_ERROR_TYPE_BY_STATUS.get(status_code)
|
||||
if mapped is not None:
|
||||
return mapped
|
||||
if status_code < status.HTTP_500_INTERNAL_SERVER_ERROR:
|
||||
return "invalid_request_error"
|
||||
return "internal_server_error"
|
||||
|
||||
|
||||
def _openai_error_param(exc: object) -> str | None:
|
||||
"""OpenAI types ``error.param`` as nullable, so an exception carrying none
|
||||
serializes as JSON ``null``."""
|
||||
carried: Final = _getattr_object(exc, "param")
|
||||
return carried if isinstance(carried, str) else None
|
||||
|
||||
|
||||
class _UpstreamHttpResponse(Protocol):
|
||||
@property
|
||||
def status_code(self) -> int: ...
|
||||
|
|
@ -540,11 +576,12 @@ def proxy_exception_from_http_exception(exc: HTTPException, headers: dict[str, s
|
|||
message, structured_fields = serialize_http_exception_detail(raw_detail)
|
||||
existing_fields: Final = getattr(exc, "provider_specific_fields", None) or {}
|
||||
merged_fields: Final = {**existing_fields, **structured_fields} if structured_fields else (existing_fields or None)
|
||||
error_status: Final = _error_status_code(exc, status.HTTP_400_BAD_REQUEST)
|
||||
return ProxyException(
|
||||
message=message,
|
||||
type=getattr(exc, "type", "None"),
|
||||
param=getattr(exc, "param", "None"),
|
||||
code=getattr(exc, "status_code", status.HTTP_400_BAD_REQUEST),
|
||||
type=_openai_error_type(exc, error_status),
|
||||
param=_openai_error_param(exc),
|
||||
code=error_status,
|
||||
provider_specific_fields=merged_fields,
|
||||
headers=headers,
|
||||
)
|
||||
|
|
@ -827,25 +864,22 @@ def sse_error_payload(exc: BaseException) -> tuple[int, Mapping[str, object]]:
|
|||
are byte-identical.
|
||||
"""
|
||||
# Preserve status code from HTTPException (e.g. guardrail blocks)
|
||||
error_status: Final = getattr(exc, "status_code", status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
error_status: Final = _error_status_code(exc, status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
raw_detail: Final = _getattr_object(exc, "detail", "Error processing stream start")
|
||||
message, structured_fields = serialize_http_exception_detail(raw_detail)
|
||||
|
||||
existing_fields: Final = getattr(exc, "provider_specific_fields", None) or {}
|
||||
merged_fields: Final = {**existing_fields, **structured_fields} if structured_fields else (existing_fields or None)
|
||||
|
||||
# Built in one statement then given its one optional key, rather than spread
|
||||
# conditionally: the spread form costs two extra dict constructions, which
|
||||
# type-discipline-budget.json's LIT002 ceiling has no room for.
|
||||
error_obj: Final = {
|
||||
"message": message,
|
||||
"type": getattr(exc, "type", "None"),
|
||||
"param": getattr(exc, "param", "None"),
|
||||
"type": _openai_error_type(exc, error_status),
|
||||
"param": _openai_error_param(exc),
|
||||
"code": str(error_status),
|
||||
}
|
||||
if merged_fields:
|
||||
error_obj["provider_specific_fields"] = merged_fields
|
||||
return error_status, error_obj
|
||||
if not merged_fields:
|
||||
return error_status, error_obj
|
||||
return error_status, {**error_obj, "provider_specific_fields": merged_fields}
|
||||
|
||||
|
||||
def _sse_error_frames(error_obj: Mapping[str, object]) -> tuple[str, str]:
|
||||
|
|
@ -922,7 +956,7 @@ async def create_response(
|
|||
"error": {
|
||||
"message": _CLIENT_DISCONNECT_DETAIL,
|
||||
"type": "client_disconnect",
|
||||
"param": "None",
|
||||
"param": None,
|
||||
"code": str(LITELLM_HTTP_STATUS_CLIENT_DISCONNECTED),
|
||||
}
|
||||
},
|
||||
|
|
@ -3417,8 +3451,8 @@ class ProxyBaseLLMRequestProcessing:
|
|||
_code = status.HTTP_500_INTERNAL_SERVER_ERROR
|
||||
raise ProxyException(
|
||||
message=redact_internal_details_from_client_message(getattr(e, "message", error_msg)),
|
||||
type=getattr(e, "type", "None"),
|
||||
param=getattr(e, "param", "None"),
|
||||
type=_openai_error_type(e, _code),
|
||||
param=_openai_error_param(e),
|
||||
openai_code=getattr(e, "code", None),
|
||||
code=_code,
|
||||
provider_specific_fields=getattr(e, "provider_specific_fields", None),
|
||||
|
|
@ -3628,11 +3662,12 @@ class ProxyBaseLLMRequestProcessing:
|
|||
|
||||
if isinstance(e, HTTPException):
|
||||
raise e
|
||||
stream_error_status: Final = _error_status_code(e, status.HTTP_500_INTERNAL_SERVER_ERROR)
|
||||
proxy_exception: Final = ProxyException(
|
||||
message=redact_internal_details_from_client_message(getattr(e, "message", str(e))),
|
||||
type=getattr(e, "type", "None"),
|
||||
param=getattr(e, "param", "None"),
|
||||
code=getattr(e, "status_code", 500),
|
||||
type=_openai_error_type(e, stream_error_status),
|
||||
param=_openai_error_param(e),
|
||||
code=stream_error_status,
|
||||
)
|
||||
stream_completed = True
|
||||
yield serialize_error(proxy_exception)
|
||||
|
|
|
|||
|
|
@ -1540,8 +1540,8 @@ class TestCommonRequestProcessingHelpers:
|
|||
expected_error_data = {
|
||||
"error": {
|
||||
"message": "Error processing stream start",
|
||||
"type": "None",
|
||||
"param": "None",
|
||||
"type": "internal_server_error",
|
||||
"param": None,
|
||||
"code": str(status.HTTP_500_INTERNAL_SERVER_ERROR),
|
||||
}
|
||||
}
|
||||
|
|
@ -1569,8 +1569,8 @@ class TestCommonRequestProcessingHelpers:
|
|||
expected_error_data = {
|
||||
"error": {
|
||||
"message": "Content blocked by guardrail",
|
||||
"type": "None",
|
||||
"param": "None",
|
||||
"type": "invalid_request_error",
|
||||
"param": None,
|
||||
"code": "400",
|
||||
}
|
||||
}
|
||||
|
|
@ -1934,6 +1934,104 @@ class TestCommonRequestProcessingHelpers:
|
|||
assert mock_tracer.trace.call_count == 0
|
||||
|
||||
|
||||
def _stringified_none_paths(node: object, path: str = "error") -> tuple[str, ...]:
|
||||
if isinstance(node, dict):
|
||||
return tuple(
|
||||
found
|
||||
for key, value in node.items()
|
||||
for found in _stringified_none_paths(value, f"{path}.{key}")
|
||||
)
|
||||
if isinstance(node, (list, tuple)):
|
||||
return tuple(
|
||||
found
|
||||
for index, value in enumerate(node)
|
||||
for found in _stringified_none_paths(value, f"{path}[{index}]")
|
||||
)
|
||||
return (path,) if node == "None" else ()
|
||||
|
||||
|
||||
def _blocked_guardrail_exception() -> HTTPException:
|
||||
return HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": "Violated guardrail policy",
|
||||
"bedrock_guardrail_response": {"action": "GUARDRAIL_INTERVENED"},
|
||||
"guardrailIdentifier": "gf3sc1mzinjw",
|
||||
"guardrailVersion": "DRAFT",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
class TestGuardrailBlockErrorPayloadNeverStringifiesNone:
|
||||
"""Regression for LIT-6808: a blocked-guardrail error body carried the literal string
|
||||
"None" for type and param instead of a real error type and JSON null."""
|
||||
|
||||
def test_non_streaming_block_payload_carries_a_real_type_and_null_param(self):
|
||||
from litellm.proxy.common_request_processing import (
|
||||
proxy_exception_from_http_exception,
|
||||
)
|
||||
|
||||
payload = json.loads(
|
||||
json.dumps(proxy_exception_from_http_exception(_blocked_guardrail_exception(), {}).to_dict())
|
||||
)
|
||||
|
||||
assert _stringified_none_paths(payload) == ()
|
||||
assert payload["type"] == "invalid_request_error"
|
||||
assert payload["param"] is None
|
||||
assert payload["code"] == "400"
|
||||
assert payload["message"] == "Violated guardrail policy"
|
||||
|
||||
def test_streaming_block_frame_carries_a_real_type_and_null_param(self):
|
||||
from litellm.proxy.common_request_processing import sse_error_payload
|
||||
|
||||
error_status, error_obj = sse_error_payload(_blocked_guardrail_exception())
|
||||
frame = json.loads(json.dumps({"error": dict(error_obj)}))
|
||||
|
||||
assert error_status == 400
|
||||
assert _stringified_none_paths(frame["error"]) == ()
|
||||
assert frame["error"]["type"] == "invalid_request_error"
|
||||
assert frame["error"]["param"] is None
|
||||
assert frame["error"]["code"] == "400"
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"status_code, expected_type",
|
||||
[
|
||||
(400, "invalid_request_error"),
|
||||
(401, "authentication_error"),
|
||||
(403, "permission_error"),
|
||||
(404, "invalid_request_error"),
|
||||
(429, "rate_limit_error"),
|
||||
(500, "internal_server_error"),
|
||||
(503, "internal_server_error"),
|
||||
],
|
||||
)
|
||||
def test_status_code_decides_the_type_when_the_exception_carries_none(self, status_code, expected_type):
|
||||
from litellm.proxy.common_request_processing import (
|
||||
proxy_exception_from_http_exception,
|
||||
)
|
||||
|
||||
payload = proxy_exception_from_http_exception(
|
||||
HTTPException(status_code=status_code, detail="blocked"), {}
|
||||
).to_dict()
|
||||
|
||||
assert payload["type"] == expected_type
|
||||
assert payload["param"] is None
|
||||
|
||||
def test_a_type_and_param_the_exception_carries_win_over_the_fallback(self):
|
||||
from litellm.proxy.common_request_processing import (
|
||||
proxy_exception_from_http_exception,
|
||||
)
|
||||
|
||||
exc = HTTPException(status_code=400, detail="unknown model")
|
||||
exc.type = "authentication_error"
|
||||
exc.param = "model"
|
||||
|
||||
payload = proxy_exception_from_http_exception(exc, {}).to_dict()
|
||||
|
||||
assert payload["type"] == "authentication_error"
|
||||
assert payload["param"] == "model"
|
||||
|
||||
|
||||
class TestExtractErrorFromSSEChunk:
|
||||
"""Tests for _extract_error_from_sse_chunk function"""
|
||||
|
||||
|
|
@ -2999,6 +3097,25 @@ class TestHandleLLMApiExceptionDictDetail:
|
|||
assert proxy_exc.message == "Content blocked by guardrail"
|
||||
assert proxy_exc.provider_specific_fields is None
|
||||
|
||||
async def test_blocked_guardrail_error_body_never_carries_the_string_none(self):
|
||||
"""Regression for LIT-6808: the error body a blocked request returns must carry a real
|
||||
error type and JSON null rather than the literal string "None"."""
|
||||
proxy_exc = await self._invoke(_blocked_guardrail_exception())
|
||||
payload = json.loads(json.dumps(proxy_exc.to_dict()))
|
||||
|
||||
assert _stringified_none_paths(payload) == ()
|
||||
assert payload["type"] == "invalid_request_error"
|
||||
assert payload["param"] is None
|
||||
|
||||
async def test_unclassified_exception_error_body_never_carries_the_string_none(self):
|
||||
"""The same holds on the generic fallback, where nothing carries a type at all."""
|
||||
proxy_exc = await self._invoke(ValueError("Something broke"))
|
||||
payload = json.loads(json.dumps(proxy_exc.to_dict()))
|
||||
|
||||
assert _stringified_none_paths(payload) == ()
|
||||
assert payload["type"] == "internal_server_error"
|
||||
assert payload["param"] is None
|
||||
|
||||
async def test_not_found_error_preserves_404(self):
|
||||
"""NotFoundError with status_code=404 should map to ProxyException code=404."""
|
||||
from litellm.exceptions import NotFoundError
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue