From 58e460164ea1b36240a7358b8b77b6025cc54e0e Mon Sep 17 00:00:00 2001 From: Ryo Kusnadi Date: Fri, 16 Oct 2026 09:12:06 +0700 Subject: [PATCH] mcp: answer plain handler errors with -32603 instead of code 0 Fixes #1394 --- docs/mcpgodebug.md | 6 +++++ internal/docs/mcpgodebug.src.md | 6 +++++ mcp/shared.go | 31 ++++++++++++++++++++++ mcp/shared_test.go | 47 +++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+) diff --git a/docs/mcpgodebug.md b/docs/mcpgodebug.md index f463c6b70..4bdc750da 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -54,6 +54,12 @@ Options listed below were added and will be removed in the 1.11.0 version of the fails, restoring the previous behavior. By default, the failure is returned only to the affected call. +- `noplainerrorinternalcode` added. If set to `1`, an error returned by a + handler that carries no JSON-RPC code is sent with the zero-value code `0`, + restoring the previous behavior. The default behavior was changed to send + such errors, with their message unchanged, with `-32603` (Internal error). See + issue #1394. + ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/internal/docs/mcpgodebug.src.md b/internal/docs/mcpgodebug.src.md index 2129e370b..0ad54c711 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -53,6 +53,12 @@ Options listed below were added and will be removed in the 1.11.0 version of the fails, restoring the previous behavior. By default, the failure is returned only to the affected call. +- `noplainerrorinternalcode` added. If set to `1`, an error returned by a + handler that carries no JSON-RPC code is sent with the zero-value code `0`, + restoring the previous behavior. The default behavior was changed to send + such errors, with their message unchanged, with `-32603` (Internal error). See + issue #1394. + ### 1.8.0 Options listed below were added and will be removed in the 1.9.0 version of the SDK. diff --git a/mcp/shared.go b/mcp/shared.go index 8cc117643..adbadd363 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -42,6 +42,15 @@ import ( // The option will be removed in a future version of the SDK. var nowrapinvalidparams = mcpgodebug.Value("nowrapinvalidparams") +// noplainerrorinternalcode is a compatibility parameter that restores the +// previous behavior of [handleReceive]. When unset (the default), an error +// returned by a handler that carries no JSON-RPC code is sent with code -32603 +// ("internal error") rather than the zero-value code 0. +// +// See the documentation for the mcpgodebug package for instructions how to enable it. +// The option will be removed in a future version of the SDK. +var noplainerrorinternalcode = mcpgodebug.Value("noplainerrorinternalcode") + const ( // latestProtocolVersion is the latest protocol version that this version of // the SDK supports. @@ -212,11 +221,33 @@ func handleReceive[S Session](ctx context.Context, session S, jreq *jsonrpc.Requ // mh might be user code, so ensure that it returns the right values for the jsonrpc2 protocol. res, err := mh(ctx, jreq.Method, req) if err != nil { + if noplainerrorinternalcode != "1" && !hasErrorCode(err) { + // A plain error carries no JSON-RPC code, which would reach the + // peer as code 0. Report it as an internal error instead, keeping + // the message and the original error for errors.Is/As. + err = &internalCodeError{err} + } return nil, err } return res, nil } +// hasErrorCode reports whether err is, or wraps, an error that already +// determines its JSON-RPC error code. +func hasErrorCode(err error) bool { + var we *jsonrpc2.WireError + return errors.As(err, &we) || + errors.Is(err, jsonrpc2.ErrNotHandled) || + errors.Is(err, jsonrpc2.ErrMethodNotFound) +} + +// internalCodeError marks a plain handler error to be sent with the +// JSON-RPC internal error code (-32603). Its message is that of the original. +type internalCodeError struct{ err error } + +func (e *internalCodeError) Error() string { return e.err.Error() } +func (e *internalCodeError) Unwrap() []error { return []error{e.err, jsonrpc2.ErrInternal} } + // checkRequest checks the given request against the provided method info, to // ensure it is a valid MCP request. // diff --git a/mcp/shared_test.go b/mcp/shared_test.go index 45667689b..dfce15856 100644 --- a/mcp/shared_test.go +++ b/mcp/shared_test.go @@ -5,6 +5,7 @@ package mcp import ( + "context" "encoding/json" "errors" "strings" @@ -658,3 +659,49 @@ func TestUnmarshalParamsInvalidParamsCode(t *testing.T) { t.Errorf("error message = %q, want it to mention unmarshaling", err.Error()) } } + +// TestPlainHandlerErrorCode checks that an error without a JSON-RPC code +// returned by a handler reaches the peer with CodeInternalError rather than 0, +// and that a coded error keeps its code. +func TestPlainHandlerErrorCode(t *testing.T) { + for _, tc := range []struct { + name string + err error + want int64 + }{ + {"plain", errors.New("boom"), jsonrpc.CodeInternalError}, + {"coded", &jsonrpc.Error{Code: jsonrpc.CodeInvalidParams, Message: "bad"}, jsonrpc.CodeInvalidParams}, + } { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + server := NewServer(&Implementation{Name: "s", Version: "v"}, nil) + server.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(ctx context.Context, method string, req Request) (Result, error) { + if method == methodListPrompts { + return nil, tc.err + } + return next(ctx, method, req) + } + }) + ct, st := NewInMemoryTransports() + ss, err := server.Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + defer ss.Close() + cs, err := NewClient(&Implementation{Name: "c", Version: "v"}, nil).Connect(ctx, ct, nil) + if err != nil { + t.Fatal(err) + } + defer cs.Close() + _, err = cs.ListPrompts(ctx, nil) + var jerr *jsonrpc.Error + if !errors.As(err, &jerr) { + t.Fatalf("expected *jsonrpc.Error, got %T: %v", err, err) + } + if int64(jerr.Code) != tc.want { + t.Errorf("code = %d, want %d", jerr.Code, tc.want) + } + }) + } +}