Skip to content

Commit 806391b

Browse files
committed
fix: do not synthesize exception for cancelled tools
1 parent d27b8ff commit 806391b

5 files changed

Lines changed: 55 additions & 25 deletions

File tree

src/strands/telemetry/tracer.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ def _end_span(
212212
status_description = error_message or str(error) or type(error).__name__
213213
span.set_status(StatusCode.ERROR, status_description)
214214
span.record_exception(error)
215+
elif error_message:
216+
span.set_status(StatusCode.ERROR, error_message)
215217
else:
216218
span.set_status(StatusCode.OK)
217219
except Exception as e:
@@ -460,15 +462,13 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error:
460462
error: Optional exception if the tool call failed.
461463
"""
462464
attributes: dict[str, AttributeValue] = {}
465+
status: str | None = None
466+
content: list[Any] = []
467+
463468
if tool_result is not None:
464469
status = tool_result.get("status")
465-
status_str = str(status) if status is not None else ""
466-
467-
attributes.update(
468-
{
469-
"gen_ai.tool.status": status_str,
470-
}
471-
)
470+
content = tool_result.get("content", [])
471+
attributes["gen_ai.tool.status"] = str(status) if status is not None else ""
472472

473473
if self.use_latest_genai_conventions:
474474
self._add_event(
@@ -483,7 +483,7 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error:
483483
{
484484
"type": "tool_call_response",
485485
"id": tool_result.get("toolUseId", ""),
486-
"response": tool_result.get("content"),
486+
"response": content,
487487
}
488488
],
489489
}
@@ -497,12 +497,16 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error:
497497
span,
498498
"gen_ai.choice",
499499
event_attributes={
500-
"message": serialize(tool_result.get("content")),
500+
"message": serialize(content),
501501
"id": tool_result.get("toolUseId", ""),
502502
},
503503
)
504504

505-
self._end_span(span, attributes, error)
505+
if error is None and status == "error":
506+
error_message = next((b["text"] for b in content if "text" in b), "tool returned error status")
507+
self._end_span(span, attributes, error_message=error_message)
508+
else:
509+
self._end_span(span, attributes, error)
506510

507511
def start_event_loop_cycle_span(
508512
self,
@@ -527,9 +531,7 @@ def start_event_loop_cycle_span(
527531
event_loop_cycle_id = str(invocation_state.get("event_loop_cycle_id"))
528532
parent_span = parent_span if parent_span else invocation_state.get("event_loop_parent_span")
529533

530-
attributes: dict[str, AttributeValue] = self._get_common_attributes(
531-
operation_name="execute_event_loop_cycle"
532-
)
534+
attributes: dict[str, AttributeValue] = self._get_common_attributes(operation_name="execute_event_loop_cycle")
533535
attributes["event_loop.cycle_id"] = event_loop_cycle_id
534536

535537
if custom_trace_attributes:

src/strands/tools/executors/_executor.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -176,10 +176,9 @@ async def _stream(
176176
tool_use,
177177
invocation_state,
178178
cancel_result,
179-
exception=Exception(cancel_message),
180179
cancel_message=cancel_message,
181180
)
182-
yield ToolResultEvent(after_event.result, exception=after_event.exception)
181+
yield ToolResultEvent(after_event.result)
183182
tool_results.append(after_event.result)
184183
return
185184

tests/strands/telemetry/test_tracer.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -722,6 +722,19 @@ def test_end_tool_call_span_with_error(mock_span):
722722
mock_span.end.assert_called_once()
723723

724724

725+
def test_end_tool_call_span_error_result_no_exception(mock_span):
726+
"""Test that an error result without an exception still sets StatusCode.ERROR."""
727+
tracer = Tracer()
728+
tool_result = {"status": "error", "content": [{"text": "tool cancelled by user"}]}
729+
730+
tracer.end_tool_call_span(mock_span, tool_result)
731+
732+
mock_span.set_attributes.assert_called_once_with({"gen_ai.tool.status": "error"})
733+
mock_span.set_status.assert_called_once_with(StatusCode.ERROR, "tool cancelled by user")
734+
mock_span.record_exception.assert_not_called()
735+
mock_span.end.assert_called_once()
736+
737+
725738
def test_start_event_loop_cycle_span(mock_tracer):
726739
"""Test starting an event loop cycle span."""
727740
with mock.patch("strands.telemetry.tracer.trace_api.get_tracer", return_value=mock_tracer):

tests/strands/tools/executors/test_executor.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -954,8 +954,8 @@ async def test_executor_stream_unknown_tool_has_exception(executor, agent, tool_
954954

955955

956956
@pytest.mark.asyncio
957-
async def test_executor_stream_cancel_has_exception(executor, agent, tool_results, invocation_state, alist):
958-
"""Test that _stream yields a ToolResultEvent with exception for cancelled tools."""
957+
async def test_executor_stream_cancel_no_exception(executor, agent, tool_results, invocation_state, alist):
958+
"""Test that _stream yields a ToolResultEvent with no exception for cancelled tools."""
959959

960960
def cancel_callback(event):
961961
event.cancel_tool = True
@@ -969,5 +969,25 @@ def cancel_callback(event):
969969
result_event = events[-1]
970970
assert isinstance(result_event, ToolResultEvent)
971971
assert result_event.tool_result["status"] == "error"
972-
assert result_event.exception is not None
973-
assert "cancelled" in str(result_event.exception)
972+
assert result_event.exception is None
973+
974+
975+
@pytest.mark.asyncio
976+
async def test_executor_stream_cancel_after_hook_sees_no_exception(
977+
executor, agent, tool_results, invocation_state, hook_events, alist
978+
):
979+
"""Test that AfterToolCallEvent.exception is None when a tool is cancelled."""
980+
981+
def cancel_callback(event):
982+
event.cancel_tool = "user denied permission"
983+
return event
984+
985+
agent.hooks.add_callback(BeforeToolCallEvent, cancel_callback)
986+
tool_use: ToolUse = {"name": "weather_tool", "toolUseId": "1", "input": {}}
987+
stream = executor._stream(agent, tool_use, tool_results, invocation_state)
988+
await alist(stream)
989+
990+
after_event = hook_events[-1]
991+
assert isinstance(after_event, AfterToolCallEvent)
992+
assert after_event.exception is None
993+
assert after_event.cancel_message == "user denied permission"

tests/strands/tools/mcp/test_mcp_client_tasks.py

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,7 @@ def test_call_tool_sync_forwards_meta_to_task(self, mock_transport, mock_session
251251

252252
with MCPClient(mock_transport["transport_callable"], tasks_config=TasksConfig()) as client:
253253
client.list_tools_sync()
254-
client.call_tool_sync(
255-
tool_use_id="test-id", name="meta_tool", arguments={"param": "value"}, meta=meta
256-
)
254+
client.call_tool_sync(tool_use_id="test-id", name="meta_tool", arguments={"param": "value"}, meta=meta)
257255

258256
experimental.call_tool_as_task.assert_called_once()
259257
call_kwargs = experimental.call_tool_as_task.call_args
@@ -281,9 +279,7 @@ def test_call_tool_sync_forwards_none_meta_to_task(self, mock_transport, mock_se
281279

282280
with MCPClient(mock_transport["transport_callable"], tasks_config=TasksConfig()) as client:
283281
client.list_tools_sync()
284-
client.call_tool_sync(
285-
tool_use_id="test-id", name="no_meta_tool", arguments={"param": "value"}
286-
)
282+
client.call_tool_sync(tool_use_id="test-id", name="no_meta_tool", arguments={"param": "value"})
287283

288284
experimental.call_tool_as_task.assert_called_once()
289285
call_kwargs = experimental.call_tool_as_task.call_args

0 commit comments

Comments
 (0)