From a4c804b6bbfbbf211f350b92fb13fcd0ee3f1df6 Mon Sep 17 00:00:00 2001 From: songkuan-zheng <252822057+songkuan-zheng@users.noreply.github.com> Date: Wed, 17 Jun 2026 19:28:52 +0800 Subject: [PATCH] fix(spend-logs): preserve error_message on ProxyException failures (#30381) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- litellm/litellm_core_utils/litellm_logging.py | 25 +++--- .../test_litellm_logging.py | 82 +++++++++++++++++++ 2 files changed, 95 insertions(+), 12 deletions(-) diff --git a/litellm/litellm_core_utils/litellm_logging.py b/litellm/litellm_core_utils/litellm_logging.py index 7ece944fd0e..1d6f13aedef 100644 --- a/litellm/litellm_core_utils/litellm_logging.py +++ b/litellm/litellm_core_utils/litellm_logging.py @@ -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, ) diff --git a/tests/test_litellm/litellm_core_utils/test_litellm_logging.py b/tests/test_litellm/litellm_core_utils/test_litellm_logging.py index b3c19a09388..e0d7f22f817 100644 --- a/tests/test_litellm/litellm_core_utils/test_litellm_logging.py +++ b/tests/test_litellm/litellm_core_utils/test_litellm_logging.py @@ -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 # ──────────────────────────────────────────────────────────────────────