capture errors correctly

Author: naji247

src/mcpcat/modules/compatibility.py

@@ -2,6 +2,8 @@
 
 from typing import Any, Protocol, runtime_checkable
 
+from mcp import ServerResult
+
 
 @runtime_checkable
 class MCPServerProtocol(Protocol):
@@ -68,3 +70,36 @@ def get_mcp_compatible_error_message(error: Any) -> str:
     if isinstance(error, Exception):
         return str(error)
     return str(error)
+
+def is_mcp_error_response(response: ServerResult) -> tuple[bool, str]:
+    """Check if the response is an MCP error."""
+    try:
+        # ServerResult is a RootModel, so we need to access its root attribute
+        if hasattr(response, 'root'):
+            result = response.root
+            # Check if it's a CallToolResult with an error
+            if hasattr(result, 'isError') and result.isError:
+                # Extract error message from content
+                if hasattr(result, 'content') and result.content:
+                    # content is a list of TextContent/ImageContent/EmbeddedResource
+                    for content_item in result.content:
+                        # Check if it has a text attribute (TextContent)
+                        if hasattr(content_item, 'text'):
+                            return True, str(content_item.text)
+                        # Check if it has type and content attributes
+                        elif hasattr(content_item, 'type') and hasattr(content_item, 'content'):
+                            if content_item.type == 'text':
+                                return True, str(content_item.content)
+                    
+                    # If no text content found, stringify the first item
+                    if result.content and len(result.content) > 0:
+                        return True, str(result.content[0])
+                    return True, "Unknown error"
+                return True, "Unknown error"
+        return False, ""
+    except (AttributeError, IndexError):
+        # Handle specific exceptions more precisely
+        return False, ""
+    except Exception as e:
+        # Log unexpected errors but still return a valid response
+        return False, f"Error checking response: {str(e)}"

src/mcpcat/modules/overrides/mcp_server.py

@@ -7,6 +7,7 @@
 from mcp.shared.context import RequestContext
 
 from mcpcat.modules import event_queue
+from mcpcat.modules.compatibility import is_mcp_error_response
 from mcpcat.modules.identify import identify_session
 from mcpcat.modules.logging import write_to_log
 from mcpcat.modules.tools import handle_report_missing
@@ -68,7 +69,7 @@ async def wrapped_list_tools_handler(request: ListToolsRequest) -> ServerResult:
             parameters=request.params.model_dump() if request.params else {},
             event_type=EventType.MCP_TOOLS_LIST.value,
         )
-        
+
         # Call the original handler to get the tools
         original_result = await original_list_tools_handler(request)
         if not original_result or not hasattr(original_result, 'root') or not hasattr(original_result.root, 'tools'):
@@ -168,7 +169,9 @@ async def wrapped_call_tool_handler(request: CallToolRequest) -> ServerResult:
             try:
                 # Call the original handler
                 result = await original_call_tool_handler(request)
-
+                is_error, error_message = is_mcp_error_response(result)
+                event.is_error = is_error
+                event.error = {"message": error_message} if is_error else None
                 # Record the trace using existing infrastructure
                 event.response = result.model_dump() if result else None
                 event_queue.publish_event(server, event)
@@ -187,4 +190,3 @@ async def wrapped_call_tool_handler(request: CallToolRequest) -> ServerResult:
     server.request_handlers[CallToolRequest] = wrapped_call_tool_handler
     server.request_handlers[ListToolsRequest] = wrapped_list_tools_handler
     server.request_handlers[InitializeRequest] = wrapped_initialize_handler
-

tests/test_event_capture_completeness.py

@@ -476,4 +476,45 @@ def custom_identify(request, server):
         # Should have actor info from custom identify
         assert event.identify_actor_given_id == "custom123"
         assert event.identify_actor_name == "Custom User"
-        assert event.identify_data == {"source": "custom_identify"}
+        assert event.identify_data == {"source": "custom_identify"}
+
+    @pytest.mark.asyncio
+    async def test_server_error_capture_in_event(self):
+        """Test that ServerResult errors are captured in the event's is_error and error fields."""
+        mock_api_client = MagicMock()
+        captured_events = []
+        
+        def capture_event(publish_event_request):
+            captured_events.append(publish_event_request)
+            
+        mock_api_client.publish_event = MagicMock(side_effect=capture_event)
+        
+        test_queue = EventQueue(api_client=mock_api_client)
+        set_event_queue(test_queue)
+        
+        server = create_todo_server()
+        options = MCPCatOptions(enable_tracing=True)
+        track(server, "test_project", options)
+        
+        async with create_test_client(server) as client:
+            # Try to complete a non-existent todo to trigger an error
+            try:
+                await client.call_tool("complete_todo", {"id": 999})
+            except Exception:
+                # The client might raise an exception, but we're interested in the event
+                pass
+            
+            time.sleep(1.0)
+            
+        # Find the tool call event for complete_todo
+        tool_events = [e for e in captured_events if e.event_type == "mcp:tools/call" and e.resource_name == "complete_todo"]
+        assert len(tool_events) > 0, "No complete_todo tool call event captured"
+        
+        event = tool_events[0]
+        
+        # Verify error fields are populated
+        assert event.is_error is True, "Event should be marked as error"
+        assert event.error is not None, "Event should have error details"
+        assert isinstance(event.error, dict), "Error should be a dictionary"
+        assert "message" in event.error, "Error should have a message"
+        assert "Todo with ID 999 not found" in event.error["message"], "Error message should contain the ValueError message"