Skip to content

Commit 68356d4

Browse files
betegonclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 10f3eaf commit 68356d4

File tree

3 files changed

+68
-2
lines changed

3 files changed

+68
-2
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,19 +81,23 @@ export function storeSpanForRequest(transport: MCPTransport, requestId: RequestI
8181
* @param requestId - Request identifier
8282
* @param result - Execution result for attribute extraction
8383
* @param options - Resolved MCP options
84+
* @param hasError - Whether the JSON-RPC response contained an error
8485
*/
8586
export function completeSpanWithResults(
8687
transport: MCPTransport,
8788
requestId: RequestId,
8889
result: unknown,
8990
options: ResolvedMcpOptions,
91+
hasError = false,
9092
): void {
9193
const spanMap = getOrCreateSpanMap(transport);
9294
const spanData = spanMap.get(requestId);
9395
if (spanData) {
9496
const { span, method } = spanData;
9597

96-
if (method === 'initialize') {
98+
if (hasError) {
99+
span.setStatus({ code: SPAN_STATUS_ERROR, message: 'internal_error' });
100+
} else if (method === 'initialize') {
97101
const sessionData = extractSessionDataFromInitializeResponse(result);
98102
const serverAttributes = buildServerAttributesFromInfo(sessionData.serverInfo);
99103

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export function wrapTransportSend(transport: MCPTransport, options: ResolvedMcpO
121121
}
122122
}
123123

124-
completeSpanWithResults(this, message.id, message.result, options);
124+
completeSpanWithResults(this, message.id, message.result, options, !!message.error);
125125
}
126126
}
127127

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

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,68 @@ describe('MCP Server Transport Instrumentation', () => {
182182
// Trigger onclose - should not throw
183183
expect(() => mockTransport.onclose?.()).not.toThrow();
184184
});
185+
186+
it('should set span status to error when JSON-RPC error response is sent', async () => {
187+
const mockSpan = {
188+
setAttributes: vi.fn(),
189+
setStatus: vi.fn(),
190+
end: vi.fn(),
191+
isRecording: vi.fn().mockReturnValue(true),
192+
};
193+
startInactiveSpanSpy.mockReturnValue(mockSpan as any);
194+
195+
await wrappedMcpServer.connect(mockTransport);
196+
197+
// Simulate an incoming tools/call request
198+
const jsonRpcRequest = {
199+
jsonrpc: '2.0',
200+
method: 'tools/call',
201+
id: 'req-err-1',
202+
params: { name: 'always-error' },
203+
};
204+
mockTransport.onmessage?.(jsonRpcRequest, {});
205+
206+
// Simulate the MCP SDK sending back a JSON-RPC error response
207+
const jsonRpcErrorResponse = {
208+
jsonrpc: '2.0',
209+
id: 'req-err-1',
210+
error: { code: -32603, message: 'Internal error: tool threw an exception' },
211+
};
212+
await mockTransport.send?.(jsonRpcErrorResponse as any);
213+
214+
expect(mockSpan.setStatus).toHaveBeenCalledWith({ code: 2, message: 'internal_error' });
215+
expect(mockSpan.end).toHaveBeenCalled();
216+
});
217+
218+
it('should not set error span status for successful JSON-RPC responses', async () => {
219+
const mockSpan = {
220+
setAttributes: vi.fn(),
221+
setStatus: vi.fn(),
222+
end: vi.fn(),
223+
isRecording: vi.fn().mockReturnValue(true),
224+
};
225+
startInactiveSpanSpy.mockReturnValue(mockSpan as any);
226+
227+
await wrappedMcpServer.connect(mockTransport);
228+
229+
const jsonRpcRequest = {
230+
jsonrpc: '2.0',
231+
method: 'tools/call',
232+
id: 'req-ok-1',
233+
params: { name: 'echo' },
234+
};
235+
mockTransport.onmessage?.(jsonRpcRequest, {});
236+
237+
const jsonRpcSuccessResponse = {
238+
jsonrpc: '2.0',
239+
id: 'req-ok-1',
240+
result: { content: [{ type: 'text', text: 'hello' }] },
241+
};
242+
await mockTransport.send?.(jsonRpcSuccessResponse as any);
243+
244+
expect(mockSpan.setStatus).not.toHaveBeenCalled();
245+
expect(mockSpan.end).toHaveBeenCalled();
246+
});
185247
});
186248

187249
describe('Stdio Transport Tests', () => {

0 commit comments

Comments
 (0)