fix(mcp): retroactively wrap handlers registered before wrapMcpServerWithSentry#20699
Draft
fix(mcp): retroactively wrap handlers registered before wrapMcpServerWithSentry#20699
Conversation
betegon
force-pushed
the
fix/mcp-retroactive-handler-wrapping
branch
from
c1e7151 to
365c47a
Compare
…WithSentry wrapMcpServerWithSentry patches the registration methods (registerTool etc.) so only handlers registered after wrapping get instrumented. When wrapping happens after registration, the already-stored executors/callbacks are never touched, silently skipping error capture for those handlers. Fix by scanning the McpServer's internal registries (_registeredTools, _registeredResources, _registeredResourceTemplates, _registeredPrompts) after patching the registration methods, and wrapping the stored callables in-place: - tools: wrap `executor` (what the SDK calls on tools/call) - resources/templates: wrap `readCallback` - prompts: wrap `handler` Both call orderings are now fully supported. JSDoc updated to reflect this and reference the upstream SDK source line where executor is invoked. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
betegon
force-pushed
the
fix/mcp-retroactive-handler-wrapping
branch
from
365c47a to
8b9fd2c
Compare
Contributor
size-limit report 📦
|
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
1 participant
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.
wrapMcpServerWithSentryworks by patching the registration methods (registerTool,registerResource,registerPrompt) so that any handler passed to them gets wrapped with Sentry error capture. The problem: if tools are registered before wrapping, those already-stored handlers are never touched, so errors from them are silently dropped.This fix adds a retroactive pass (
wrapExistingHandlers) that runs immediately after the registration methods are patched. It walks the McpServer's internal registries (_registeredTools,_registeredResources,_registeredResourceTemplates,_registeredPrompts) and wraps the stored callables in-place —executorfor tools,readCallbackfor resources and templates,handlerfor prompts. These are the properties the MCP SDK reads at call time (not captured by closure at registration), so replacing them is equivalent to having wrapped the original call.Both orderings are now fully supported. Wrapping at construction is still recommended for consistency with how other SDK integrations work, but it's not required.
Recommended (wrap at construction):
Also works (wrap after registration):
The retroactive wrapping intentionally accesses private MCP SDK internals. The JSDoc on
wrapExistingHandlersincludes a pinned upstream source link and a note to re-verify the internal shapes when upgrading the MCP SDK — all access is defensive and skips silently if a property isn't found.Checklist
Retroactive handler wrappingdescribe block inmcpServerWrapper.test.ts)Closes #issue_link_here