mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-08 22:21:35 +00:00
fix(spend-logs): preserve error_message on ProxyException failures (#30381)
* fix(spend-logs): preserve error_message on ProxyException failures
`StandardLoggingPayloadSetup.get_error_information` used
`str(original_exception)` to populate the human-readable error message
stored in `spend_logs.metadata.error_information.error_message`.
`ProxyException` (litellm/proxy/_types.py:3453) sets `self.message` in
its constructor but does NOT call `super().__init__(message)` and does
NOT define `__str__`. As a result, `str(ProxyException(...))` returns
the empty string, and every auth/budget/quota rejection was landing
in spend_logs with `error_message=""` despite a fully populated
traceback.
Operator impact: dashboard "LLM Failure" rows became untriageable —
the only way to tell a 401 from a 429 was to manually unpack the
traceback JSON via psql. Burst failure patterns (e.g. a UI session
polling with a stale token) produced 20-30 indistinguishable
`error_code=401` rows per second.
Fix: prefer the `.message` attribute (set by ProxyException and every
litellm.exceptions.* class) over `str(exc)`. The `str(exc)` fallback
is retained for non-litellm exception types, preserving prior behavior.
Test plan:
- 2 new unit tests in tests/test_litellm/litellm_core_utils/
test_litellm_logging.py:
* test_get_error_information_prefers_message_attribute_over_str
* test_get_error_information_falls_back_to_str_when_no_message_attr
- Existing test_get_error_information_error_code_priority still passes
- End-to-end verified: bad-key 401 now stores full
"Authentication Error, Invalid proxy server token passed..."
message in spend_logs.metadata.error_information.error_message
* fix(spend-logs): preserve explicit empty .message + drop dead reference
Greptile P2 on #30381. The truthiness check `if message_attr:`
silently skipped an explicit empty-string `.message` and fell
through to `str(original_exception)`. For ProxyException-shaped
objects both produce empty, so the bug was latent; for other
exception types it would inject a different string into
error_information.error_message and corrupt the signal.
Use `is not None` so an empty string survives verbatim.
Also drop the stale `See e2e/cases/11.` comment reference — that
path does not exist anywhere in the repo and confuses future
readers.
Regression test added: an exception with `.message=""` and a
non-empty `super().__init__()` arg must yield error_message == "".
* ci: retrigger workflows after base branch change to litellm_internal_staging
This commit is contained in:
parent
67662565e8
commit
a4c804b6bb
2 changed files with 95 additions and 12 deletions
|
|
@ -5416,19 +5416,20 @@ class StandardLoggingPayloadSetup:
|
|||
tb_lines[:MAXIMUM_TRACEBACK_LINES_TO_LOG]
|
||||
) # Limit to first 100 lines
|
||||
|
||||
# Prefer the `.message` attribute (set by ProxyException and every
|
||||
# litellm.exceptions.* class) over str(exc); ProxyException does not
|
||||
# call super().__init__() nor define __str__, so str() on it returns
|
||||
# an empty string, which used to silently strip the human-readable
|
||||
# message from spend_logs.metadata.error_information.
|
||||
# Use isinstance, not truthiness: an explicit empty string on
|
||||
# `.message` is a deliberate value and must not be replaced by
|
||||
# `str(exc)`.
|
||||
explicit_message = getattr(original_exception, "message", None)
|
||||
error_message = (
|
||||
explicit_message
|
||||
if isinstance(explicit_message, str) and explicit_message
|
||||
else str(original_exception)
|
||||
)
|
||||
if isinstance(explicit_message, str):
|
||||
error_message = explicit_message
|
||||
else:
|
||||
error_message = str(original_exception) if original_exception else ""
|
||||
|
||||
# Duck-typed read so bare-Exception subclasses like
|
||||
# `litellm.BudgetExceededError` can participate without joining the
|
||||
# RateLimitError hierarchy (which would break `except BudgetExceededError`).
|
||||
# Validated against the enum value sets so a third-party exception that
|
||||
# happens to declare a `.category` or `.rate_limit_type` string attribute
|
||||
# can't leak garbage into the payload or Prometheus label cardinality.
|
||||
rate_limit_category = validate_rate_limit_category(
|
||||
getattr(original_exception, "category", None)
|
||||
)
|
||||
|
|
@ -5441,7 +5442,7 @@ class StandardLoggingPayloadSetup:
|
|||
error_class=error_class,
|
||||
llm_provider=_llm_provider_in_exception,
|
||||
traceback=traceback_info,
|
||||
error_message=error_message if original_exception else "",
|
||||
error_message=error_message,
|
||||
error_rate_limit_category=rate_limit_category,
|
||||
error_rate_limit_type=rate_limit_type,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -2116,6 +2116,88 @@ def test_get_error_information_error_code_priority():
|
|||
assert result["error_class"] == "NoCodeException"
|
||||
|
||||
|
||||
def test_get_error_information_prefers_message_attribute_over_str():
|
||||
"""
|
||||
Regression for empty-error_message-in-spend-logs.
|
||||
|
||||
ProxyException sets `self.message` but does NOT call
|
||||
`super().__init__(message)` nor define `__str__`, so `str(exc)`
|
||||
returns the empty string. Before the fix, get_error_information
|
||||
used `str(original_exception)` and silently stripped the
|
||||
human-readable message from spend_logs.metadata.error_information,
|
||||
making dashboard "LLM Failure" rows un-triagable.
|
||||
|
||||
Asserts the `.message` attribute is consulted first.
|
||||
"""
|
||||
from litellm.litellm_core_utils.litellm_logging import StandardLoggingPayloadSetup
|
||||
|
||||
# Simulate a ProxyException-shaped exception: .message set, but
|
||||
# super().__init__() NOT called and no __str__ override.
|
||||
class ProxyExceptionLike(Exception):
|
||||
def __init__(self, message, code):
|
||||
self.message = str(message)
|
||||
self.code = str(code)
|
||||
# NOTE: deliberately NOT calling super().__init__(message)
|
||||
|
||||
msg = "Authentication Error, Invalid proxy server token passed. key=..."
|
||||
exc = ProxyExceptionLike(message=msg, code=401)
|
||||
|
||||
# Sanity check: this exception type's str() really is empty
|
||||
assert str(exc) == "", (
|
||||
"Test premise broken — bare-base Exception now returns message; "
|
||||
"review whether ProxyException fix landed at the class level instead"
|
||||
)
|
||||
|
||||
result = StandardLoggingPayloadSetup.get_error_information(exc)
|
||||
assert (
|
||||
result["error_message"] == msg
|
||||
), f"expected message from .message attribute, got {result['error_message']!r}"
|
||||
assert result["error_code"] == "401"
|
||||
assert result["error_class"] == "ProxyExceptionLike"
|
||||
|
||||
|
||||
def test_get_error_information_preserves_explicit_empty_message():
|
||||
"""
|
||||
An exception that deliberately sets `.message = ""` must surface
|
||||
the empty string verbatim, not fall through to `str(exc)`.
|
||||
|
||||
Regression for greptile P2 finding on PR #30381: a truthiness
|
||||
check (`if message_attr:`) would silently mask an explicit empty
|
||||
message and substitute `str(original_exception)` — which for
|
||||
ProxyException-shaped objects is also empty, but for plain
|
||||
`Exception("boom")` would inject the wrong string and corrupt
|
||||
the error_information signal.
|
||||
"""
|
||||
from litellm.litellm_core_utils.litellm_logging import StandardLoggingPayloadSetup
|
||||
|
||||
class ProxyExceptionLike(Exception):
|
||||
def __init__(self, message, code):
|
||||
self.message = message
|
||||
self.code = str(code)
|
||||
super().__init__("unrelated-args-summary")
|
||||
|
||||
exc = ProxyExceptionLike(message="", code=500)
|
||||
result = StandardLoggingPayloadSetup.get_error_information(exc)
|
||||
assert result["error_message"] == "", (
|
||||
"explicit empty .message must survive verbatim; got "
|
||||
f"{result['error_message']!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_get_error_information_falls_back_to_str_when_no_message_attr():
|
||||
"""
|
||||
Plain Exception (no `.message` attr) must still produce a useful
|
||||
error_message via str(exc), preserving prior behavior for
|
||||
non-litellm exception types.
|
||||
"""
|
||||
from litellm.litellm_core_utils.litellm_logging import StandardLoggingPayloadSetup
|
||||
|
||||
exc = ValueError("boom")
|
||||
result = StandardLoggingPayloadSetup.get_error_information(exc)
|
||||
assert result["error_message"] == "boom"
|
||||
assert result["error_class"] == "ValueError"
|
||||
|
||||
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
# Tests for _get_assembled_streaming_response non-streaming early return
|
||||
# ──────────────────────────────────────────────────────────────────────
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue