From 4d504dbbe1cd5edb09f1ab9c1f1e9788a89345fb Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 1 Apr 2026 20:42:38 +0200 Subject: [PATCH 1/2] fix(core): set span.status to error when MCP tool returns JSON-RPC error response When a tool handler threw an error, completeSpanWithResults() was ending the span without setting its status to error. This caused all MCP tool spans to appear with span.status=ok in Sentry, breaking the failure_rate() metric in the MCP insights dashboard. The fix passes hasError=true to completeSpanWithResults() when the outgoing JSON-RPC response contains an error object, setting the span status to internal_error directly on the stored span (bypassing getActiveSpan() which doesn't return the right span at send() time). Co-Authored-By: claude-sonnet-4-6 --- .../integrations/mcp-server/correlation.ts | 6 +- .../src/integrations/mcp-server/transport.ts | 2 +- .../transportInstrumentation.test.ts | 62 +++++++++++++++++++ 3 files changed, 68 insertions(+), 2 deletions(-) diff --git a/packages/core/src/integrations/mcp-server/correlation.ts b/packages/core/src/integrations/mcp-server/correlation.ts index b47f0f9b4a69..a831a822039c 100644 --- a/packages/core/src/integrations/mcp-server/correlation.ts +++ b/packages/core/src/integrations/mcp-server/correlation.ts @@ -81,19 +81,23 @@ export function storeSpanForRequest(transport: MCPTransport, requestId: RequestI * @param requestId - Request identifier * @param result - Execution result for attribute extraction * @param options - Resolved MCP options + * @param hasError - Whether the JSON-RPC response contained an error */ export function completeSpanWithResults( transport: MCPTransport, requestId: RequestId, result: unknown, options: ResolvedMcpOptions, + hasError = false, ): void { const spanMap = getOrCreateSpanMap(transport); const spanData = spanMap.get(requestId); if (spanData) { const { span, method } = spanData; - if (method === 'initialize') { + if (hasError) { + span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' }); + } else if (method === 'initialize') { const sessionData = extractSessionDataFromInitializeResponse(result); const serverAttributes = buildServerAttributesFromInfo(sessionData.serverInfo); diff --git a/packages/core/src/integrations/mcp-server/transport.ts b/packages/core/src/integrations/mcp-server/transport.ts index 5e4fd6e75c23..4b04a78f43b0 100644 --- a/packages/core/src/integrations/mcp-server/transport.ts +++ b/packages/core/src/integrations/mcp-server/transport.ts @@ -121,7 +121,7 @@ export function wrapTransportSend(transport: MCPTransport, options: ResolvedMcpO } } - completeSpanWithResults(this, message.id, message.result, options); + completeSpanWithResults(this, message.id, message.result, options, !!message.error); } } diff --git a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts index c720512cb97b..71590fd17712 100644 --- a/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts +++ b/packages/core/test/lib/integrations/mcp-server/transportInstrumentation.test.ts @@ -182,6 +182,68 @@ describe('MCP Server Transport Instrumentation', () => { // Trigger onclose - should not throw expect(() => mockTransport.onclose?.()).not.toThrow(); }); + + it('should set span status to error when JSON-RPC error response is sent', async () => { + const mockSpan = { + setAttributes: vi.fn(), + setStatus: vi.fn(), + end: vi.fn(), + isRecording: vi.fn().mockReturnValue(true), + }; + startInactiveSpanSpy.mockReturnValue(mockSpan as any); + + await wrappedMcpServer.connect(mockTransport); + + // Simulate an incoming tools/call request + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/call', + id: 'req-err-1', + params: { name: 'always-error' }, + }; + mockTransport.onmessage?.(jsonRpcRequest, {}); + + // Simulate the MCP SDK sending back a JSON-RPC error response + const jsonRpcErrorResponse = { + jsonrpc: '2.0', + id: 'req-err-1', + error: { code: -32603, message: 'Internal error: tool threw an exception' }, + }; + await mockTransport.send?.(jsonRpcErrorResponse as any); + + expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: 2, message: 'internal_error' }); + expect(mockSpan.end).toHaveBeenCalled(); + }); + + it('should not set error span status for successful JSON-RPC responses', async () => { + const mockSpan = { + setAttributes: vi.fn(), + setStatus: vi.fn(), + end: vi.fn(), + isRecording: vi.fn().mockReturnValue(true), + }; + startInactiveSpanSpy.mockReturnValue(mockSpan as any); + + await wrappedMcpServer.connect(mockTransport); + + const jsonRpcRequest = { + jsonrpc: '2.0', + method: 'tools/call', + id: 'req-ok-1', + params: { name: 'echo' }, + }; + mockTransport.onmessage?.(jsonRpcRequest, {}); + + const jsonRpcSuccessResponse = { + jsonrpc: '2.0', + id: 'req-ok-1', + result: { content: [{ type: 'text', text: 'hello' }] }, + }; + await mockTransport.send?.(jsonRpcSuccessResponse as any); + + expect(mockSpan.setStatus).not.toHaveBeenCalled(); + expect(mockSpan.end).toHaveBeenCalled(); + }); }); describe('Stdio Transport Tests', () => { From 34dd8e29becc97300f295027b965b5cfab6f492c Mon Sep 17 00:00:00 2001 From: betegon Date: Wed, 1 Apr 2026 22:07:56 +0200 Subject: [PATCH 2/2] test(e2e): add MCP error tool e2e test for span.status=internal_error Adds an `always-error` tool to the MCP e2e test apps and a test step that verifies the resulting transaction has span.status=internal_error when a tool throws. This covers the fix in the span status correlation path end-to-end, complementing the existing unit tests. Co-Authored-By: Claude Sonnet 4.6 --- .../node-express-v5/src/mcp.ts | 4 ++++ .../node-express-v5/tests/mcp.test.ts | 17 +++++++++++++++++ .../test-applications/node-express/src/mcp.ts | 4 ++++ .../node-express/tests/mcp.test.ts | 17 +++++++++++++++++ .../test-applications/tsx-express/src/mcp.ts | 4 ++++ .../tsx-express/tests/mcp.test.ts | 17 +++++++++++++++++ 6 files changed, 63 insertions(+) diff --git a/dev-packages/e2e-tests/test-applications/node-express-v5/src/mcp.ts b/dev-packages/e2e-tests/test-applications/node-express-v5/src/mcp.ts index a819858c8f37..f17de442d9ab 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-v5/src/mcp.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-v5/src/mcp.ts @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({ ], })); +server.tool('always-error', {}, async () => { + throw new Error('intentional error for span status testing'); +}); + const transports: Record = {}; mcpRouter.get('/sse', async (_, res) => { diff --git a/dev-packages/e2e-tests/test-applications/node-express-v5/tests/mcp.test.ts b/dev-packages/e2e-tests/test-applications/node-express-v5/tests/mcp.test.ts index 73dfc1d69432..cdb78220454b 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-v5/tests/mcp.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-v5/tests/mcp.test.ts @@ -120,6 +120,23 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => { // TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction }); + + await test.step('error tool sets span status to internal_error', async () => { + const toolTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => { + return transactionEvent.transaction === 'tools/call always-error'; + }); + + try { + await client.callTool({ name: 'always-error', arguments: {} }); + } catch { + // Expected: MCP SDK throws when the tool returns a JSON-RPC error + } + + const toolTransaction = await toolTransactionPromise; + expect(toolTransaction).toBeDefined(); + expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server'); + expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error'); + }); }); /** diff --git a/dev-packages/e2e-tests/test-applications/node-express/src/mcp.ts b/dev-packages/e2e-tests/test-applications/node-express/src/mcp.ts index 638462423d11..d576cfc6c495 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/src/mcp.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/src/mcp.ts @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({ ], })); +server.tool('always-error', {}, async () => { + throw new Error('intentional error for span status testing'); +}); + const transports: Record = {}; mcpRouter.get('/sse', async (_, res) => { diff --git a/dev-packages/e2e-tests/test-applications/node-express/tests/mcp.test.ts b/dev-packages/e2e-tests/test-applications/node-express/tests/mcp.test.ts index 143867c773e6..d746330f7c71 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/tests/mcp.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/tests/mcp.test.ts @@ -126,6 +126,23 @@ test('Should record transactions for mcp handlers', async ({ baseURL }) => { expect(promptTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('prompts/get'); // TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction }); + + await test.step('error tool sets span status to internal_error', async () => { + const toolTransactionPromise = waitForTransaction('node-express', transactionEvent => { + return transactionEvent.transaction === 'tools/call always-error'; + }); + + try { + await client.callTool({ name: 'always-error', arguments: {} }); + } catch { + // Expected: MCP SDK throws when the tool returns a JSON-RPC error + } + + const toolTransaction = await toolTransactionPromise; + expect(toolTransaction).toBeDefined(); + expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server'); + expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error'); + }); }); /** diff --git a/dev-packages/e2e-tests/test-applications/tsx-express/src/mcp.ts b/dev-packages/e2e-tests/test-applications/tsx-express/src/mcp.ts index b3b401ac294c..fec65bcf590d 100644 --- a/dev-packages/e2e-tests/test-applications/tsx-express/src/mcp.ts +++ b/dev-packages/e2e-tests/test-applications/tsx-express/src/mcp.ts @@ -47,6 +47,10 @@ server.prompt('echo', { message: z.string() }, ({ message }, extra) => ({ ], })); +server.tool('always-error', {}, async () => { + throw new Error('intentional error for span status testing'); +}); + const transports: Record = {}; mcpRouter.get('/sse', async (_, res) => { diff --git a/dev-packages/e2e-tests/test-applications/tsx-express/tests/mcp.test.ts b/dev-packages/e2e-tests/test-applications/tsx-express/tests/mcp.test.ts index a89cfcaa11c6..921bb1120d84 100644 --- a/dev-packages/e2e-tests/test-applications/tsx-express/tests/mcp.test.ts +++ b/dev-packages/e2e-tests/test-applications/tsx-express/tests/mcp.test.ts @@ -123,6 +123,23 @@ test('Records transactions for mcp handlers', async ({ baseURL }) => { // TODO: When https://github.com/modelcontextprotocol/typescript-sdk/pull/358 is released check for trace id equality between the post transaction and the handler transaction }); + + await test.step('error tool sets span status to internal_error', async () => { + const toolTransactionPromise = waitForTransaction('tsx-express', transactionEvent => { + return transactionEvent.transaction === 'tools/call always-error'; + }); + + try { + await client.callTool({ name: 'always-error', arguments: {} }); + } catch { + // Expected: MCP SDK throws when the tool returns a JSON-RPC error + } + + const toolTransaction = await toolTransactionPromise; + expect(toolTransaction).toBeDefined(); + expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server'); + expect(toolTransaction.contexts?.trace?.status).toEqual('internal_error'); + }); }); /**