From 9576afc114e59ad30b6070f9ffc5982a81bf6984 Mon Sep 17 00:00:00 2001 From: Guglielmo Colombo Date: Tue, 14 Jul 2026 16:26:18 +0200 Subject: [PATCH] mcp: wrap unmarshalParams with jsonrpc error (#1087) Fixes #976 --- docs/mcpgodebug.md | 7 +++++++ internal/docs/mcpgodebug.src.md | 7 +++++++ mcp/shared.go | 24 +++++++++++++++++++++++- mcp/shared_test.go | 26 ++++++++++++++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) diff --git a/docs/mcpgodebug.md b/docs/mcpgodebug.md index 847e2e7..45b9c7d 100644 --- a/docs/mcpgodebug.md +++ b/docs/mcpgodebug.md @@ -50,6 +50,13 @@ Options listed below were added and will be removed in the 1.9.0 version of the restoring the previous behavior. The default behavior was changed so that the client always attempts to surface the underlying JSON-RPC error. +- `nowrapinvalidparams` added. If set to `1`, the server will not wrap + params-decoding failures with `jsonrpc2.ErrInvalidParams`, so wire responses + carry the zero-value error code `0` instead of `-32602` ("invalid params"), + restoring the previous behavior. The default behavior was changed so that + the server always reports params-decoding failures with the standard + `-32602` code. + - `disablecompleteparamsvalidation` added. If set to `1`, `Server.complete` will not validate that the required `ref` and `argument.name` fields on `CompleteParams` are present, restoring the previous behavior of dispatching diff --git a/internal/docs/mcpgodebug.src.md b/internal/docs/mcpgodebug.src.md index 5c9c9d9..e3ca770 100644 --- a/internal/docs/mcpgodebug.src.md +++ b/internal/docs/mcpgodebug.src.md @@ -49,6 +49,13 @@ Options listed below were added and will be removed in the 1.9.0 version of the restoring the previous behavior. The default behavior was changed so that the client always attempts to surface the underlying JSON-RPC error. +- `nowrapinvalidparams` added. If set to `1`, the server will not wrap + params-decoding failures with `jsonrpc2.ErrInvalidParams`, so wire responses + carry the zero-value error code `0` instead of `-32602` ("invalid params"), + restoring the previous behavior. The default behavior was changed so that + the server always reports params-decoding failures with the standard + `-32602` code. + - `disablecompleteparamsvalidation` added. If set to `1`, `Server.complete` will not validate that the required `ref` and `argument.name` fields on `CompleteParams` are present, restoring the previous behavior of dispatching diff --git a/mcp/shared.go b/mcp/shared.go index 0fc19c5..6e8419a 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -26,9 +26,21 @@ import ( "github.com/modelcontextprotocol/go-sdk/auth" internaljson "github.com/modelcontextprotocol/go-sdk/internal/json" "github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2" + "github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug" "github.com/modelcontextprotocol/go-sdk/jsonrpc" ) +// nowrapinvalidparams is a compatibility parameter that restores the previous +// behavior of [methodInfo.unmarshalParams]. When unset (the default), a +// params-decoding failure is wrapped with [jsonrpc2.ErrInvalidParams] so the +// wire response carries error code -32602 ("invalid params") rather than the +// zero-value code 0. See: +// https://github.com/modelcontextprotocol/go-sdk/issues/976#issuecomment-4829124838. +// +// 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 nowrapinvalidparams = mcpgodebug.Value("nowrapinvalidparams") + const ( // latestProtocolVersion is the latest protocol version that this version of // the SDK supports. @@ -304,7 +316,17 @@ func newMethodInfo[P paramsPtr[T], R Result, T any](flags methodFlags) methodInf var p P if m != nil { if err := internaljson.Unmarshal(m, &p); err != nil { - return nil, fmt.Errorf("unmarshaling %q into a %T: %w", m, p, err) + // Legacy behavior: pre-fix versions surfaced this as a + // plain wrapped error, which caused the wire response to + // carry code 0 instead of -32602. Restore via + // MCPGODEBUG=nowrapinvalidparams=1. + if nowrapinvalidparams == "1" { + return nil, fmt.Errorf("unmarshaling %q into a %T: %w", m, p, err) + } + // Wrap jsonrpc2.ErrInvalidParams so toWireError surfaces + // code -32602 ("invalid params") while preserving the + // descriptive message. + return nil, fmt.Errorf("%w: unmarshaling %q into a %T: %w", jsonrpc2.ErrInvalidParams, m, p, err) } } // We must check missingParamsOK here, in addition to checkRequest, to diff --git a/mcp/shared_test.go b/mcp/shared_test.go index 1d0ec70..caf5dd6 100644 --- a/mcp/shared_test.go +++ b/mcp/shared_test.go @@ -512,3 +512,29 @@ func TestImplementationDescriptionJSON(t *testing.T) { // } // }) // } + +// TestUnmarshalParamsInvalidParamsCode is a regression test for +// https://github.com/modelcontextprotocol/go-sdk/issues/976#issuecomment-4829124838. +// Malformed params for a registered method must surface as a JSON-RPC error +// whose code is CodeInvalidParams (-32602) rather than the zero-value 0. +func TestUnmarshalParamsInvalidParamsCode(t *testing.T) { + info, ok := serverMethodInfos[methodPing] + if !ok { + t.Fatalf("no methodInfo for %q", methodPing) + } + // Array where a struct is expected. + _, err := info.unmarshalParams(json.RawMessage(`["a","b"]`)) + if err == nil { + t.Fatal("unmarshalParams returned nil error for malformed params") + } + var jerr *jsonrpc.Error + if !errors.As(err, &jerr) { + t.Fatalf("expected *jsonrpc.Error in chain, got %T: %v", err, err) + } + if jerr.Code != jsonrpc.CodeInvalidParams { + t.Errorf("error code = %d, want %d (CodeInvalidParams)", jerr.Code, jsonrpc.CodeInvalidParams) + } + if !strings.Contains(err.Error(), "unmarshaling") { + t.Errorf("error message = %q, want it to mention unmarshaling", err.Error()) + } +}