Skip to content

Commit 9576afc

Browse files
mcp: wrap unmarshalParams with jsonrpc error (#1087)
Fixes #976
1 parent 0a8953d commit 9576afc

4 files changed

Lines changed: 63 additions & 1 deletion

File tree

docs/mcpgodebug.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,13 @@ Options listed below were added and will be removed in the 1.9.0 version of the
5050
restoring the previous behavior. The default behavior was changed so that
5151
the client always attempts to surface the underlying JSON-RPC error.
5252

53+
- `nowrapinvalidparams` added. If set to `1`, the server will not wrap
54+
params-decoding failures with `jsonrpc2.ErrInvalidParams`, so wire responses
55+
carry the zero-value error code `0` instead of `-32602` ("invalid params"),
56+
restoring the previous behavior. The default behavior was changed so that
57+
the server always reports params-decoding failures with the standard
58+
`-32602` code.
59+
5360
- `disablecompleteparamsvalidation` added. If set to `1`, `Server.complete`
5461
will not validate that the required `ref` and `argument.name` fields on
5562
`CompleteParams` are present, restoring the previous behavior of dispatching

internal/docs/mcpgodebug.src.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,13 @@ Options listed below were added and will be removed in the 1.9.0 version of the
4949
restoring the previous behavior. The default behavior was changed so that
5050
the client always attempts to surface the underlying JSON-RPC error.
5151

52+
- `nowrapinvalidparams` added. If set to `1`, the server will not wrap
53+
params-decoding failures with `jsonrpc2.ErrInvalidParams`, so wire responses
54+
carry the zero-value error code `0` instead of `-32602` ("invalid params"),
55+
restoring the previous behavior. The default behavior was changed so that
56+
the server always reports params-decoding failures with the standard
57+
`-32602` code.
58+
5259
- `disablecompleteparamsvalidation` added. If set to `1`, `Server.complete`
5360
will not validate that the required `ref` and `argument.name` fields on
5461
`CompleteParams` are present, restoring the previous behavior of dispatching

mcp/shared.go

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,21 @@ import (
2626
"github.com/modelcontextprotocol/go-sdk/auth"
2727
internaljson "github.com/modelcontextprotocol/go-sdk/internal/json"
2828
"github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2"
29+
"github.com/modelcontextprotocol/go-sdk/internal/mcpgodebug"
2930
"github.com/modelcontextprotocol/go-sdk/jsonrpc"
3031
)
3132

33+
// nowrapinvalidparams is a compatibility parameter that restores the previous
34+
// behavior of [methodInfo.unmarshalParams]. When unset (the default), a
35+
// params-decoding failure is wrapped with [jsonrpc2.ErrInvalidParams] so the
36+
// wire response carries error code -32602 ("invalid params") rather than the
37+
// zero-value code 0. See:
38+
// https://github.com/modelcontextprotocol/go-sdk/issues/976#issuecomment-4829124838.
39+
//
40+
// See the documentation for the mcpgodebug package for instructions how to enable it.
41+
// The option will be removed in a future version of the SDK.
42+
var nowrapinvalidparams = mcpgodebug.Value("nowrapinvalidparams")
43+
3244
const (
3345
// latestProtocolVersion is the latest protocol version that this version of
3446
// the SDK supports.
@@ -304,7 +316,17 @@ func newMethodInfo[P paramsPtr[T], R Result, T any](flags methodFlags) methodInf
304316
var p P
305317
if m != nil {
306318
if err := internaljson.Unmarshal(m, &p); err != nil {
307-
return nil, fmt.Errorf("unmarshaling %q into a %T: %w", m, p, err)
319+
// Legacy behavior: pre-fix versions surfaced this as a
320+
// plain wrapped error, which caused the wire response to
321+
// carry code 0 instead of -32602. Restore via
322+
// MCPGODEBUG=nowrapinvalidparams=1.
323+
if nowrapinvalidparams == "1" {
324+
return nil, fmt.Errorf("unmarshaling %q into a %T: %w", m, p, err)
325+
}
326+
// Wrap jsonrpc2.ErrInvalidParams so toWireError surfaces
327+
// code -32602 ("invalid params") while preserving the
328+
// descriptive message.
329+
return nil, fmt.Errorf("%w: unmarshaling %q into a %T: %w", jsonrpc2.ErrInvalidParams, m, p, err)
308330
}
309331
}
310332
// We must check missingParamsOK here, in addition to checkRequest, to

mcp/shared_test.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -512,3 +512,29 @@ func TestImplementationDescriptionJSON(t *testing.T) {
512512
// }
513513
// })
514514
// }
515+
516+
// TestUnmarshalParamsInvalidParamsCode is a regression test for
517+
// https://github.com/modelcontextprotocol/go-sdk/issues/976#issuecomment-4829124838.
518+
// Malformed params for a registered method must surface as a JSON-RPC error
519+
// whose code is CodeInvalidParams (-32602) rather than the zero-value 0.
520+
func TestUnmarshalParamsInvalidParamsCode(t *testing.T) {
521+
info, ok := serverMethodInfos[methodPing]
522+
if !ok {
523+
t.Fatalf("no methodInfo for %q", methodPing)
524+
}
525+
// Array where a struct is expected.
526+
_, err := info.unmarshalParams(json.RawMessage(`["a","b"]`))
527+
if err == nil {
528+
t.Fatal("unmarshalParams returned nil error for malformed params")
529+
}
530+
var jerr *jsonrpc.Error
531+
if !errors.As(err, &jerr) {
532+
t.Fatalf("expected *jsonrpc.Error in chain, got %T: %v", err, err)
533+
}
534+
if jerr.Code != jsonrpc.CodeInvalidParams {
535+
t.Errorf("error code = %d, want %d (CodeInvalidParams)", jerr.Code, jsonrpc.CodeInvalidParams)
536+
}
537+
if !strings.Contains(err.Error(), "unmarshaling") {
538+
t.Errorf("error message = %q, want it to mention unmarshaling", err.Error())
539+
}
540+
}

0 commit comments

Comments
 (0)