Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/strands/event_loop/streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,11 @@ def handle_content_block_stop(state: dict[str, Any]) -> dict[str, Any]:
try:
current_tool_use["input"] = json.loads(current_tool_use["input"])
except ValueError:
logger.warning(
"Failed to parse tool input JSON for '%s': %s",
current_tool_use.get("name", "unknown"),
current_tool_use["input"][:200] if isinstance(current_tool_use.get("input"), str) else "",
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The log message doesn't follow the project's structured logging format defined in AGENTS.md.

The format requires field=<value> pairs separated by commas, followed by a pipe | and a lowercase human-readable message. Compare with the existing warning on line 349 in this same file:

logger.warning(
    "original_stop_reason=<%s>, new_stop_reason=<%s> | "
    "overriding stop reason due to toolUse blocks in response",
    "end_turn",
    "tool_use",
)

Suggestion:

logger.warning(
    "tool_name=<%s>, raw_input=<%s> | failed to parse tool input json, defaulting to empty dict",
    current_tool_use.get("name", "unknown"),
    current_tool_use["input"][:200] if isinstance(current_tool_use.get("input"), str) else "",
)

)
current_tool_use["input"] = {}

tool_use_id = current_tool_use["toolUseId"]
Expand Down
19 changes: 19 additions & 0 deletions tests/strands/event_loop/test_streaming.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,25 @@ def test_handle_content_block_delta(event: ContentBlockDeltaEvent, event_type, s
"redactedContent": b"",
},
),
# Tool Use - Malformed input JSON
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue: The new parametrized test case verifies the state transformation (malformed JSON → {}) but doesn't assert that logger.warning was actually called. Since the purpose of this PR is to add the warning log, the test should verify it.

Suggestion: Add a dedicated test that uses unittest.mock.patch to assert the warning is emitted with the expected arguments:

@unittest.mock.patch("strands.event_loop.streaming.logger")
def test_handle_content_block_stop_logs_warning_on_malformed_json(mock_logger):
    state = {
        "content": [],
        "current_tool_use": {"toolUseId": "123", "name": "test_tool", "input": "{invalid json}"},
        "text": "",
        "reasoningText": "",
        "citationsContent": [],
        "redactedContent": b"",
    }

    strands.event_loop.streaming.handle_content_block_stop(state)

    mock_logger.warning.assert_called_once()
    call_args = mock_logger.warning.call_args
    assert "test_tool" in str(call_args)
    assert "{invalid json}" in str(call_args)

(
{
"content": [],
"current_tool_use": {"toolUseId": "123", "name": "test", "input": "{invalid json}"},
"text": "",
"reasoningText": "",
"citationsContent": [],
"redactedContent": b"",
},
{
"content": [{"toolUse": {"toolUseId": "123", "name": "test", "input": {}}}],
"current_tool_use": {},
"text": "",
"reasoningText": "",
"citationsContent": [],
"redactedContent": b"",
},
),
# Text
(
{
Expand Down
Loading