| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
A backpressured `StdioClientTransport.send()` waited for a `'drain'` event, but a pipe destroyed by the server process exiting never drains, so the promise stayed pending for the lifetime of the process and the `'drain'` listener leaked. Settle from the `write()` callback instead, which Node invokes on flush or on failure, so the send rejects with the underlying write error. This is what `StdioServerTransport.send()` already does for its own stdout.
🦋 Changeset detectedLatest commit: 25eab69 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
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 |
Sorry, something went wrong.
|
@modelcontextprotocol/client
npm i https://pkg.pr.new/@modelcontextprotocol/client@2552
npm i https://pkg.pr.new/@modelcontextprotocol/codemod@2552
npm i https://pkg.pr.new/@modelcontextprotocol/core@2552
npm i https://pkg.pr.new/@modelcontextprotocol/server@2552
npm i https://pkg.pr.new/@modelcontextprotocol/server-legacy@2552
npm i https://pkg.pr.new/@modelcontextprotocol/express@2552
npm i https://pkg.pr.new/@modelcontextprotocol/fastify@2552
npm i https://pkg.pr.new/@modelcontextprotocol/hono@2552
npm i https://pkg.pr.new/@modelcontextprotocol/node@2552 commit: 25eab69 |
Sorry, something went wrong.
|
Now that 2.0.0 is out, this sits in a released package rather than in a prerelease. @modelcontextprotocol/client@2.0.0 ships the 'drain'-only send at dist/stdio.cjs:208, and the v1 line has the same shape in @modelcontextprotocol/sdk@1.30.0 at dist/cjs/client/stdio.js:196. Nothing has changed on my side since I opened this. The branch is seven commits behind main but still mergeable, packages/client/src/client/stdio.ts has not been touched since #2514, and the changeset is a patch on the client. I re-checked against main at cc4b416: without the change the regression test still reports the hang, with it the client suite is green (33 files, 798 tests). |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
StdioClientTransport.send() now settles from the write() callback, so a backpressured send can't stay pending forever when the pipe to the server dies.
Motivation and Context
I noticed this while reading the two stdio transports side by side. The server one rejects a failed write, the client one has no error path at all: its promise executor only takes resolve, and a backpressured write resolves on 'drain'. If the server process then exits, or close() escalates to SIGTERM and SIGKILL, the stream is destroyed, and a destroyed stream never drains. The promise just stays pending, and the 'drain' listener is never removed.
The symptom is that await client.notification(...) never comes back. The notification path awaits the send with no timeout, and the connection-closed teardown settles pending responses but not pending sends, so nothing rescues it. I tried it against a server that answers initialize, stops reading stdin and exits, with 8 MB in flight: the await is still pending after 8 seconds, and with this change it rejects in 0.4 seconds. Requests were already covered by the teardown, so for them the only difference is that they now report the actual EPIPE instead of a generic connection-closed error.
You need a write the pipe won't take in one go to get there, which on Linux is between 160 KB and 224 KB depending on the Node version. Base64 image payloads and file contents in tool arguments reach that routinely.
The server transport has rejected on a write failure since #1568, with a test pinning its listener cleanup, so this brings the client side in line. The fast path stays as it was, since a write() that returns true still resolves right away and only the failure path differs.
How Has This Been Tested?
Breaking Changes
None. send() already returns Promise<void> and the sibling stdio transport rejects it on a write failure, so callers that awaited it keep working.
v1.x has the same pattern in src/client/stdio.ts, and in src/server/stdio.ts too since #1568 only landed on main. Happy to send that separately if you want it.
Types of changes
Checklist