Repository navigation
Conversation
An error thrown by a listChanged onChanged callback escaped as an unhandled promise rejection. With autoRefresh the callback was also called a second time, with its own error passed as a failed refresh. Catch the error where the callback is invoked and report it on Client.onerror, so the callback runs once per notification.
🦋 Changeset detectedLatest commit: 61699e6 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.
Fixes #3175.
Motivation and Context
An error thrown by a
ClientOptions.listChangedonChangedcallback never reachesclient.onerror._setupListChangedHandlerstartsrefresh()from asetTimeout, or calls it without returning it, so the rejection escapes as an unhandled promise rejection. In a plain Node script that ends the process with exit code 1.With
autoRefresh(the default) the callback also runs inside thetryaround the list fetch, so it is called a second time with its own error as if the refresh had failed. The docs example would log that asRefresh failed:.This wraps the callback where it is invoked. A throw is reported once on
onerrorasUncaught error in <list type> listChanged callback: <error>, following the wordingProtocoluses for notification handlers, and thecatcharound the fetch now only sees fetch errors.How Has This Been Tested?
listChangedtests intest/integration/test/client/client.test.ts:autoRefreshwithout debounce, and manual refresh with debounce. Both fail onmainand pass here.nodeagainst the built packages: exit code 1 before, clean exit with oneclient.onerrorcall after.pnpm build:all,pnpm -r typecheck,pnpm -r lint, the snippet and skills checks, the dist type smoke test and client conformance pass. Unit and integration suites pass for every SDK package (test/integration: 469 passed). In my sandboxcloudflareWorkers.test.ts, theexamples/sharedtests and seven e2e scenarios (six stdio, one SSE timeout) cannot run, and they fail the same way onmainthere. Not run locally:docs:check, server conformance, the examples run, Bun and Deno.Breaking Changes
None. A callback that throws used to be called twice and then end the process through the unhandled rejection. It is now called once and the error goes to
onerror.Types of changes
Checklist
Additional context
The same code is on
v1.x(src/client/index.ts). I can send a[v1.x]backport after this one if you want it.Only a synchronous throw is handled. An
asynccallback that rejects stays the caller's promise, sinceListChangedCallbackreturnsvoid.AI disclosure: the investigation, the change, the tests and this description were written primarily by Claude Code, an AI coding assistant, working at my direction on my machine. I have read the diff. The checks listed above were run locally there.