commit 55c0d20318ea92e190a47425ac55d4ccce12dcb4 equwal <13551856+equwal@users.noreply.github.com> 2026-10-04 04:32:34 -0700 Send tool error messages to the client mcp 2.x shows the client only "Error executing tool <name>" for every exception except ToolError. The agent could not read "no DSL named x" or a bad name, a bad ext, or a missing interpreter. The server now raises ToolError for each of these, and for a run that times out. Tests: the stdio test asks for a missing DSL and reads the message; a new test covers the timeout.
server.py | 33 +++++++++++++++++++++------------ test_server.py | 14 +++++++++++++- 2 files changed, 34 insertions(+), 13 deletions(-)
diff --git a/server.py b/server.py index 6645b5c..595f29c 100644 --- a/server.py +++ b/server.py @@ -18,6 +18,10 @@ from pathlib import Path from mcp.server.mcpserver import MCPServer +# The client sees the message of a ToolError. For any other exception it sees +# only "Error executing tool <name>". +from mcp.server.mcpserver.exceptions import ToolError + mcp = MCPServer("dsl") NAME = re.compile(r"[a-z0-9][a-z0-9_-]{0,63}") @@ -52,14 +56,14 @@ def table() -> dict[str, list[str]]: def folder(name: str) -> Path: # The name becomes a path segment, so reject anything that can leave home(). if not NAME.fullmatch(name): - raise ValueError(f"name must match {NAME.pattern}") + raise ToolError(f"name must match {NAME.pattern}") return home() / name def impl(name: str) -> Path: found = sorted(folder(name).glob("impl.*")) if not found: - raise ValueError(f"no DSL named {name}") + raise ToolError(f"no DSL named {name}") return found[0] @@ -95,7 +99,7 @@ def put(name: str, grammar: str, ext: str, source: str) -> str: with a message on stderr for a bad program. """ if ext not in table(): - raise ValueError(f"ext must be one of {sorted(table())}") + raise ToolError(f"ext must be one of {sorted(table())}") d = folder(name) d.mkdir(parents=True, exist_ok=True) new = d / f"impl.{ext}" @@ -115,19 +119,24 @@ def run(name: str, program: str, timeout: float = 10) -> dict[str, str | int]: Return stdout, stderr and the exit code. """ f = impl(name) - argv = table()[f.suffix[1:]] + argv = table().get(f.suffix[1:]) + if argv is None: + raise ToolError(f"no runtime for {f.name}") exe = shutil.which(argv[0]) if exe is None: - raise ValueError(f"{argv[0]} is not on PATH") + raise ToolError(f"{argv[0]} is not on PATH") # ponytail: captures all output in memory, then cuts it to LIMIT. # Read through a capped pipe if a DSL can flood stdout within the timeout. - done = subprocess.run( - [exe, *argv[1:], str(f)], - input=program.encode(), - capture_output=True, - timeout=timeout, - check=False, - ) + try: + done = subprocess.run( + [exe, *argv[1:], str(f)], + input=program.encode(), + capture_output=True, + timeout=timeout, + check=False, + ) + except subprocess.TimeoutExpired: + raise ToolError(f"timed out after {timeout} s") from None return { "stdout": done.stdout.decode(errors="replace")[:LIMIT], "stderr": done.stderr.decode(errors="replace")[:LIMIT], diff --git a/test_server.py b/test_server.py index c4806f8..7b4f397 100644 --- a/test_server.py +++ b/test_server.py @@ -13,6 +13,7 @@ from hypothesis import example, given from hypothesis import strategies as st from mcp import ClientSession, StdioServerParameters from mcp.client.stdio import stdio_client +from mcp.server.mcpserver.exceptions import ToolError import server @@ -64,10 +65,16 @@ def test_put_then_get_returns_the_same_dsl(grammar: str, ext: str, source: str) @example("a/b") @example("") def test_put_rejects_a_name_that_is_not_a_slug(name: str) -> None: - with pytest.raises(ValueError): + with pytest.raises(ToolError): server.put(name, "", "py", "") +def test_run_reports_a_timeout() -> None: + server.put("slow", "program <- .*", "py", "import time\ntime.sleep(30)\n") + with pytest.raises(ToolError, match="timed out"): + server.run("slow", "", timeout=0.5) + + @pytest.mark.parametrize("ext", sorted(server.RUNTIMES)) def test_each_installed_runtime_runs_an_interpreter(ext: str) -> None: if not server.runtimes()[ext]: @@ -89,6 +96,11 @@ def test_put_and_run_over_stdio() -> None: await session.initialize() tools = {tool.name for tool in (await session.list_tools()).tools} assert tools == {"runtimes", "ls", "get", "put", "run"} + # Regression: mcp 2.x hides the message of every error but ToolError, + # so the client saw only "Error executing tool get". + missing = await session.call_tool("get", {"name": "nope"}) + assert missing.is_error + assert "no DSL named nope" in getattr(missing.content[0], "text", "") dsl = { "name": "upper", "grammar": "text <- .*",