Skip to content

Commit bfb167d

Browse files
committed
improve error handling for tools and server methods (modelcontextprotocol#311)
- Fix tool error handling to properly distinguish between protocol errors and tool execution errors - Return structured JSON-RPC errors directly for protocol-level issues - Embed regular tool errors in CallToolResult as per MCP specification - Improve server error responses for unknown prompts and tools - Add comprehensive tests for both structured and regular error handling
1 parent 6ec60dd commit bfb167d

File tree

4 files changed

+113
-5
lines changed

4 files changed

+113
-5
lines changed

mcp/server.go

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,11 @@ func (s *Server) getPrompt(ctx context.Context, req *ServerRequest[*GetPromptPar
328328
prompt, ok := s.prompts.get(req.Params.Name)
329329
s.mu.Unlock()
330330
if !ok {
331-
// TODO: surface the error code over the wire, instead of flattening it into the string.
332-
return nil, fmt.Errorf("%s: unknown prompt %q", jsonrpc2.ErrInvalidParams, req.Params.Name)
331+
// Return a proper JSON-RPC error with the correct error code
332+
return nil, &jsonrpc2.WireError{
333+
Code: CodeInvalidParams,
334+
Message: fmt.Sprintf("unknown prompt %q", req.Params.Name),
335+
}
333336
}
334337
return prompt.handler(ctx, req.Session, req.Params)
335338
}
@@ -353,7 +356,10 @@ func (s *Server) callTool(ctx context.Context, req *ServerRequest[*CallToolParam
353356
st, ok := s.tools.get(req.Params.Name)
354357
s.mu.Unlock()
355358
if !ok {
356-
return nil, fmt.Errorf("%s: unknown tool %q", jsonrpc2.ErrInvalidParams, req.Params.Name)
359+
return nil, &jsonrpc2.WireError{
360+
Code: CodeInvalidParams,
361+
Message: fmt.Sprintf("unknown tool %q", req.Params.Name),
362+
}
357363
}
358364
// TODO: if handler returns nil content, it will serialize as null.
359365
// Add a test and fix.

mcp/shared.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,8 @@ const (
311311
CodeResourceNotFound = -32002
312312
// The error code if the method exists and was called properly, but the peer does not support it.
313313
CodeUnsupportedMethod = -31001
314+
// The error code for invalid parameters
315+
CodeInvalidParams = -32602
314316
)
315317

316318
// notifySessions calls Notify on all the sessions.

mcp/tool.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"reflect"
1313

1414
"github.com/google/jsonschema-go/jsonschema"
15+
"github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2"
1516
)
1617

1718
// A ToolHandler handles a call to tools/call.
@@ -70,9 +71,16 @@ func newServerTool[In, Out any](t *Tool, h ToolHandlerFor[In, Out]) (*serverTool
7071
Params: params,
7172
Extra: req.Extra,
7273
})
73-
// TODO(rfindley): investigate why server errors are embedded in this strange way,
74-
// rather than returned as jsonrpc2 server errors.
74+
// Handle server errors appropriately:
75+
// - If the handler returns a structured error (like jsonrpc2.WireError), return it directly
76+
// - If the handler returns a regular error, wrap it in a CallToolResult with IsError=true
77+
// - This allows tools to distinguish between protocol errors and tool execution errors
7578
if err != nil {
79+
// Check if this is already a structured JSON-RPC error
80+
if wireErr, ok := err.(*jsonrpc2.WireError); ok {
81+
return nil, wireErr
82+
}
83+
// For regular errors, embed them in the tool result as per MCP spec
7684
return &CallToolResult{
7785
Content: []Content{&TextContent{Text: err.Error()}},
7886
IsError: true,

mcp/tool_test.go

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,16 @@ package mcp
77
import (
88
"context"
99
"encoding/json"
10+
"errors"
11+
"fmt"
1012
"reflect"
13+
"strings"
1114
"testing"
1215

1316
"github.com/google/go-cmp/cmp"
1417
"github.com/google/go-cmp/cmp/cmpopts"
1518
"github.com/google/jsonschema-go/jsonschema"
19+
"github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2"
1620
)
1721

1822
// testToolHandler is used for type inference in TestNewServerTool.
@@ -132,3 +136,91 @@ func TestUnmarshalSchema(t *testing.T) {
132136

133137
}
134138
}
139+
140+
func TestToolErrorHandling(t *testing.T) {
141+
// Construct server and add both tools at the top level
142+
server := NewServer(testImpl, nil)
143+
144+
// Create a tool that returns a structured error
145+
structuredErrorHandler := func(ctx context.Context, req *ServerRequest[*CallToolParamsFor[map[string]any]]) (*CallToolResultFor[any], error) {
146+
return nil, &jsonrpc2.WireError{
147+
Code: CodeInvalidParams,
148+
Message: "internal server error",
149+
}
150+
}
151+
152+
// Create a tool that returns a regular error
153+
regularErrorHandler := func(ctx context.Context, req *ServerRequest[*CallToolParamsFor[map[string]any]]) (*CallToolResultFor[any], error) {
154+
return nil, fmt.Errorf("tool execution failed")
155+
}
156+
157+
AddTool(server, &Tool{Name: "error_tool", Description: "returns structured error"}, structuredErrorHandler)
158+
AddTool(server, &Tool{Name: "regular_error_tool", Description: "returns regular error"}, regularErrorHandler)
159+
160+
// Connect server and client once
161+
ct, st := NewInMemoryTransports()
162+
_, err := server.Connect(context.Background(), st, nil)
163+
if err != nil {
164+
t.Fatal(err)
165+
}
166+
167+
client := NewClient(testImpl, nil)
168+
cs, err := client.Connect(context.Background(), ct, nil)
169+
if err != nil {
170+
t.Fatal(err)
171+
}
172+
defer cs.Close()
173+
174+
// Test that structured JSON-RPC errors are returned directly
175+
t.Run("structured_error", func(t *testing.T) {
176+
// Call the tool
177+
_, err = cs.CallTool(context.Background(), &CallToolParams{
178+
Name: "error_tool",
179+
Arguments: map[string]any{},
180+
})
181+
182+
// Should get the structured error directly
183+
if err == nil {
184+
t.Fatal("expected error, got nil")
185+
}
186+
187+
var wireErr *jsonrpc2.WireError
188+
if !errors.As(err, &wireErr) {
189+
t.Fatalf("expected WireError, got %[1]T: %[1]v", err)
190+
}
191+
192+
if wireErr.Code != CodeInvalidParams {
193+
t.Errorf("expected error code %d, got %d", CodeInvalidParams, wireErr.Code)
194+
}
195+
})
196+
197+
// Test that regular errors are embedded in tool results
198+
t.Run("regular_error", func(t *testing.T) {
199+
// Call the tool
200+
result, err := cs.CallTool(context.Background(), &CallToolParams{
201+
Name: "regular_error_tool",
202+
Arguments: map[string]any{},
203+
})
204+
205+
// Should not get an error at the protocol level
206+
if err != nil {
207+
t.Fatalf("unexpected protocol error: %v", err)
208+
}
209+
210+
// Should get a result with IsError=true
211+
if !result.IsError {
212+
t.Error("expected IsError=true, got false")
213+
}
214+
215+
// Should have error message in content
216+
if len(result.Content) == 0 {
217+
t.Error("expected error content, got empty")
218+
}
219+
220+
if textContent, ok := result.Content[0].(*TextContent); !ok {
221+
t.Error("expected TextContent")
222+
} else if !strings.Contains(textContent.Text, "tool execution failed") {
223+
t.Errorf("expected error message in content, got: %s", textContent.Text)
224+
}
225+
})
226+
}

0 commit comments

Comments
 (0)