From b37e497f0a19d4b03881650a985ab27ca741827d Mon Sep 17 00:00:00 2001 From: Maciej Kisiel Date: Wed, 22 Apr 2026 16:35:27 +0200 Subject: [PATCH] mcp: don't close session when keepalive ping returns method-not-found (#900) Since ping is optional per the MCP spec, a peer that doesn't implement it will respond with a -32601 (method not found) error. Previously, startKeepalive treated all ping errors identically, closing the session even when the peer was perfectly healthy. Now, a method-not-found response stops the keepalive goroutine without closing the session. Fixes #897 --- mcp/mcp_test.go | 56 +++++++++++++++++++++++++++++++++++++++++++++++++ mcp/shared.go | 5 +++++ 2 files changed, 61 insertions(+) diff --git a/mcp/mcp_test.go b/mcp/mcp_test.go index 5df8b38..1417323 100644 --- a/mcp/mcp_test.go +++ b/mcp/mcp_test.go @@ -25,6 +25,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/jsonschema-go/jsonschema" + "github.com/modelcontextprotocol/go-sdk/internal/jsonrpc2" "github.com/modelcontextprotocol/go-sdk/jsonrpc" ) @@ -938,6 +939,61 @@ func TestKeepAlive(t *testing.T) { }) } +func TestKeepAliveMethodNotFound(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + ctx := context.Background() + + ct, st := NewInMemoryTransports() + + // Server that rejects ping with method-not-found, simulating a + // server that does not implement the optional ping method. + s := NewServer(testImpl, nil) + AddTool(s, greetTool(), sayHi) + s.AddReceivingMiddleware(func(next MethodHandler) MethodHandler { + return func(ctx context.Context, method string, req Request) (Result, error) { + if method == "ping" { + return nil, jsonrpc2.ErrMethodNotFound + } + return next(ctx, method, req) + } + }) + ss, err := s.Connect(ctx, st, nil) + if err != nil { + t.Fatal(err) + } + defer ss.Close() + + clientOpts := &ClientOptions{ + KeepAlive: 50 * time.Millisecond, + } + c := NewClient(testImpl, clientOpts) + cs, err := c.Connect(ctx, ct, nil) + if err != nil { + t.Fatal(err) + } + defer cs.Close() + + // Advance past several keepalive cycles. + time.Sleep(200 * time.Millisecond) + + // The session should still be alive despite the server not + // supporting ping. + result, err := cs.CallTool(ctx, &CallToolParams{ + Name: "greet", + Arguments: map[string]any{"Name": "user"}, + }) + if err != nil { + t.Fatalf("call failed after keepalive with method-not-found: %v", err) + } + if len(result.Content) == 0 { + t.Fatal("expected content in result") + } + if textContent, ok := result.Content[0].(*TextContent); !ok || textContent.Text != "hi user" { + t.Fatalf("unexpected result: %v", result.Content[0]) + } + }) +} + func TestElicitationUnsupportedMethod(t *testing.T) { ctx := context.Background() ct, st := NewInMemoryTransports() diff --git a/mcp/shared.go b/mcp/shared.go index 05397a1..cc28de8 100644 --- a/mcp/shared.go +++ b/mcp/shared.go @@ -14,6 +14,7 @@ package mcp import ( "context" "encoding/json" + "errors" "fmt" "log/slog" "net/http" @@ -605,6 +606,10 @@ func startKeepalive(session keepaliveSession, interval time.Duration, cancelPtr err := session.Ping(pingCtx, nil) pingCancel() if err != nil { + if errors.Is(err, jsonrpc2.ErrMethodNotFound) { + // Peer doesn't support ping, stop the keepalive process. + return + } // Ping failed; log it before closing the session so the // failure is observable to operators. See #218. logger.Error("keepalive ping failed; closing session", "error", err)