fix(core): avoid retrying failing MCP handlers

dev-packages/node-integration-tests/suites/tracing/mcp-handler-exact-once/instrument.mjs

@@ -0,0 +1,8 @@
+import * as Sentry from '@sentry/node';
+import { loggingTransport } from '@sentry-internal/node-integration-tests';
+
+Sentry.init({
+  dsn: 'https://public@dsn.ingest.sentry.io/1337',
+  tracesSampleRate: 1,
+  transport: loggingTransport,
+});

dev-packages/node-integration-tests/suites/tracing/mcp-handler-exact-once/run.cjs

@@ -0,0 +1,52 @@
+const assert = require('node:assert/strict');
+
+module.exports = async function run({ Client, InMemoryTransport, McpServer, Sentry }) {
+  await Sentry.startSpan({ name: 'handler-regression' }, async span => {
+    const calls = { tool: 0, resource: 0, prompt: 0, existingResource: 0 };
+    const failOnce = (name, result) => () => {
+      calls[name] += 1;
+      if (calls[name] === 1) {
+        throw new Error(`${name} failed`);
+      }
+      return result;
+    };
+    const server = new McpServer({ name: 'handler-test-server', version: '1.0.0' });
+    server.registerResource(
+      'existingResource',
+      'test://existing-resource',
+      {},
+      failOnce('existingResource', { contents: [{ uri: 'test://existing-resource', text: 'unexpected retry' }] }),
+    );
+    Sentry.wrapMcpServerWithSentry(server);
+    server.registerTool('tool', {}, failOnce('tool', { content: [{ type: 'text', text: 'unexpected retry' }] }));
+    server.registerResource(
+      'resource',
+      'test://resource',
+      {},
+      failOnce('resource', { contents: [{ uri: 'test://resource', text: 'unexpected retry' }] }),
+    );
+    server.registerPrompt(
+      'prompt',
+      {},
+      failOnce('prompt', { messages: [{ role: 'user', content: { type: 'text', text: 'unexpected retry' } }] }),
+    );
+    const client = new Client({ name: 'handler-test-client', version: '1.0.0' });
+    const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair();
+
+    try {
+      await Promise.all([server.connect(serverTransport), client.connect(clientTransport)]);
+      assert.deepEqual(await client.callTool({ name: 'tool', arguments: {} }), {
+        content: [{ type: 'text', text: 'tool failed' }],
+        isError: true,
+      });
+      await assert.rejects(client.readResource({ uri: 'test://resource' }), /resource failed$/);
+      await assert.rejects(client.getPrompt({ name: 'prompt' }), /prompt failed$/);
+      await assert.rejects(client.readResource({ uri: 'test://existing-resource' }), /existingResource failed$/);
+      assert.deepEqual(calls, { tool: 1, resource: 1, prompt: 1, existingResource: 1 });
+    } finally {
+      await client.close();
+      await server.close();
+    }
+    span.setAttribute('test.mcp.handlers_verified', 4);
+  });
+};

dev-packages/node-integration-tests/suites/tracing/mcp-handler-exact-once/scenario-sdk-v1.mjs

@@ -0,0 +1,7 @@
+import { Client } from '@modelcontextprotocol/sdk/client/index.js';
+import { InMemoryTransport } from '@modelcontextprotocol/sdk/inMemory.js';
+import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
+import * as Sentry from '@sentry/node';
+import run from './run.cjs';
+
+run({ Client, InMemoryTransport, McpServer, Sentry });

dev-packages/node-integration-tests/suites/tracing/mcp-handler-exact-once/scenario-sdk-v2.mjs

@@ -0,0 +1,6 @@
+import { Client } from '@modelcontextprotocol/client';
+import { InMemoryTransport, McpServer } from '@modelcontextprotocol/server';
+import * as Sentry from '@sentry/node';
+import run from './run.cjs';
+
+run({ Client, InMemoryTransport, McpServer, Sentry });

dev-packages/node-integration-tests/suites/tracing/mcp-handler-exact-once/test.ts

@@ -0,0 +1,42 @@
+import type { SerializedStreamedSpanContainer } from '@sentry/core';
+import { describe, expect } from 'vitest';
+import { createEsmAndCjsTests } from '../../../utils/runner';
+
+describe.each(['v1', 'v2'])('MCP TypeScript SDK %s', sdk => {
+  createEsmAndCjsTests(
+    __dirname,
+    `scenario-sdk-${sdk}.mjs`,
+    'instrument.mjs',
+    (createTestRunner, test) => {
+      test('preserves handler errors without repeating their side effects', async () => {
+        let root: SerializedStreamedSpanContainer['items'][number] | undefined;
+
+        await createTestRunner()
+          .unordered()
+          .expect({
+            event: event => {
+              expect(event.exception?.values).toHaveLength(1);
+              expect(event.exception?.values?.[0]?.value).toBe('tool failed');
+              expect(event.exception?.values?.[0]?.mechanism?.type).toBe('auto.ai.mcp_server');
+            },
+          })
+          .expect({
+            span: container => {
+              const segment = container.items.find(item => item.is_segment && item.name === 'handler-regression');
+              expect(segment?.name).toBe('handler-regression');
+              root = segment;
+            },
+          })
+          .start()
+          .completed();
+
+        expect(root?.status).toBe('ok');
+        expect(root?.attributes['test.mcp.handlers_verified']).toEqual({ type: 'integer', value: 4 });
+      });
+    },
+    {
+      additionalDependencies: sdk === 'v1' ? { '@modelcontextprotocol/sdk': '1.30.0' } : undefined,
+      copyPaths: ['run.cjs'],
+    },
+  );
+});

packages/core/src/integrations/mcp-server/handlers.ts

@@ -5,8 +5,6 @@
  * and prompt handlers.
  */
 
-import { DEBUG_BUILD } from '../../debug-build';
-import { debug } from '../../utils/debug-logger';
 import { isObjectLike } from '../../utils/is';
 import { fill } from '../../utils/object';
 import { captureError } from './errorCapture';
@@ -44,46 +42,21 @@ function wrapMethodHandler(serverInstance: MCPServerInstance, methodName: keyof
 function createWrappedHandler(originalHandler: MCPHandler, methodName: keyof MCPServerInstance, handlerName: string) {
   return function (this: unknown, ...handlerArgs: unknown[]): unknown {
     try {
-      return createErrorCapturingHandler.call(this, originalHandler, methodName, handlerName, handlerArgs);
-    } catch (error) {
-      DEBUG_BUILD && debug.warn('MCP handler wrapping failed:', error);
-      return originalHandler.apply(this, handlerArgs);
-    }
-  };
-}
+      const result = originalHandler.apply(this, handlerArgs);
 
-/**
- * Creates an error-capturing wrapper for handler execution
- * @internal
- * @param originalHandler - Original handler function
- * @param methodName - MCP method name
- * @param handlerName - Handler identifier
- * @param handlerArgs - Handler arguments
- * @param extraHandlerData - Additional handler context
- * @returns Handler execution result
- */
-function createErrorCapturingHandler(
-  this: MCPServerInstance,
-  originalHandler: MCPHandler,
-  methodName: keyof MCPServerInstance,
-  handlerName: string,
-  handlerArgs: unknown[],
-): unknown {
-  try {
-    const result = originalHandler.apply(this, handlerArgs);
+      if (isObjectLike(result) && typeof (result as { then?: unknown }).then === 'function') {
+        return Promise.resolve(result).catch(error => {
+          captureHandlerError(error, methodName, handlerName);
+          throw error;
+        });
+      }
 
-    if (isObjectLike(result) && typeof (result as { then?: unknown }).then === 'function') {
-      return Promise.resolve(result).catch(error => {
-        captureHandlerError(error, methodName, handlerName);
-        throw error;
-      });
+      return result;
+    } catch (error) {
+      captureHandlerError(error as Error, methodName, handlerName);
+      throw error;
     }
-
-    return result;
-  } catch (error) {
-    captureHandlerError(error as Error, methodName, handlerName);
-    throw error;
-  }
+  };
 }
 
 /**

packages/core/test/lib/integrations/mcp-server/mcpServerErrorCapture.test.ts

@@ -3,6 +3,7 @@ import * as currentScopes from '../../../../src/currentScopes';
 import * as exports from '../../../../src/exports';
 import { wrapMcpServerWithSentry } from '../../../../src/integrations/mcp-server';
 import { captureError } from '../../../../src/integrations/mcp-server/errorCapture';
+import type { MCPHandler } from '../../../../src/integrations/mcp-server/types';
 import { createMockClient, createMockMcpServer } from './testUtils';
 
 describe('MCP Server Error Capture', () => {
@@ -145,41 +146,86 @@ describe('MCP Server Error Capture', () => {
   });
 
   describe('Error Capture Integration', () => {
-    let mockMcpServer: ReturnType<typeof createMockMcpServer>;
     let wrappedMcpServer: ReturnType<typeof createMockMcpServer>;
+    let registeredHandler: MCPHandler;
 
     beforeEach(() => {
-      mockMcpServer = createMockMcpServer();
+      captureExceptionSpy.mockReturnValue('event-id');
+      const mockMcpServer = createMockMcpServer();
+      mockMcpServer.tool.mockImplementation((_name: string, handler: MCPHandler) => {
+        registeredHandler = handler;
+      });
       wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer);
     });
 
-    it('should capture tool execution errors and continue normal flow', async () => {
+    it('should not retry a handler after a synchronous error', () => {
       const toolError = new Error('Tool execution failed');
-      const mockToolHandler = vi.fn().mockRejectedValue(toolError);
+      const mockToolHandler = vi
+        .fn()
+        .mockImplementationOnce(() => {
+          throw toolError;
+        })
+        .mockReturnValue({ content: [] });
+      wrappedMcpServer.tool('failing-tool', mockToolHandler);
+
+      expect(() => registeredHandler()).toThrow(toolError);
+
+      expect(mockToolHandler).toHaveBeenCalledTimes(1);
+      expect(captureExceptionSpy).toHaveBeenCalledExactlyOnceWith(toolError, {
+        mechanism: {
+          type: 'auto.ai.mcp_server',
+          handled: false,
+          data: { error_type: 'tool_execution', tool_name: 'failing-tool' },
+        },
+      });
+    });
 
+    it('should capture and rethrow asynchronous errors without retrying', async () => {
+      const toolError = new Error('Tool execution failed');
+      const mockToolHandler = vi.fn().mockRejectedValue(toolError);
       wrappedMcpServer.tool('failing-tool', mockToolHandler);
 
-      await expect(mockToolHandler({ input: 'test' }, { requestId: 'req-123', sessionId: 'sess-456' })).rejects.toThrow(
-        'Tool execution failed',
-      );
+      await expect(registeredHandler()).rejects.toBe(toolError);
 
-      // The capture should be set up correctly
-      expect(captureExceptionSpy).toHaveBeenCalledTimes(0); // No capture yet since we didn't call the wrapped handler
+      expect(mockToolHandler).toHaveBeenCalledTimes(1);
+      expect(captureExceptionSpy).toHaveBeenCalledExactlyOnceWith(toolError, {
+        mechanism: {
+          type: 'auto.ai.mcp_server',
+          handled: false,
+          data: { error_type: 'tool_execution', tool_name: 'failing-tool' },
+        },
+      });
     });
 
-    it('should handle Sentry capture errors gracefully', async () => {
+    it('should not retry a failing handler when Sentry capture also throws', () => {
       captureExceptionSpy.mockImplementation(() => {
         throw new Error('Sentry error');
       });
-
-      // Test that the capture function itself doesn't throw
       const toolError = new Error('Tool execution failed');
-      const mockToolHandler = vi.fn().mockRejectedValue(toolError);
-
+      const mockToolHandler = vi.fn(() => {
+        throw toolError;
+      });
       wrappedMcpServer.tool('failing-tool', mockToolHandler);
 
-      // The error capture should be resilient to Sentry errors
-      expect(captureExceptionSpy).toHaveBeenCalledTimes(0);
+      expect(() => registeredHandler()).toThrow(toolError);
+
+      expect(mockToolHandler).toHaveBeenCalledTimes(1);
+      expect(captureExceptionSpy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should preserve the handler receiver, arguments, and return value', () => {
+      const result = { content: [] };
+      const mockToolHandler = vi.fn().mockReturnValue(result);
+      const receiver = {};
+      const args = { input: 'test' };
+      const extra = { requestId: 'req-123', sessionId: 'sess-456' };
+      wrappedMcpServer.tool('successful-tool', mockToolHandler);
+
+      expect(registeredHandler.call(receiver, args, extra)).toBe(result);
+
+      expect(mockToolHandler).toHaveBeenCalledExactlyOnceWith(args, extra);
+      expect(mockToolHandler.mock.contexts).toEqual([receiver]);
+      expect(captureExceptionSpy).not.toHaveBeenCalled();
     });
   });
 });