Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 41 additions & 11 deletions mcp/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Expand Down
9 changes: 6 additions & 3 deletions mcp/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}

Expand Down
13 changes: 10 additions & 3 deletions mcp/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
22 changes: 16 additions & 6 deletions mcp/testdata/conformance/server/tools.txtar
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
{
Expand All @@ -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
}
}
{
Expand Down