Skip to content

fix(node): reuse getRequestListener instead of creating one per request#2091

Open
RomKadria wants to merge 2 commits into
modelcontextprotocol:mainfrom
RomKadria:fix/reuse-request-listener
Open

fix(node): reuse getRequestListener instead of creating one per request#2091
RomKadria wants to merge 2 commits into
modelcontextprotocol:mainfrom
RomKadria:fix/reuse-request-listener

Conversation

@RomKadria
Copy link
Copy Markdown

Problem

NodeStreamableHTTPServerTransport.handleRequest() creates a new getRequestListener from @hono/node-server on every call (line 173), even though the constructor already creates one (this._requestListener, line 80).

The constructor's listener was intended to be reusable — it uses a WeakMap<Request, ...> to pass per-request context. But this approach has a fundamental flaw: the Web Standard Request object is created inside getRequestListener by Hono, so the WeakMap key can never be set before the callback fires. The handleRequest method works around this by creating a new listener per call that closes over authInfo and parsedBody directly.

This means every request allocates a new getRequestListener with its internal conversion infrastructure, contributing to GC pressure under sustained load.

Fix

Replace the WeakMap with AsyncLocalStorage to pass per-request context (authInfo, parsedBody) through to the shared listener callback. This is:

  • Concurrent-safe — each request gets its own async context
  • Appropriate — this is the Node.js-specific middleware package (packages/middleware/node)
  • Zero per-request allocations — the single getRequestListener from the constructor is reused for all requests

Before

async handleRequest(req, res, parsedBody) {
    const handler = getRequestListener(async (webRequest) => {  // NEW per request
        return this._webStandardTransport.handleRequest(webRequest, { authInfo, parsedBody });
    }, { overrideGlobalObjects: false });
    await handler(req, res);
}

After

async handleRequest(req, res, parsedBody) {
    await this._requestContext.run({ authInfo: req.auth, parsedBody }, () => {
        return this._requestListener(req, res);  // reuses constructor's listener
    });
}

Test results

All 74 tests pass (packages/middleware/node).

Relates to #2090

NodeStreamableHTTPServerTransport.handleRequest() creates a new
getRequestListener from @hono/node-server on every call, even though
the constructor already creates one (this._requestListener). The
constructor's version was unusable because it relied on a WeakMap
keyed by the Web Standard Request object — but that object is created
inside getRequestListener, so the WeakMap key can never be set before
the callback fires.

Fix: replace the WeakMap with AsyncLocalStorage to pass per-request
context (authInfo, parsedBody) through to the shared listener callback.
This is concurrent-safe and appropriate for this Node.js-specific
middleware package.

Impact: eliminates one getRequestListener allocation + closure per
HTTP request. In production MCP servers handling sustained traffic,
this reduces GC pressure from per-request overhead.

Relates to modelcontextprotocol#2090
@RomKadria RomKadria requested a review from a team as a code owner May 14, 2026 17:20
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 14, 2026

🦋 Changeset detected

Latest commit: a303cc7

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

This PR includes changesets to release 1 package
Name Type
@modelcontextprotocol/node 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
Copy link
Copy Markdown

pkg-pr-new Bot commented May 14, 2026

Open in StackBlitz

@modelcontextprotocol/client

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

@modelcontextprotocol/server

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

@modelcontextprotocol/express

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

@modelcontextprotocol/fastify

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

@modelcontextprotocol/hono

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

@modelcontextprotocol/node

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

commit: a303cc7

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