Skip to content

Commit 87499ee

Browse files
committed
fix: handle ValueError and RecursionError in _safe_json_serialize
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 #5411 Fixes #5412
1 parent 6380f6a commit 87499ee

File tree

4 files changed

+138
-3
lines changed

4 files changed

+138
-3
lines changed

src/google/adk/models/lite_llm.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -543,8 +543,11 @@ def _safe_json_serialize(obj) -> str:
543543
try:
544544
# Try direct JSON serialization first
545545
return json.dumps(obj, ensure_ascii=False)
546-
except (TypeError, OverflowError):
547-
return str(obj)
546+
except (TypeError, OverflowError, ValueError, RecursionError):
547+
try:
548+
return str(obj)
549+
except RecursionError:
550+
return '<non-serializable: recursion depth exceeded>'
548551

549552

550553
def _part_has_payload(part: types.Part) -> bool:

src/google/adk/telemetry/tracing.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ def _safe_json_serialize(obj) -> str:
128128
return json.dumps(
129129
obj, ensure_ascii=False, default=lambda o: '<not serializable>'
130130
)
131-
except (TypeError, OverflowError):
131+
except (TypeError, OverflowError, ValueError, RecursionError):
132132
return '<not serializable>'
133133

134134

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests for _safe_json_serialize in models/lite_llm.py.
16+
17+
Verifies that the function never raises exceptions, even for inputs that
18+
cause json.dumps to raise ValueError (circular references) or
19+
RecursionError (deeply nested structures).
20+
21+
Fixes https://github.com/google/adk-python/issues/5412
22+
"""
23+
24+
import pytest
25+
26+
from google.adk.models.lite_llm import _safe_json_serialize
27+
28+
29+
def test_circular_reference_returns_str_fallback():
30+
"""json.dumps raises ValueError on circular references; should fall back to str()."""
31+
32+
class Node:
33+
def __init__(self):
34+
self.ref = self
35+
36+
obj = Node()
37+
result = _safe_json_serialize(obj)
38+
assert isinstance(result, str)
39+
# Should return str(obj) fallback rather than raising ValueError
40+
41+
42+
def test_deeply_nested_structure_returns_str_fallback():
43+
"""json.dumps raises RecursionError on deeply nested structures."""
44+
obj = current = {}
45+
for _ in range(10000):
46+
current["child"] = {}
47+
current = current["child"]
48+
49+
result = _safe_json_serialize(obj)
50+
assert isinstance(result, str)
51+
# str(obj) itself can raise RecursionError, so expect the safe fallback
52+
assert "recursion" in result.lower() or result # non-empty string
53+
54+
55+
def test_normal_dict_serializes():
56+
"""Normal dicts should serialize as JSON."""
57+
result = _safe_json_serialize({"key": "value", "num": 42})
58+
assert '"key"' in result
59+
assert '"value"' in result
60+
61+
62+
def test_non_serializable_object_falls_back_to_str():
63+
"""Objects without a JSON representation should fall back to str()."""
64+
obj = object()
65+
result = _safe_json_serialize(obj)
66+
assert isinstance(result, str)
67+
assert "object" in result.lower()
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Copyright 2025 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Tests for _safe_json_serialize in telemetry/tracing.py.
16+
17+
Verifies that the function never raises exceptions, even for inputs that
18+
cause json.dumps to raise ValueError (circular references) or
19+
RecursionError (deeply nested structures).
20+
21+
Fixes https://github.com/google/adk-python/issues/5411
22+
"""
23+
24+
import pytest
25+
26+
from google.adk.telemetry.tracing import _safe_json_serialize
27+
28+
29+
def test_circular_reference_returns_fallback():
30+
"""json.dumps raises ValueError on circular references; should not propagate."""
31+
32+
class Node:
33+
def __init__(self):
34+
self.ref = self
35+
36+
obj = Node()
37+
result = _safe_json_serialize(obj)
38+
assert isinstance(result, str)
39+
# Should return the fallback rather than raising
40+
assert "not serializable" in result.lower() or result # non-empty string
41+
42+
43+
def test_deeply_nested_structure_returns_fallback():
44+
"""json.dumps raises RecursionError on deeply nested structures."""
45+
obj = current = {}
46+
for _ in range(10000):
47+
current["child"] = {}
48+
current = current["child"]
49+
50+
result = _safe_json_serialize(obj)
51+
assert isinstance(result, str)
52+
53+
54+
def test_normal_dict_serializes():
55+
"""Normal dicts should serialize without issue."""
56+
result = _safe_json_serialize({"key": "value", "num": 42})
57+
assert '"key"' in result
58+
assert '"value"' in result
59+
60+
61+
def test_non_serializable_object_uses_default():
62+
"""Objects without a JSON representation use the default callback."""
63+
result = _safe_json_serialize(object())
64+
assert isinstance(result, str)
65+
assert "not serializable" in result.lower()

0 commit comments

Comments
 (0)