Skip to content

docs(server): document Err vs Ok(CallToolResult::error) visibility contract on ServerHandler::call_tool#854

Open
gregvirgin-ls wants to merge 1 commit into
modelcontextprotocol:mainfrom
gregvirgin-ls:visibility-contract-docs
Open

docs(server): document Err vs Ok(CallToolResult::error) visibility contract on ServerHandler::call_tool#854
gregvirgin-ls wants to merge 1 commit into
modelcontextprotocol:mainfrom
gregvirgin-ls:visibility-contract-docs

Conversation

@gregvirgin-ls
Copy link
Copy Markdown

Summary

Pure rustdoc PR. Documents on ServerHandler::call_tool and CallToolResult::error the distinction between the two failure modes the MCP spec defines, since they look identical at the Rust call site but reach the caller's UI very differently:

  • Ok(CallToolResult::error(content)) — tool-level error. The content is rendered in the caller's MCP client; the user sees the message. The right shape for almost every "the tool didn't work" path.
  • Err(McpError) — JSON-RPC protocol error. Used for METHOD_NOT_FOUND, INVALID_PARAMS (-32602), INTERNAL_ERROR (-32603), and other unroutable-request cases. Clients render these opaquely — the caller does not see the message text.

The two-failure-mode shape exists in the SDK today but isn't currently explained anywhere a handler author would notice. In practice, handlers reach for Err(...) because it looks like the natural Rust return value, and the caller sees "Tool result missing due to internal error" instead of the actual failure reason.

What this PR does

  1. crates/rmcp/src/handler/server.rs — rustdoc on the call_tool method generated by server_handler_methods!. Explains both return paths, when each is correct, and points at CallToolResult::error for the worked example.
  2. crates/rmcp/src/model.rs — rustdoc on CallToolResult::error with a worked example contrasting:
    • Protocol error: caller passes a missing required parameter → Err(McpError::invalid_params(...)).
    • Tool error: caller asks for a row that doesn't exist → Ok(CallToolResult::error(vec![Content::text("no such row")])).

Total diff: 81 lines added across the two files.

What this PR does NOT do

  • Does not add any new types. A future change may introduce a typed ToolOutcome sum type to enforce the distinction at compile time (tracked as a separate follow-up); this PR is the lower-risk docs-only version that unblocks the class immediately.
  • Does not document any dispatcher behaviour (panic catching, default timeouts, etc.). Those are the subject of separate planned work on opt-in safety semantics for call_tool; once that lands, a small follow-up doc-update PR will extend the rustdoc here to cover the new behaviour. Keeping the two changes independent so each can land on its own merit.

Behaviour change

None. Pure rustdoc addition.

Test matrix

cargo fmt --all -- --check
cargo clippy --all-targets --all-features -- -D warnings
cargo test --all-features
cargo doc -p rmcp --features "client,server,macros" --no-deps

(Local verification: rebased onto current upstream/main (v1.7.0+ tip); cargo build -p rmcp --features "client,server,macros" and cargo doc -p rmcp --features "client,server,macros" --no-deps both succeeded. Full test/clippy matrix above not run locally — left to CI.)

Downstream motivation

Multiple MCP-server consumers we've worked with built up the Err-everywhere pattern in their handlers, then had to do a sweep when they realised callers couldn't read their error messages. The docstring is the cheapest possible intervention that prevents that sweep for future implementations.

…ntract

The MCP spec separates two failure modes that surface very differently in
clients:

  - Err(ErrorData) is a JSON-RPC protocol error. Most MCP clients render
    it opaquely ("Tool result missing due to internal error") - the
    caller does not see the message text.
  - Ok(CallToolResult::error(content)) is a tool-level error. Clients
    render the content; the caller reads the message.

The right shape for "the tool didn't work" is the latter, but Err is
what most handlers reach for because it looks like the natural Rust
return value. This commit adds rustdoc on both ServerHandler::call_tool
and CallToolResult::error pointing handlers at the correct shape, with
a worked example showing protocol errors (-32602 invalid_params) vs
tool errors (empty result, downstream failure).

This is the docs half of the visibility-contract ask. A follow-up may
introduce a typed ToolOutcome sum type to enforce the distinction at
compile time; this PR is the lower-risk version that unblocks the
class immediately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@gregvirgin-ls gregvirgin-ls requested a review from a team as a code owner May 18, 2026 00:18
@github-actions github-actions Bot added T-core Core library changes T-handler Handler implementation changes labels May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-core Core library changes T-handler Handler implementation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant