Repository navigation
fix(server): answer 499 to a legacy request aborted before subscribing - #3137
Open
mariano-aguero wants to merge 1 commit into
Open
mariano-aguero wants to merge 1 commit into
mariano-aguero wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 1c92678 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 |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
Fixes #3136.
The legacy stateless fallback of
createMcpHandlersubscribes torequest.signalonly afterawait factory()andawait product.connect(transport). Anabortthat fires during those awaits is never delivered to the listener added afterwards, so the request runs to completion,ctx.mcpReq.signalnever aborts, and the per-request pair is only torn down when the response ends. The modern path already handles this case:PerRequestHTTPServerTransport.handleMessagecheckssignal.abortedand the entry answers499.The fallback now checks
request.signal.abortedright after subscribing, tears the pair down and answers499with no body, as the modern path does, without dispatching the request.How Has This Been Tested?
legacyStatelessFallback.test.tsthat sends a request whose signal is already aborted. It asserts a499, that the product closes and that the tool never runs. On unmodifiedmainit fails withexpected 200 to be 499, and it passes with this change.pnpm build:all, thenpnpm --filter @modelcontextprotocol/server check(typecheck, eslint, prettier) andtest(675 tests) passed.test/e2epassed (3126 passed, 141 expected fail, unchanged).Breaking Changes
None. A request whose client already went away now gets
499instead of being run. No client is left to read the answer either way.Types of changes
Checklist