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 # ──────────────────────────────────────────────────────────────────────