From 231677f761a222c0dd0f7dc87800482ee07a07b8 Mon Sep 17 00:00:00 2001 From: Ayla Croft Date: Sun, 6 Sep 2026 16:32:01 -0400 Subject: [PATCH] Carry errors across the wire as JSON, not as Elixir terms A failed tool call returned this to the client: "structuredContent": {"error": "%{reason: \"invalid arguments: missing required property: a\", tool: :t}"} That is inspect/1 output on a map: map-literal syntax, a bare atom with its leading colon, Elixir-escaped inner quotes. The useful parts -- which tool, which property, why -- were all present and recoverable only by parsing Elixir. It was wrong at every protocol revision, which is why it was not folded into the negotiation slice. Red first: 5 tests, 3 failures a validation failure carries structured fields, not an inspected map the human-readable content carries no Elixir syntax either a host's error term is carried as JSON, not as an inspected term Green: 5 tests 0 failures, suite 33 0, gate exit 0. was "error": "%{reason: \"invalid arguments: ...\", tool: :t}" now "error": {"tool": "widget", "reason": "invalid arguments: missing required property: a"} "text": "widget: invalid arguments: missing required property: a" structuredContent carries the error as data so a client can read a field; content carries a sentence a person can read. format_reason/1 and its inspect/1 fallback are gone, and no inspect/1 remains in server.ex. A host's error term is carried as JSON too, so a dispatch returning {:error, %{code: "upstream_timeout", retry_after: 30}} reaches the client as an object with those fields rather than as a stringified term. A plain-string error survives unchanged. One existing assertion used =~ against what was a string and is now an object; it now reads the reason field. That is a consequence of the shape change. The tests assert the absence of Elixir syntax by pattern -- map literals and bare atoms -- rather than by comparing to a fixed string, so a future regression of a different shape is still caught. Signed-off-by: Ayla Croft --- CHANGELOG.md | 10 ++- lib/beam_mcp/server.ex | 32 ++++++-- test/beam_mcp/error_payload_test.exs | 100 ++++++++++++++++++++++++ test/beam_mcp/tool_spec_schema_test.exs | 3 +- 4 files changed, 134 insertions(+), 11 deletions(-) create mode 100644 test/beam_mcp/error_payload_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b97695..0fac369 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,6 +57,13 @@ fails quietly against the revision most evaluators would try first. - Argument keys are derived from the tool's schema; values pass through unchanged, since coercing a string to a domain term belongs to the host (`c5da02f`). +### Fixed + +- **Error payloads no longer carry `inspect/1` output.** A failed call returned Elixir term + syntax — a map literal and a bare atom — to a client with no way to parse it and no reason + to know the server's language. `structuredContent` now carries the error as a JSON object + and `content` carries a sentence. + ### Removed - Per-tool `input_schema/1` clauses, `input_schema_for/1`, the hardcoded argument-key @@ -84,7 +91,6 @@ Each is additive and can be adopted without a breaking change. - CI has not run against this history at the time of writing; a committed workflow is not a working one until a run exists. -- Error payloads carry `inspect/1` output, so Elixir term syntax reaches the wire. A boundary - defect at every revision, and its own slice before publish. +- Streamable HTTP and the rest of the non-stdio surface (see below). - Streamable HTTP, `subscriptions/listen`, MRTR, tasks, authorization, elicitation, sampling and roots are not implemented. This is a stdio, tools-only server. diff --git a/lib/beam_mcp/server.ex b/lib/beam_mcp/server.ex index c48c64f..bd33440 100644 --- a/lib/beam_mcp/server.ex +++ b/lib/beam_mcp/server.ex @@ -195,16 +195,34 @@ defmodule BeamMCP.Server do } end + # An error crossing the wire carries JSON. `structuredContent` gets the term as data so a + # client can read a field; `content` gets a sentence a person can read. Neither carries + # Elixir syntax: a caller has no reason to know what language this is written in, and no way + # to parse its terms. defp tool_failure(reason) do - message = format_reason(reason) - %{ - "content" => [%{"type" => "text", "text" => message}], - "structuredContent" => %{"error" => message}, + "content" => [%{"type" => "text", "text" => error_text(reason)}], + "structuredContent" => %{"error" => to_json_value(reason)}, "isError" => true } end + defp error_text(reason) when is_binary(reason), do: reason + + defp error_text(%{"tool" => tool, "reason" => detail}), do: "#{tool}: #{detail}" + + defp error_text(reason) when is_map(reason) do + reason + |> to_json_value() + |> Enum.map_join(", ", fn {key, value} -> "#{key}: #{stringify(value)}" end) + end + + defp error_text(reason), do: stringify(to_json_value(reason)) + + defp stringify(value) when is_binary(value), do: value + defp stringify(value) when is_number(value), do: to_string(value) + defp stringify(value), do: Jason.encode!(value) + # One lookup governs both paths: a tool is callable exactly when the injected catalog names # it, and the spec it returns carries the schema that will be enforced. defp find_tool(state, name) when is_binary(name) do @@ -228,7 +246,8 @@ defmodule BeamMCP.Server do state.dispatch.(spec.name, args, state.dispatch_opts) {:error, reason} -> - {:error, %{tool: spec.name, reason: "invalid arguments: #{reason}"}} + {:error, + %{"tool" => Atom.to_string(spec.name), "reason" => "invalid arguments: #{reason}"}} end end @@ -270,9 +289,6 @@ defmodule BeamMCP.Server do defp to_json_value(value) when is_atom(value), do: Atom.to_string(value) defp to_json_value(value), do: value - defp format_reason(reason) when is_binary(reason), do: reason - defp format_reason(reason), do: inspect(reason) - defp unsupported_version(id, requested) do %{ "jsonrpc" => "2.0", diff --git a/test/beam_mcp/error_payload_test.exs b/test/beam_mcp/error_payload_test.exs new file mode 100644 index 0000000..9cecece --- /dev/null +++ b/test/beam_mcp/error_payload_test.exs @@ -0,0 +1,100 @@ +# SPDX-FileCopyrightText: 2026 Sudo Apt Holdings LLC +# SPDX-License-Identifier: Apache-2.0 + +defmodule BeamMCP.ErrorPayloadTest do + @moduledoc """ + An error crossing the wire carries JSON, not Elixir. + + A client has no reason to know what language the server is written in, and no way to parse + its term syntax. `inspect/1` output in a response is both unusable and a disclosure of + implementation detail across the boundary this package exists to keep clean. + """ + use ExUnit.Case, async: true + + alias BeamMCP.Server + + defmodule Catalog do + @behaviour BeamMCP.ToolCatalog + + @impl true + def all do + [ + %BeamMCP.ToolSpec{ + name: :widget, + command_class: :observe, + mode: :read_only, + description: "d", + input_schema: %{ + "type" => "object", + "properties" => %{"a" => %{"type" => "string"}}, + "required" => ["a"] + } + } + ] + end + end + + defp call(dispatch, args) do + Server.new(tool_catalog: Catalog, dispatch: dispatch) + |> Server.handle_message(%{ + "jsonrpc" => "2.0", + "id" => 1, + "method" => "tools/call", + "params" => %{"name" => "widget", "arguments" => args} + }) + |> elem(1) + end + + defp ok_dispatch, do: fn _n, a, _o -> {:ok, a} end + + # Elixir syntax that must never appear in a wire payload: map literals, bare atoms. + defp elixir_syntax?(text), do: text =~ ~r/%\{|(? {:error, %{code: "upstream_timeout", retry_after: 30}} end, %{ + "a" => "x" + }) + + err = r["result"]["structuredContent"]["error"] + + assert err["code"] == "upstream_timeout" + assert err["retry_after"] == 30 + refute elixir_syntax?(Jason.encode!(err)) + end + + test "a host's plain-string error survives unchanged" do + r = call(fn _n, _a, _o -> {:error, "upstream unavailable"} end, %{"a" => "x"}) + + assert r["result"]["structuredContent"]["error"] == "upstream unavailable" + assert hd(r["result"]["content"])["text"] == "upstream unavailable" + end + + test "every error result is still flagged isError" do + assert call(ok_dispatch(), %{})["result"]["isError"] + assert call(fn _n, _a, _o -> {:error, "x"} end, %{"a" => "y"})["result"]["isError"] + end +end diff --git a/test/beam_mcp/tool_spec_schema_test.exs b/test/beam_mcp/tool_spec_schema_test.exs index 0b7fc30..309ee9a 100644 --- a/test/beam_mcp/tool_spec_schema_test.exs +++ b/test/beam_mcp/tool_spec_schema_test.exs @@ -75,7 +75,8 @@ defmodule BeamMCP.ToolSpecSchemaTest do assert resp["result"]["isError"], "the server accepted a call that violates the schema its own catalog advertises" - assert resp["result"]["structuredContent"]["error"] =~ "widget_id" + # The error is a JSON object now, not a string: `=~` no longer applies to it. + assert resp["result"]["structuredContent"]["error"]["reason"] =~ "widget_id" end test "a call carrying a property the catalog's schema forbids is refused" do