Merge pull request #1582 from oraios/raise-mcp-errors

Make tool call errors surface explicitly as errors at the MCP level

CHANGELOG.md

@@ -19,6 +19,7 @@ Status of the `main` branch. Changes prior to the next official version change w
     for additional prompts that explain tool name differences)
   - Improve quoting/escaping of arguments in shell executions on Windows (via `oslex` dependency)
   - Add tool parameter alias support, adding `name_path` as an alias for `name_path_pattern` in `find_symbol` tools
+  - Make tool call errors surface explicitly as errors at the MCP protocol level
 
 * Language Servers:
   - `typescript_vts`: Add `initialization_options` setting in `ls_specific_settings.typescript_vts`.

src/serena/mcp.py

@@ -11,6 +11,7 @@ from typing import Any, Literal, cast
 
 import docstring_parser
 from mcp.server.fastmcp import server
+from mcp.server.fastmcp.exceptions import ToolError
 from mcp.server.fastmcp.server import Context, FastMCP, Settings
 from mcp.server.fastmcp.tools.base import Tool as FastMCPTool
 from mcp.server.session import ServerSessionT
@@ -26,7 +27,7 @@ from serena.agent import (
 from serena.config.context_mode import SerenaAgentContext
 from serena.config.serena_config import LanguageBackend, ModeSelectionDefinition
 from serena.constants import DEFAULT_CONTEXT, SERENA_LOG_FORMAT
-from serena.tools import Tool
+from serena.tools import Tool, ToolCallError
 from serena.util.exception import show_fatal_exception_safe
 from serena.util.logging import MemoryLogHandler
 
@@ -94,7 +95,10 @@ class SerenaFastMCPTool(FastMCPTool):
                 properties["description"] = param_desc[0].upper() + param_desc[1:]
 
         def execute_fn(**kwargs) -> str:  # type: ignore
-            return tool.apply_ex(log_call=True, catch_exceptions=True, **kwargs)
+            try:
+                return tool.apply_ex(log_call=True, catch_exceptions=False, **kwargs)
+            except ToolCallError as e:
+                raise ToolError(e.get_error_message()) from e
 
         # Generate human-readable title from snake_case tool name
         tool_title = " ".join(word.capitalize() for word in func_name.split("_"))

src/serena/memories/memory_manager.py

@@ -191,7 +191,7 @@ class MemoryManager:
         self._check_not_ignored(name)
         memory_file_path = self.get_memory_file_path(name)
         if not memory_file_path.exists():
-            return f"Memory file {name} not found, consider creating it with the `write_memory` tool if you need it."
+            raise FileNotFoundError(f"Memory named '{name}' not found")
         with open(memory_file_path, encoding=self._encoding) as f:
             return f.read()
 

src/serena/tools/tools_base.py

@@ -126,6 +126,19 @@ class ApplyMethodProtocol(Protocol):
         pass
 
 
+class ToolCallError(Exception):
+    """
+    Represents an error raised during a tool call execution
+    """
+
+    def __init__(self, error_message: str):
+        super().__init__(error_message)
+        self._error_message = error_message
+
+    def get_error_message(self) -> str:
+        return self._error_message
+
+
 class Tool(Component):
     # NOTE: each tool should implement the apply method, which is then used in
     # the central method of the Tool class `apply_ex`.
@@ -316,8 +329,14 @@ class Tool(Component):
 
     def apply_ex(self, log_call: bool = True, catch_exceptions: bool = True, mcp_ctx: Context | None = None, **kwargs) -> str:  # type: ignore
         """
-        Applies the tool with logging and exception handling, using the given keyword arguments
+        Applies the tool with logging and exception handling, using the given keyword arguments.
+        This method either returns a string result or raises a ToolCallError in case of an error during tool application
+        (but if `catch_exception is enabled, it will return the error message as a string instead of raising the exception).
+
+        :param log_call: whether to log the tool call and its result
+        :param catch_exceptions: whether to catch exceptions and return their messages as strings, instead of raising a ToolCallError
         """
+        # obtain session ID and client info
         session_id = "global"
         if mcp_ctx is not None:
             try:
@@ -337,18 +356,18 @@ class Tool(Component):
 
             try:
                 if not self.is_active():
-                    return f"Error: Tool '{self.get_name_from_cls()}' is not active. Active tools: {self.agent.get_active_tool_names()}"
-            except Exception as e:
-                return f"RuntimeError while checking if tool {self.get_name_from_cls()} is active: {e}"
+                    raise ToolCallError(
+                        f"Tool '{self.get_name_from_cls()}' is not active. Active tools: {self.agent.get_active_tool_names()}"
+                    )
+
+                if log_call:
+                    self._log_tool_application(inspect.currentframe(), session_id)
 
-            if log_call:
-                self._log_tool_application(inspect.currentframe(), session_id)
-            try:
                 # check whether the tool requires an active project and language server
                 if not isinstance(self, ToolMarkerDoesNotRequireActiveProject):
                     if self.agent.get_active_project() is None:
-                        return (
-                            "Error: No active project. Ask the user to provide the project path or to select a project from this list of known projects: "
+                        raise ToolCallError(
+                            "No active project. Ask the user to provide the project path or to select a project from this list of known projects: "
                             + f"{self.agent.serena_config.project_names}"
                         )
 
@@ -380,12 +399,12 @@ class Tool(Component):
                 # record tool usage
                 self.agent.record_tool_usage(apply_kwargs, result, self)
 
+            except ToolCallError:
+                raise
             except Exception as e:
-                if not catch_exceptions:
-                    raise
-                msg = f"Error executing tool: {e.__class__.__name__} - {e}"
-                log.error(f"Error executing tool: {e}", exc_info=e)
-                result = msg
+                msg = f"{e.__class__.__name__}: {e}"
+                log.error(msg, exc_info=e)
+                raise ToolCallError(msg)
 
             if log_call:
                 log.info(f"Result: {result}")
@@ -400,13 +419,20 @@ class Tool(Component):
             return result
 
         # execute the tool in the agent's task executor, with timeout
+        tool_call_error: ToolCallError
         try:
             task_exec = self.agent.issue_task(task, name=self.__class__.__name__)
             return task_exec.result(timeout=self.agent.serena_config.tool_timeout)
-        except Exception as e:  # typically TimeoutError (other exceptions caught in task)
-            msg = f"Error: {e.__class__.__name__} - {e}"
+        except ToolCallError as e:
+            tool_call_error = e
+        except Exception as e:  # typically TimeoutError (other exceptions caught and forwarded as ToolCallError in the task)
+            msg = f"{e.__class__.__name__}: {e}"
             log.error(msg)
-            return msg
+            tool_call_error = ToolCallError(msg)
+        if catch_exceptions:
+            return tool_call_error.get_error_message()
+        else:
+            raise tool_call_error
 
     @staticmethod
     def _to_json(x: Any) -> str: