[nileshpatil6]

Problem

_safe_json_serialize in telemetry/tracing.py only catches TypeError and OverflowError, but json.dumps can also raise:

• ValueError — e.g. circular references (json.dumps raises ValueError: Circular reference detected)
• RecursionError — e.g. deeply nested structures exceeding Python's recursion limit

Because this function is called inside finally blocks during tool execution telemetry, an uncaught exception here can interrupt normal execution flow and affect tool results.

Fixes #5411

Fix

Add ValueError and RecursionError to the existing except clause:

except (TypeError, OverflowError, ValueError, RecursionError):
    return '<not serializable>'

[nileshpatil6]

I have read the Contributor License Agreement and I hereby agree to its terms.

[adk-bot]

Response from ADK Triaging Agent

Hello @nileshpatil6, thank you for creating this PR!

Could you please include a testing plan section in your PR to describe how you tested your changes? This will help reviewers to review your PR more efficiently. Thanks!

[nileshpatil6]

Testing Plan

Manual verification:

import json
from google.adk.telemetry.tracing import _safe_json_serialize

# Test 1: Circular reference (previously raised ValueError)
circular = {}
circular['self'] = circular
assert _safe_json_serialize(circular) == '<not serializable>'

# Test 2: Deeply nested structure (previously raised RecursionError)
deep = {}
node = deep
for _ in range(1000):
    node['child'] = {}
    node = node['child']
result = _safe_json_serialize(deep)
assert result == '<not serializable>'

# Test 3: Normal objects still serialize correctly
assert _safe_json_serialize({'key': 'value'}) == '{"key": "value"}'

print('All tests passed')

Before fix: _safe_json_serialize with a circular reference raises ValueError: Circular reference detected, which propagates out of the finally block and crashes tool execution.

After fix: Returns '<not serializable>' gracefully for all failure modes.

[rohityan]

Hi @nileshpatil6 , Thank you for your contribution through this pull request! This PR has merge conflicts that require changes from your end. Could you please rebase your branch with the latest main branch to address these? Once this is complete, please let us know so we can proceed with the review.

[nileshpatil6]

Rebased on latest main. I noticed upstream already included ValueError in the except clause -- our change now only adds RecursionError to guard against deeply nested structures causing recursion during serialization. Please proceed with review when you get a chance.

[rohityan]

Hi @nileshpatil6 , please resolve the merge conflicts

[nileshpatil6]

hi @rohityan, resolved the conflicts and merged latest main in. the only change here now is adding RecursionError to the except clause in _safe_json_serialize (ValueError already got covered upstream in the meantime). kept main's new error_type param untouched while resolving. serialize tests pass locally. ready for review whenever you get a chance.

[haranrk]

Thanks for this fix and the thorough write-up, @nileshpatil6!

This has since been addressed at HEAD. The RecursionError guard was added to all of the safe-JSON serialization helpers — telemetry/_serialization.py, telemetry/_experimental_semconv.py, and models/lite_llm.py — in commit 5515800 (5515800), which closed #5411 and also added deterministic regression tests for the RecursionError path. That change corresponds to internal cl/933161771.

Since #5411 is now resolved on main, this PR is redundant, so I'm closing it. I'm leaving the branch intact. Thanks again for taking the time to report and fix this — it's much appreciated!