Skip to content

Commit f665331

Browse files
ericapisaniclaude
andauthored
fix(utils): Avoid double serialization of strings in safe_serialize (#5587)
When `serialize_item()` inside `safe_serialize()` already returns a string (for plain strings, callables, or objects with `__dict__`), the subsequent `json.dumps()` call wraps it in extra quotes with escaped characters. For example, a JSON string `'{"param": "value"}'` becomes `'"{\\"param\\": \\"value\\"}"'`. This was causing double-serialized tool arguments in pydantic-ai integrations. The fix skips `json.dumps` when `serialize_item` has already produced a string, and adds tests covering plain strings, JSON strings, dicts, callables, and objects. Part of PY-2114 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 5db5874 commit f665331

File tree

4 files changed

+46
-13
lines changed

4 files changed

+46
-13
lines changed

sentry_sdk/utils.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2005,7 +2005,11 @@ def serialize_item(
20052005

20062006
try:
20072007
serialized = serialize_item(data)
2008-
return json.dumps(serialized, default=str)
2008+
return (
2009+
json.dumps(serialized, default=str)
2010+
if not isinstance(serialized, str)
2011+
else serialized
2012+
)
20092013
except Exception:
20102014
return str(data)
20112015

tests/integrations/google_genai/test_google_genai.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,7 +1625,7 @@ def test_generate_content_with_function_response(
16251625
assert messages[0]["role"] == "tool"
16261626
assert messages[0]["content"]["toolCallId"] == "call_123"
16271627
assert messages[0]["content"]["toolName"] == "get_weather"
1628-
assert messages[0]["content"]["output"] == '"Sunny, 72F"'
1628+
assert messages[0]["content"]["output"] == "Sunny, 72F"
16291629

16301630

16311631
def test_generate_content_with_mixed_string_and_content(
@@ -1891,7 +1891,7 @@ def test_extract_contents_messages_function_response():
18911891
assert result[0]["role"] == "tool"
18921892
assert result[0]["content"]["toolCallId"] == "call_123"
18931893
assert result[0]["content"]["toolName"] == "get_weather"
1894-
assert result[0]["content"]["output"] == '"sunny"'
1894+
assert result[0]["content"]["output"] == "sunny"
18951895

18961896

18971897
def test_extract_contents_messages_function_response_with_output_key():
@@ -1908,7 +1908,7 @@ def test_extract_contents_messages_function_response_with_output_key():
19081908
assert result[0]["content"]["toolCallId"] == "call_456"
19091909
assert result[0]["content"]["toolName"] == "get_time"
19101910
# Should prefer "output" key
1911-
assert result[0]["content"]["output"] == '"3:00 PM"'
1911+
assert result[0]["content"]["output"] == "3:00 PM"
19121912

19131913

19141914
def test_extract_contents_messages_mixed_parts():

tests/integrations/mcp/test_mcp.py

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -241,15 +241,12 @@ async def test_tool_async(tool_name, arguments):
241241
assert span["data"][SPANDATA.MCP_TRANSPORT] == "http"
242242
assert span["data"][SPANDATA.MCP_REQUEST_ID] == "req-456"
243243
assert span["data"][SPANDATA.MCP_SESSION_ID] == session_id
244-
assert span["data"]["mcp.request.argument.data"] == '"test"'
244+
assert span["data"]["mcp.request.argument.data"] == "test"
245245

246246
# Check PII-sensitive data
247247
if send_default_pii and include_prompts:
248-
# TODO: Investigate why tool result is double-serialized.
249248
assert span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == json.dumps(
250-
json.dumps(
251-
{"status": "completed"},
252-
)
249+
{"status": "completed"}
253250
)
254251
else:
255252
assert SPANDATA.MCP_TOOL_RESULT_CONTENT not in span["data"]
@@ -366,8 +363,8 @@ async def test_prompt(name, arguments):
366363
assert span["data"][SPANDATA.MCP_METHOD_NAME] == "prompts/get"
367364
assert span["data"][SPANDATA.MCP_TRANSPORT] == "stdio"
368365
assert span["data"][SPANDATA.MCP_REQUEST_ID] == "req-prompt"
369-
assert span["data"]["mcp.request.argument.name"] == '"code_help"'
370-
assert span["data"]["mcp.request.argument.language"] == '"python"'
366+
assert span["data"]["mcp.request.argument.name"] == "code_help"
367+
assert span["data"]["mcp.request.argument.language"] == "python"
371368

372369
# Message count is always captured
373370
assert span["data"][SPANDATA.MCP_PROMPT_RESULT_MESSAGE_COUNT] == 1
@@ -752,7 +749,7 @@ def test_tool_unstructured(tool_name, arguments):
752749
# Should extract and join text from content blocks only with PII
753750
if send_default_pii and include_prompts:
754751
assert (
755-
span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == '"First part Second part"'
752+
span["data"][SPANDATA.MCP_TOOL_RESULT_CONTENT] == "First part Second part"
756753
)
757754
else:
758755
assert SPANDATA.MCP_TOOL_RESULT_CONTENT not in span["data"]
@@ -959,7 +956,7 @@ def test_tool_complex(tool_name, arguments):
959956
assert span["data"]["mcp.request.argument.nested"] == json.dumps(
960957
{"key": "value", "list": [1, 2, 3]}
961958
)
962-
assert span["data"]["mcp.request.argument.string"] == '"test"'
959+
assert span["data"]["mcp.request.argument.string"] == "test"
963960
assert span["data"]["mcp.request.argument.number"] == "42"
964961

965962

tests/test_utils.py

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
exc_info_from_error,
3636
get_lines_from_file,
3737
package_version,
38+
safe_serialize,
3839
)
3940

4041

@@ -1062,5 +1063,36 @@ def fake_getlines(filename):
10621063
assert result == expected_result
10631064

10641065

1066+
def test_safe_serialize_plain_string():
1067+
assert safe_serialize("already a string") == "already a string"
1068+
1069+
1070+
def test_safe_serialize_json_string():
1071+
assert safe_serialize('{"key": "value"}') == '{"key": "value"}'
1072+
1073+
1074+
def test_safe_serialize_dict():
1075+
assert safe_serialize({"key": "value"}) == '{"key": "value"}'
1076+
1077+
1078+
def test_safe_serialize_callable():
1079+
def my_func():
1080+
pass
1081+
1082+
result = safe_serialize(my_func)
1083+
assert result.startswith("<function")
1084+
assert '"' not in result[:1] # no wrapping quotes from json.dumps
1085+
1086+
1087+
def test_safe_serialize_object():
1088+
class MyClass:
1089+
def __init__(self):
1090+
self.x = 1
1091+
1092+
result = safe_serialize(MyClass())
1093+
assert result.startswith("<MyClass")
1094+
assert '"' not in result[:1] # no wrapping quotes from json.dumps
1095+
1096+
10651097
def test_package_version_is_none():
10661098
assert package_version("non_existent_package") is None

0 commit comments

Comments
 (0)