From ef60bc36ff7658fed2151b5ff6156fdbf3ed397a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?d=20=F0=9F=94=B9?= Date: Mon, 20 Apr 2026 17:09:42 +0000 Subject: [PATCH 1/2] fix: handle ValueError and RecursionError in _safe_json_serialize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/google/adk/models/lite_llm.py | 7 +- src/google/adk/telemetry/tracing.py | 2 +- .../models/test_litellm_safe_serialize.py | 67 +++++++++++++++++++ .../telemetry/test_safe_json_serialize.py | 65 ++++++++++++++++++ 4 files changed, 138 insertions(+), 3 deletions(-) create mode 100644 tests/unittests/models/test_litellm_safe_serialize.py create mode 100644 tests/unittests/telemetry/test_safe_json_serialize.py diff --git a/src/google/adk/models/lite_llm.py b/src/google/adk/models/lite_llm.py index 3a6c36624d..32e3e8cf0c 100644 --- a/src/google/adk/models/lite_llm.py +++ b/src/google/adk/models/lite_llm.py @@ -556,8 +556,11 @@ def _safe_json_serialize(obj) -> str: try: # Try direct JSON serialization first return json.dumps(obj, ensure_ascii=False) - except (TypeError, ValueError, OverflowError): - return str(obj) + except (TypeError, OverflowError, ValueError, RecursionError): + try: + return str(obj) + except RecursionError: + return '' def _part_has_payload(part: types.Part) -> bool: diff --git a/src/google/adk/telemetry/tracing.py b/src/google/adk/telemetry/tracing.py index 84a061a48b..91ff93ad80 100644 --- a/src/google/adk/telemetry/tracing.py +++ b/src/google/adk/telemetry/tracing.py @@ -128,7 +128,7 @@ def _safe_json_serialize(obj) -> str: return json.dumps( obj, ensure_ascii=False, default=lambda o: '' ) - except (TypeError, ValueError, OverflowError): + except (TypeError, OverflowError, ValueError, RecursionError): return '' diff --git a/tests/unittests/models/test_litellm_safe_serialize.py b/tests/unittests/models/test_litellm_safe_serialize.py new file mode 100644 index 0000000000..10fd0dd9a7 --- /dev/null +++ b/tests/unittests/models/test_litellm_safe_serialize.py @@ -0,0 +1,67 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for _safe_json_serialize in models/lite_llm.py. + +Verifies that the function never raises exceptions, even for inputs that +cause json.dumps to raise ValueError (circular references) or +RecursionError (deeply nested structures). + +Fixes https://github.com/google/adk-python/issues/5412 +""" + +import pytest + +from google.adk.models.lite_llm import _safe_json_serialize + + +def test_circular_reference_returns_str_fallback(): + """json.dumps raises ValueError on circular references; should fall back to str().""" + + class Node: + def __init__(self): + self.ref = self + + obj = Node() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # Should return str(obj) fallback rather than raising ValueError + + +def test_deeply_nested_structure_returns_str_fallback(): + """json.dumps raises RecursionError on deeply nested structures.""" + obj = current = {} + for _ in range(10000): + current["child"] = {} + current = current["child"] + + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # str(obj) itself can raise RecursionError, so expect the safe fallback + assert "recursion" in result.lower() or result # non-empty string + + +def test_normal_dict_serializes(): + """Normal dicts should serialize as JSON.""" + result = _safe_json_serialize({"key": "value", "num": 42}) + assert '"key"' in result + assert '"value"' in result + + +def test_non_serializable_object_falls_back_to_str(): + """Objects without a JSON representation should fall back to str().""" + obj = object() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + assert "object" in result.lower() diff --git a/tests/unittests/telemetry/test_safe_json_serialize.py b/tests/unittests/telemetry/test_safe_json_serialize.py new file mode 100644 index 0000000000..f07e4535c7 --- /dev/null +++ b/tests/unittests/telemetry/test_safe_json_serialize.py @@ -0,0 +1,65 @@ +# Copyright 2025 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for _safe_json_serialize in telemetry/tracing.py. + +Verifies that the function never raises exceptions, even for inputs that +cause json.dumps to raise ValueError (circular references) or +RecursionError (deeply nested structures). + +Fixes https://github.com/google/adk-python/issues/5411 +""" + +import pytest + +from google.adk.telemetry.tracing import _safe_json_serialize + + +def test_circular_reference_returns_fallback(): + """json.dumps raises ValueError on circular references; should not propagate.""" + + class Node: + def __init__(self): + self.ref = self + + obj = Node() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # Should return the fallback rather than raising + assert "not serializable" in result.lower() or result # non-empty string + + +def test_deeply_nested_structure_returns_fallback(): + """json.dumps raises RecursionError on deeply nested structures.""" + obj = current = {} + for _ in range(10000): + current["child"] = {} + current = current["child"] + + result = _safe_json_serialize(obj) + assert isinstance(result, str) + + +def test_normal_dict_serializes(): + """Normal dicts should serialize without issue.""" + result = _safe_json_serialize({"key": "value", "num": 42}) + assert '"key"' in result + assert '"value"' in result + + +def test_non_serializable_object_uses_default(): + """Objects without a JSON representation use the default callback.""" + result = _safe_json_serialize(object()) + assert isinstance(result, str) + assert "not serializable" in result.lower() From c231cf82b03cfdd5370eac846ddacbfb94bfd9d9 Mon Sep 17 00:00:00 2001 From: voidborne-d Date: Tue, 21 Apr 2026 03:08:18 +0000 Subject: [PATCH 2/2] style: run autoformat.sh --- src/google/adk/models/lite_llm.py | 2 +- .../models/test_litellm_safe_serialize.py | 56 +++++++++---------- .../telemetry/test_safe_json_serialize.py | 52 ++++++++--------- 3 files changed, 55 insertions(+), 55 deletions(-) diff --git a/src/google/adk/models/lite_llm.py b/src/google/adk/models/lite_llm.py index 32e3e8cf0c..8747210db1 100644 --- a/src/google/adk/models/lite_llm.py +++ b/src/google/adk/models/lite_llm.py @@ -560,7 +560,7 @@ def _safe_json_serialize(obj) -> str: try: return str(obj) except RecursionError: - return '' + return "" def _part_has_payload(part: types.Part) -> bool: diff --git a/tests/unittests/models/test_litellm_safe_serialize.py b/tests/unittests/models/test_litellm_safe_serialize.py index 10fd0dd9a7..5bf2af9f82 100644 --- a/tests/unittests/models/test_litellm_safe_serialize.py +++ b/tests/unittests/models/test_litellm_safe_serialize.py @@ -21,47 +21,47 @@ Fixes https://github.com/google/adk-python/issues/5412 """ -import pytest - from google.adk.models.lite_llm import _safe_json_serialize +import pytest def test_circular_reference_returns_str_fallback(): - """json.dumps raises ValueError on circular references; should fall back to str().""" + """json.dumps raises ValueError on circular references; should fall back to str().""" + + class Node: - class Node: - def __init__(self): - self.ref = self + def __init__(self): + self.ref = self - obj = Node() - result = _safe_json_serialize(obj) - assert isinstance(result, str) - # Should return str(obj) fallback rather than raising ValueError + obj = Node() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # Should return str(obj) fallback rather than raising ValueError def test_deeply_nested_structure_returns_str_fallback(): - """json.dumps raises RecursionError on deeply nested structures.""" - obj = current = {} - for _ in range(10000): - current["child"] = {} - current = current["child"] + """json.dumps raises RecursionError on deeply nested structures.""" + obj = current = {} + for _ in range(10000): + current["child"] = {} + current = current["child"] - result = _safe_json_serialize(obj) - assert isinstance(result, str) - # str(obj) itself can raise RecursionError, so expect the safe fallback - assert "recursion" in result.lower() or result # non-empty string + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # str(obj) itself can raise RecursionError, so expect the safe fallback + assert "recursion" in result.lower() or result # non-empty string def test_normal_dict_serializes(): - """Normal dicts should serialize as JSON.""" - result = _safe_json_serialize({"key": "value", "num": 42}) - assert '"key"' in result - assert '"value"' in result + """Normal dicts should serialize as JSON.""" + result = _safe_json_serialize({"key": "value", "num": 42}) + assert '"key"' in result + assert '"value"' in result def test_non_serializable_object_falls_back_to_str(): - """Objects without a JSON representation should fall back to str().""" - obj = object() - result = _safe_json_serialize(obj) - assert isinstance(result, str) - assert "object" in result.lower() + """Objects without a JSON representation should fall back to str().""" + obj = object() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + assert "object" in result.lower() diff --git a/tests/unittests/telemetry/test_safe_json_serialize.py b/tests/unittests/telemetry/test_safe_json_serialize.py index f07e4535c7..04f42bc2c1 100644 --- a/tests/unittests/telemetry/test_safe_json_serialize.py +++ b/tests/unittests/telemetry/test_safe_json_serialize.py @@ -21,45 +21,45 @@ Fixes https://github.com/google/adk-python/issues/5411 """ -import pytest - from google.adk.telemetry.tracing import _safe_json_serialize +import pytest def test_circular_reference_returns_fallback(): - """json.dumps raises ValueError on circular references; should not propagate.""" + """json.dumps raises ValueError on circular references; should not propagate.""" + + class Node: - class Node: - def __init__(self): - self.ref = self + def __init__(self): + self.ref = self - obj = Node() - result = _safe_json_serialize(obj) - assert isinstance(result, str) - # Should return the fallback rather than raising - assert "not serializable" in result.lower() or result # non-empty string + obj = Node() + result = _safe_json_serialize(obj) + assert isinstance(result, str) + # Should return the fallback rather than raising + assert "not serializable" in result.lower() or result # non-empty string def test_deeply_nested_structure_returns_fallback(): - """json.dumps raises RecursionError on deeply nested structures.""" - obj = current = {} - for _ in range(10000): - current["child"] = {} - current = current["child"] + """json.dumps raises RecursionError on deeply nested structures.""" + obj = current = {} + for _ in range(10000): + current["child"] = {} + current = current["child"] - result = _safe_json_serialize(obj) - assert isinstance(result, str) + result = _safe_json_serialize(obj) + assert isinstance(result, str) def test_normal_dict_serializes(): - """Normal dicts should serialize without issue.""" - result = _safe_json_serialize({"key": "value", "num": 42}) - assert '"key"' in result - assert '"value"' in result + """Normal dicts should serialize without issue.""" + result = _safe_json_serialize({"key": "value", "num": 42}) + assert '"key"' in result + assert '"value"' in result def test_non_serializable_object_uses_default(): - """Objects without a JSON representation use the default callback.""" - result = _safe_json_serialize(object()) - assert isinstance(result, str) - assert "not serializable" in result.lower() + """Objects without a JSON representation use the default callback.""" + result = _safe_json_serialize(object()) + assert isinstance(result, str) + assert "not serializable" in result.lower()