diff --git a/src/strands/telemetry/tracer.py b/src/strands/telemetry/tracer.py index d5d399f95..52fd80bb7 100644 --- a/src/strands/telemetry/tracer.py +++ b/src/strands/telemetry/tracer.py @@ -212,6 +212,8 @@ def _end_span( status_description = error_message or str(error) or type(error).__name__ span.set_status(StatusCode.ERROR, status_description) span.record_exception(error) + elif error_message: + span.set_status(StatusCode.ERROR, error_message) else: span.set_status(StatusCode.OK) except Exception as e: @@ -460,15 +462,13 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error: error: Optional exception if the tool call failed. """ attributes: dict[str, AttributeValue] = {} + status: str | None = None + content: list[Any] = [] + if tool_result is not None: status = tool_result.get("status") - status_str = str(status) if status is not None else "" - - attributes.update( - { - "gen_ai.tool.status": status_str, - } - ) + content = tool_result.get("content", []) + attributes["gen_ai.tool.status"] = str(status) if status is not None else "" if self.use_latest_genai_conventions: self._add_event( @@ -483,7 +483,7 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error: { "type": "tool_call_response", "id": tool_result.get("toolUseId", ""), - "response": tool_result.get("content"), + "response": content, } ], } @@ -497,12 +497,16 @@ def end_tool_call_span(self, span: Span, tool_result: ToolResult | None, error: span, "gen_ai.choice", event_attributes={ - "message": serialize(tool_result.get("content")), + "message": serialize(content), "id": tool_result.get("toolUseId", ""), }, ) - self._end_span(span, attributes, error) + if error is None and status == "error": + error_message = next((b["text"] for b in content if "text" in b), "tool returned error status") + self._end_span(span, attributes, error_message=error_message) + else: + self._end_span(span, attributes, error) def start_event_loop_cycle_span( self, @@ -527,9 +531,7 @@ def start_event_loop_cycle_span( event_loop_cycle_id = str(invocation_state.get("event_loop_cycle_id")) parent_span = parent_span if parent_span else invocation_state.get("event_loop_parent_span") - attributes: dict[str, AttributeValue] = self._get_common_attributes( - operation_name="execute_event_loop_cycle" - ) + attributes: dict[str, AttributeValue] = self._get_common_attributes(operation_name="execute_event_loop_cycle") attributes["event_loop.cycle_id"] = event_loop_cycle_id if custom_trace_attributes: diff --git a/src/strands/tools/executors/_executor.py b/src/strands/tools/executors/_executor.py index 2c602a560..3993f332b 100644 --- a/src/strands/tools/executors/_executor.py +++ b/src/strands/tools/executors/_executor.py @@ -176,10 +176,9 @@ async def _stream( tool_use, invocation_state, cancel_result, - exception=Exception(cancel_message), cancel_message=cancel_message, ) - yield ToolResultEvent(after_event.result, exception=after_event.exception) + yield ToolResultEvent(after_event.result) tool_results.append(after_event.result) return diff --git a/tests/strands/telemetry/test_tracer.py b/tests/strands/telemetry/test_tracer.py index 6b622bb3e..c5586f080 100644 --- a/tests/strands/telemetry/test_tracer.py +++ b/tests/strands/telemetry/test_tracer.py @@ -722,6 +722,19 @@ def test_end_tool_call_span_with_error(mock_span): mock_span.end.assert_called_once() +def test_end_tool_call_span_error_result_no_exception(mock_span): + """Test that an error result without an exception still sets StatusCode.ERROR.""" + tracer = Tracer() + tool_result = {"status": "error", "content": [{"text": "tool cancelled by user"}]} + + tracer.end_tool_call_span(mock_span, tool_result) + + mock_span.set_attributes.assert_called_once_with({"gen_ai.tool.status": "error"}) + mock_span.set_status.assert_called_once_with(StatusCode.ERROR, "tool cancelled by user") + mock_span.record_exception.assert_not_called() + mock_span.end.assert_called_once() + + def test_start_event_loop_cycle_span(mock_tracer): """Test starting an event loop cycle span.""" with mock.patch("strands.telemetry.tracer.trace_api.get_tracer", return_value=mock_tracer): diff --git a/tests/strands/tools/executors/test_executor.py b/tests/strands/tools/executors/test_executor.py index 34b37dab0..9c38340b9 100644 --- a/tests/strands/tools/executors/test_executor.py +++ b/tests/strands/tools/executors/test_executor.py @@ -954,8 +954,8 @@ async def test_executor_stream_unknown_tool_has_exception(executor, agent, tool_ @pytest.mark.asyncio -async def test_executor_stream_cancel_has_exception(executor, agent, tool_results, invocation_state, alist): - """Test that _stream yields a ToolResultEvent with exception for cancelled tools.""" +async def test_executor_stream_cancel_no_exception(executor, agent, tool_results, invocation_state, alist): + """Test that _stream yields a ToolResultEvent with no exception for cancelled tools.""" def cancel_callback(event): event.cancel_tool = True @@ -969,5 +969,25 @@ def cancel_callback(event): result_event = events[-1] assert isinstance(result_event, ToolResultEvent) assert result_event.tool_result["status"] == "error" - assert result_event.exception is not None - assert "cancelled" in str(result_event.exception) + assert result_event.exception is None + + +@pytest.mark.asyncio +async def test_executor_stream_cancel_after_hook_sees_no_exception( + executor, agent, tool_results, invocation_state, hook_events, alist +): + """Test that AfterToolCallEvent.exception is None when a tool is cancelled.""" + + def cancel_callback(event): + event.cancel_tool = "user denied permission" + return event + + agent.hooks.add_callback(BeforeToolCallEvent, cancel_callback) + tool_use: ToolUse = {"name": "weather_tool", "toolUseId": "1", "input": {}} + stream = executor._stream(agent, tool_use, tool_results, invocation_state) + await alist(stream) + + after_event = hook_events[-1] + assert isinstance(after_event, AfterToolCallEvent) + assert after_event.exception is None + assert after_event.cancel_message == "user denied permission" diff --git a/tests/strands/tools/mcp/test_mcp_client_tasks.py b/tests/strands/tools/mcp/test_mcp_client_tasks.py index c21db9e28..d566ac6f5 100644 --- a/tests/strands/tools/mcp/test_mcp_client_tasks.py +++ b/tests/strands/tools/mcp/test_mcp_client_tasks.py @@ -251,9 +251,7 @@ def test_call_tool_sync_forwards_meta_to_task(self, mock_transport, mock_session with MCPClient(mock_transport["transport_callable"], tasks_config=TasksConfig()) as client: client.list_tools_sync() - client.call_tool_sync( - tool_use_id="test-id", name="meta_tool", arguments={"param": "value"}, meta=meta - ) + client.call_tool_sync(tool_use_id="test-id", name="meta_tool", arguments={"param": "value"}, meta=meta) experimental.call_tool_as_task.assert_called_once() 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 with MCPClient(mock_transport["transport_callable"], tasks_config=TasksConfig()) as client: client.list_tools_sync() - client.call_tool_sync( - tool_use_id="test-id", name="no_meta_tool", arguments={"param": "value"} - ) + client.call_tool_sync(tool_use_id="test-id", name="no_meta_tool", arguments={"param": "value"}) experimental.call_tool_as_task.assert_called_once() call_kwargs = experimental.call_tool_as_task.call_args