Skip to content

fix(core): carry the transport's last error into the Connection closed rejection - #2792

Open
BerkantACUN wants to merge 1 commit into
modelcontextprotocol:mainfrom
BerkantACUN:fix/close-reason-carries-transport-error
Open

fix(core): carry the transport's last error into the Connection closed rejection#2792
BerkantACUN wants to merge 1 commit into
modelcontextprotocol:mainfrom
BerkantACUN:fix/close-reason-carries-transport-error

Conversation

@BerkantACUN

@BerkantACUN BerkantACUN commented Sep 11, 2026

Copy link
Copy Markdown

Draft only because non-collaborators may have one non-draft PR open at a time on this repository; the change is complete and ready for review.

When a transport reports why it is closing and then closes, every pending request was rejected with a bare SdkError('Connection closed') and the reason went only to onerror. Protocol now keeps the most recent transport error and settles pending requests with Connection closed: <reason> and that error as cause.

Motivation and Context

First point of #2775. The stdio transport already knows exactly what went wrong — ReadBuffer exceeded maximum size of 10485760 bytes names the cause and implies the fix (maxBufferSize) — but it says so on onerror and then closes, and the awaiting caller gets:

[onerror] ReadBuffer exceeded maximum size of 10485760 bytes
[caller ] MCP error -32000: Connection closed

onerror is a side channel that code doing the ordinary thing (await client.listTools()) never sees unless it knew in advance to wire it up. Same shape in #1049 from a different cause. With this change the same repro gives:

[onerror] ReadBuffer exceeded maximum size of 10485760 bytes
[caller ] Connection closed: ReadBuffer exceeded maximum size of 10485760 bytes

and error.cause is the transport's Error, so loggers that walk the cause chain get it too.

How it works

  • Protocol.connect records the error in its transport.onerror wrapper, and forgets it in its transport.onmessage wrapper: a message delivered after an error means the transport recovered from it, so it is not why the connection closed. Without that, an SSE reconnect error from an hour ago would be blamed for an unrelated close.
  • _onclose builds the rejection through a new protected _connectionClosedError(), and Client._onclose uses the same helper for its listen() state so both settle with one reason instead of one with and one without.
  • Protocol.close() clears the retained error first: a close the caller asked for has no transport-reported reason, whatever the transport complained about earlier.
  • The message keeps the Connection closed prefix and SdkErrorCode.ConnectionClosed is unchanged; nothing in the repo matches the message exactly (checked packages/, test/, examples/, docs/). No transport changes and no public API change beyond the error message and cause.

The other two points of #2775 are separate: the ReadBuffer resync is #2793, and @errmakov has the BOM case in #2790.

How Has This Been Tested?

Five Protocol unit tests with the mock transport (reason carried; plain fallback; forgotten after a later message; cleared on caller-initiated close(); not carried across connect() to a second transport) and one end-to-end through the real StdioClientTransport with maxBufferSize: 1024 against a server whose tools/list result is larger — the repro from the issue, scaled down. The positive cases fail on main before the change (Connection closed with no cause). core-internal, client and server suites pass; pnpm typecheck:all and pnpm lint:all pass.

I also ran the issue's original 20 MB repro against this branch: one onerror, and the caller's rejection reads Connection closed: ReadBuffer exceeded maximum size of 10485760 bytes.

Breaking Changes

None. The message gains a suffix when a reason is known; the code and the prefix are unchanged.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Disclosure: written with Claude Code; I reviewed the diff and ran the repro and the suites myself.

…d rejection

When a transport reports why it is closing and then closes, pending
requests were rejected with a bare `Connection closed` and the reason
went only to `onerror`. Protocol now keeps the most recent transport
error since the last delivered message and settles pending requests
with `Connection closed: <reason>` and that error as `cause`. Plain
`Connection closed` stays for a close with no reported reason, a close
the caller asked for, and an error the transport recovered from.

First point of modelcontextprotocol#2775.
@changeset-bot

changeset-bot Bot commented Sep 11, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9c35605

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/core-internal Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Sep 11, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/@modelcontextprotocol/client@2792

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2792

@modelcontextprotocol/core

npm i https://pkg.pr.new/@modelcontextprotocol/core@2792

@modelcontextprotocol/server

npm i https://pkg.pr.new/@modelcontextprotocol/server@2792

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2792

@modelcontextprotocol/express

npm i https://pkg.pr.new/@modelcontextprotocol/express@2792

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2792

@modelcontextprotocol/hono

npm i https://pkg.pr.new/@modelcontextprotocol/hono@2792

@modelcontextprotocol/node

npm i https://pkg.pr.new/@modelcontextprotocol/node@2792

commit: 9c35605

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant