Skip to content

test(@modelcontextprotocol/node): cover pre-read body pattern in stateless mode#2026

Open
Zelys-DFKH wants to merge 3 commits into
modelcontextprotocol:mainfrom
Zelys-DFKH:test/stateless-pre-read-body
Open

test(@modelcontextprotocol/node): cover pre-read body pattern in stateless mode#2026
Zelys-DFKH wants to merge 3 commits into
modelcontextprotocol:mainfrom
Zelys-DFKH:test/stateless-pre-read-body

Conversation

@Zelys-DFKH
Copy link
Copy Markdown

Closes #1994 (partial; #1995 handles the v1.x production fix)

The handleRequest(req, res, parsedBody) overload exists for body-parser users: drain the stream first, then pass the parsed body. It's documented in the JSDoc with an Express example. The stateless describe block had no test covering that pattern, so it was possible to break it without anyone noticing.

This adds one. An initialize request followed by a tools/list request, both with the body pre-read via for await and handed off as parsedBody. The second request is what was missing.

I found this while digging into #1994. The v1.x issue was a real bug (_hasHandledRequest threw on the second request and produced an opaque 500). In v2 that restriction was removed entirely, so both requests succeed fine. This test makes sure it stays that way.

Test plan

pnpm --filter @modelcontextprotocol/node test  # 75/75
pnpm exec prettier --check test/streamableHttp.test.ts  # clean

@Zelys-DFKH Zelys-DFKH requested a review from a team as a code owner May 6, 2026 19:22
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 6, 2026

⚠️ No Changeset found

Latest commit: de450db

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 6, 2026

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@2026

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@2026

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@2026

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@2026

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@2026

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@2026

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@2026

commit: de450db

@Zelys-DFKH Zelys-DFKH force-pushed the test/stateless-pre-read-body branch from 9f10aeb to 39164fd Compare May 12, 2026 22:57
@Zelys-DFKH
Copy link
Copy Markdown
Author

The Node 20 failure is in test/integration/test/server/cloudflareWorkers.test.ts; our change only touches packages/middleware/node/test/streamableHttp.test.ts, a different package with no shared state.

The cloudflare workers test connects successfully (the initialize request goes through) then fails on the second request with Network connection lost.. Commit 5a2c8a6 added retry logic to this test for this same reason. Node 22 and Node 24 pass; main's Node 20 run from 2026-05-04 passed too.

Zelys-DFKH and others added 3 commits May 21, 2026 14:01
Wrangler on Node 20 accepts the initialize handshake before it can reliably
handle subsequent requests. The previous retry loop only wrapped connect(),
so a successful connect followed by a failed callTool() still caused the
test to fail. Extend the loop to retry the full interaction (connect +
callTool + assertion) and close the client on each failed attempt before
retrying.
…eless mode

Add a test for the handleRequest(req, res, parsedBody) overload in stateless
mode: drain the IncomingMessage body upstream (body-parser pattern), pass it
as parsedBody, and verify that both an initialize request and a subsequent
tools/list request succeed on the same reused transport.

Found while investigating modelcontextprotocol#1994. In v2 the _hasHandledRequest reuse
restriction was removed, so both requests succeed. This test pins that.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@Zelys-DFKH Zelys-DFKH force-pushed the test/stateless-pre-read-body branch from 39164fd to de450db Compare May 21, 2026 19:27
@Zelys-DFKH
Copy link
Copy Markdown
Author

The first two commits here are from #2079 (extend cloudflare workers retry to cover full request cycle). That fix is what prevents the Node 20 flake from showing up in this PR's CI. Once #2079 merges I'll rebase this down to one commit.

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.

Stateless StreamableHTTPServerTransport: non-initialize requests return 500 with empty body when transport is reused (regression vs 1.24.3)

1 participant