Repository navigation
mcp: wrap ErrConnectionClosed for notifications and interrupted calls - #1449
Conversation
Client.Connect documents that when the server closes the connection, calls or notifications return an error wrapping ErrConnectionClosed. Notifications returned the jsonrpc2 closing error unchanged, and calls interrupted by peer termination were retired with the raw read error, so errors.Is(err, mcp.ErrConnectionClosed) was false in both cases. Translate notification closing errors as calls already do, and mark calls retired by the reader so the MCP layer adds ErrConnectionClosed while keeping the underlying cause in the chain. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TTGAv6fe859ebbCLLfLHP
| return nil | ||
| case errors.Is(err, jsonrpc2.ErrClientClosing), errors.Is(err, jsonrpc2.ErrServerClosing): | ||
| return fmt.Errorf("%w: calling %q: %v", ErrConnectionClosed, method, err) | ||
| return fmt.Errorf("%w: calling %q: %w", ErrConnectionClosed, method, err) |
There was a problem hiding this comment.
I agree with exposing the original error that broke the connection (e.g. the read error in readIncoming), but we should not expose ErrClientClosing and ErrServerClosing: they are internal errors, and with %w their codes (-32003/-32004) leak to callers.
Please strip them from the returned error, both in call() (mcp/transport.go) and in the notification path (mcp/shared.go), so it wraps only ErrConnectionClosed and the root cause.
There was a problem hiding this comment.
Done in 1895f49. The closing-error translations in call() and the notification path now strip ErrClientClosing and ErrServerClosing, preserving ErrConnectionClosed and any remaining cause. Tests check cause preservation and removal of the internal markers and codes with errors.Is and errors.As.
Calls and notifications on a closed connection now return an error that wraps ErrConnectionClosed and the cause that broke the connection, such as the read error, but not jsonrpc2.ErrClientClosing or ErrServerClosing: those are internal, and wrapping them exposed their codes (-32003/-32004) to callers through errors.As. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019TTGAv6fe859ebbCLLfLHP
Fixes #1448
Client.Connectpromises that calls and notifications wrapErrConnectionClosedwhen the server closes the connection. Notifications after observed closure returned only the internal closing error, while calls interrupted by reader termination returned the raw read error.Translate notification closing errors and mark outstanding calls when the reader retires them. Preserve the underlying cause when adding
ErrConnectionClosed, including for subsequent calls on the closed connection. Classification uses the closing marker, so EOF returned while decoding a result is not mistaken for connection termination.Both closing-error translations now remove the internal
jsonrpc2.ErrClientClosingandErrServerClosingmarkers before addingErrConnectionClosed, preserving any remaining causes. Their codes (-32003/-32004) are therefore absent from these translated errors. With EOF as the cause, a call readsconnection closed: calling "tools/call": EOF, and a notification readsconnection closed: EOF. With no remaining cause, the call retains its method name and the notification returnsErrConnectionCloseddirectly.Tests
For the first commit, I ran
go build ./...,go test -race ./internal/jsonrpc2 ./mcp,go test ./..., andgo vet ./internal/jsonrpc2 ./mcpwith Go 1.27.3 on Ubuntu 22.04.2 LTS. All passed. I rangofmt -lon all changed Go files; it reported no files.The first regression run with the new tests and base production code failed as expected. Tests cover notifications after closure, calls interrupted by a stdio peer exit, the subsequent call, underlying error preservation, and controls for cancellation, ordinary JSON-RPC errors, and result-decoding errors.
The second commit adds assertions for calls and notifications that preserve matching of
ErrConnectionClosedand any remaining causes, reject both internal closing markers witherrors.Is, and reject closing codes exposed througherrors.As. Cases include local close, EOF, read errors, and wrapped and joined closing errors.At
1895f49, with Go 1.27.3 on Ubuntu 24.04.5 LTS,go build ./...,go test -race ./mcp,go test ./...,go vet ./mcp, and the formatting check passed. The regression run with the new tests and the first commit's production code failed as expected.The change was drafted with AI assistance (OpenAI Codex, reviewed with Claude Code).
🤖 Generated with Claude Code