From 902dfd20948729caf60478315d64fccf16c4fe1f Mon Sep 17 00:00:00 2001 From: Ravish Date: Fri, 27 Mar 2026 13:58:39 -0700 Subject: [PATCH] mcp: return input validation errors as tool results, not JSON-RPC errors Input validation errors (missing required fields, wrong types) during tool calls are now returned as CallToolResult with IsError=true instead of JSON-RPC -32602 errors. This allows LLMs to see the validation error and self-correct, per the MCP spec. Fixes #450 --- mcp/error_test.go | 52 ++++++++++++++++----- mcp/server.go | 9 ++-- mcp/server_test.go | 13 ++++-- mcp/testdata/conformance/server/tools.txtar | 22 ++++++--- 4 files changed, 73 insertions(+), 23 deletions(-) diff --git a/mcp/error_test.go b/mcp/error_test.go index dad47aa9..47a99f81 100644 --- a/mcp/error_test.go +++ b/mcp/error_test.go @@ -51,17 +51,9 @@ func TestServerErrors(t *testing.T) { executeCall func() error expectedCode int64 }{ - { - name: "missing required param", - executeCall: func() error { - _, err := cs.CallTool(ctx, &CallToolParams{ - Name: "validate", - Arguments: map[string]any{}, // Missing required "name" field - }) - return err - }, - expectedCode: jsonrpc.CodeInvalidParams, - }, + // Note: "missing required param" is tested separately below, because + // input validation errors are returned as tool results with IsError=true + // rather than JSON-RPC errors (see #450). { name: "unknown tool", executeCall: func() error { @@ -119,6 +111,44 @@ func TestServerErrors(t *testing.T) { } } +// TestInputValidationToolError validates that input validation errors (missing +// required params, wrong types) are returned as tool results with IsError=true, +// not as JSON-RPC errors. This allows LLMs to see the error and self-correct. +// See #450. +func TestInputValidationToolError(t *testing.T) { + ctx := context.Background() + + type RequiredParams struct { + Name string `json:"name" jsonschema:"the name is required"` + } + handler := func(ctx context.Context, req *CallToolRequest, args RequiredParams) (*CallToolResult, any, error) { + return &CallToolResult{ + Content: []Content{&TextContent{Text: "success"}}, + }, nil, nil + } + + cs, _, cleanup := basicConnection(t, func(s *Server) { + AddTool(s, &Tool{Name: "validate", Description: "validates params"}, handler) + }) + defer cleanup() + + // Call the tool with missing required "name" field. + result, err := cs.CallTool(ctx, &CallToolParams{ + Name: "validate", + Arguments: map[string]any{}, + }) + if err != nil { + t.Fatalf("CallTool returned error: %v; want tool result with IsError", err) + } + if !result.IsError { + t.Fatal("got IsError=false, want IsError=true for missing required param") + } + text := result.Content[0].(*TextContent).Text + if !strings.Contains(text, "name") { + t.Errorf("error text %q does not mention missing field \"name\"", text) + } +} + // TestURLElicitationRequired validates that URL elicitation required errors // are properly created and handled by the client. func TestURLElicitationRequired(t *testing.T) { diff --git a/mcp/server.go b/mcp/server.go index e3c03e27..b8f35809 100644 --- a/mcp/server.go +++ b/mcp/server.go @@ -321,15 +321,18 @@ func toolForErr[In, Out any](t *Tool, h ToolHandlerFor[In, Out], cache *SchemaCa var err error input, err = applySchema(input, inputResolved) if err != nil { - // TODO(#450): should this be considered a tool error? (and similar below) - return nil, fmt.Errorf("%w: validating \"arguments\": %v", jsonrpc2.ErrInvalidParams, err) + var errRes CallToolResult + errRes.SetError(fmt.Errorf("validating \"arguments\": %v", err)) + return &errRes, nil } // Unmarshal and validate args. var in In if input != nil { if err := internaljson.Unmarshal(input, &in); err != nil { - return nil, fmt.Errorf("%w: %v", jsonrpc2.ErrInvalidParams, err) + var errRes CallToolResult + errRes.SetError(err) + return &errRes, nil } } diff --git a/mcp/server_test.go b/mcp/server_test.go index 227e7be3..388045b8 100644 --- a/mcp/server_test.go +++ b/mcp/server_test.go @@ -676,12 +676,19 @@ func testToolForSchema[In, Out any](t *testing.T, tool *Tool, in string, out Out } result, err := goth(context.Background(), ctr) if wantErrContaining != "" { - if err == nil { - t.Errorf("got nil error, want error containing %q", wantErrContaining) - } else { + // Input validation errors are returned as tool results with IsError=true, + // not as Go errors. Check both possibilities. + if err != nil { if !strings.Contains(err.Error(), wantErrContaining) { t.Errorf("got error %q, want containing %q", err, wantErrContaining) } + } else if result != nil && result.IsError { + text := result.Content[0].(*TextContent).Text + if !strings.Contains(text, wantErrContaining) { + t.Errorf("got tool error %q, want containing %q", text, wantErrContaining) + } + } else { + t.Errorf("got no error, want error containing %q", wantErrContaining) } } else if err != nil { t.Errorf("got error %v, want no error", err) diff --git a/mcp/testdata/conformance/server/tools.txtar b/mcp/testdata/conformance/server/tools.txtar index ba0facc2..aadad122 100644 --- a/mcp/testdata/conformance/server/tools.txtar +++ b/mcp/testdata/conformance/server/tools.txtar @@ -222,9 +222,14 @@ inc { "jsonrpc": "2.0", "id": 8, - "error": { - "code": -32602, - "message": "invalid params: validating \"arguments\": validating root: required: missing properties: [\"In\"]" + "result": { + "content": [ + { + "type": "text", + "text": "validating \"arguments\": validating root: required: missing properties: [\"In\"]" + } + ], + "isError": true } } { @@ -245,9 +250,14 @@ inc { "jsonrpc": "2.0", "id": 10, - "error": { - "code": -32602, - "message": "invalid params: validating \"arguments\": validating root: required: missing properties: [\"Name\"]" + "result": { + "content": [ + { + "type": "text", + "text": "validating \"arguments\": validating root: required: missing properties: [\"Name\"]" + } + ], + "isError": true } } {