Skip to content

fix: handle ValueError and RecursionError in _safe_json_serialize#5414

Open
voidborne-d wants to merge 1 commit intogoogle:mainfrom
voidborne-d:fix/safe-json-serialize-exception-handling
Open

fix: handle ValueError and RecursionError in _safe_json_serialize#5414
voidborne-d wants to merge 1 commit intogoogle:mainfrom
voidborne-d:fix/safe-json-serialize-exception-handling

Conversation

@voidborne-d
Copy link
Copy Markdown

Summary

Both _safe_json_serialize implementations only catch TypeError and OverflowError from json.dumps, but json.dumps can also raise:

  • ValueError: circular references (e.g. obj.ref = obj)
  • RecursionError: deeply nested structures exceeding Python's recursion limit

Impact

telemetry/tracing.py (critical): _safe_json_serialize is called inside a finally block during trace_tool_call. An unhandled exception here can suppress the original tool execution result — telemetry failures should never affect runtime correctness.

models/lite_llm.py: An unhandled exception during message serialization causes hard LLM request construction failures where a graceful fallback is expected.

Root Cause

The except clause catches (TypeError, OverflowError) but not ValueError or RecursionError. Both are documented json.dumps failure modes:

  • ValueError for circular references (per Python docs)
  • RecursionError for structures exceeding sys.getrecursionlimit()

Note: in tracing.py, the default=lambda o: ... callback handles TypeError for individual non-serializable objects, but circular references and deep recursion bypass the default callback entirely.

Fix

  • Add ValueError and RecursionError to the exception tuple in both implementations.
  • In lite_llm.py, wrap the str() fallback in a secondary try/except RecursionError since str()__repr__() can also fail on deeply recursive objects.

Reproduction

class Node:
    def __init__(self):
        self.ref = self

obj = Node()
# Before fix: raises ValueError
# After fix: returns fallback string
_safe_json_serialize(obj)

Tests

8 regression tests added across both modules:

  • test_safe_json_serialize.py (telemetry): circular ref, deep nesting, normal dict, non-serializable object
  • test_litellm_safe_serialize.py (models): circular ref, deep nesting, normal dict, non-serializable object

All 8 tests pass.

Fixes #5411
Fixes #5412

@adk-bot adk-bot added models [Component] Issues related to model support tracing [Component] This issue is related to OpenTelemetry tracing labels Apr 20, 2026
Both _safe_json_serialize implementations only catch TypeError and
OverflowError from json.dumps, but json.dumps can also raise:

- ValueError: circular references (e.g. object referencing itself)
- RecursionError: deeply nested structures exceeding recursion limit

In telemetry/tracing.py, this is especially critical because
_safe_json_serialize is called inside a finally block during
trace_tool_call — an unhandled exception here can suppress the
original tool execution result.

In models/lite_llm.py, an unhandled exception during message
serialization causes hard LLM request construction failures where
a graceful fallback is expected.

Fix:
- Add ValueError and RecursionError to the exception tuple in both
  implementations.
- In lite_llm.py, wrap the str() fallback in a secondary try/except
  RecursionError since str() can also fail on deeply recursive objects.

8 regression tests added across both modules.

Fixes google#5411
Fixes google#5412
@voidborne-d voidborne-d force-pushed the fix/safe-json-serialize-exception-handling branch from 17dc740 to 87499ee Compare April 20, 2026 22:06
@rohityan rohityan self-assigned this Apr 20, 2026
@rohityan rohityan added request clarification [Status] The maintainer need clarification or more information from the author and removed models [Component] Issues related to model support labels Apr 20, 2026
@rohityan
Copy link
Copy Markdown
Collaborator

Hi @voidborne-d , Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors by running autoformat.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author tracing [Component] This issue is related to OpenTelemetry tracing

Projects

None yet

3 participants