Skip to content

fix(mcp-server): Add defensive patches for Transport edge cases #17291

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Aug 12, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 10 additions & 17 deletions packages/core/src/integrations/mcp-server/correlation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,24 +86,17 @@ export function completeSpanWithResults(transport: MCPTransport, requestId: Requ
/**
* Cleans up pending spans for a specific transport (when that transport closes)
* @param transport - MCP transport instance
* @returns Number of pending spans that were cleaned up
*/
export function cleanupPendingSpansForTransport(transport: MCPTransport): number {
export function cleanupPendingSpansForTransport(transport: MCPTransport): void {
const spanMap = transportToSpanMap.get(transport);
if (!spanMap) {
return 0;
}

const pendingCount = spanMap.size;

for (const [, spanData] of spanMap) {
spanData.span.setStatus({
code: SPAN_STATUS_ERROR,
message: 'cancelled',
});
spanData.span.end();
if (spanMap) {
for (const [, spanData] of spanMap) {
spanData.span.setStatus({
code: SPAN_STATUS_ERROR,
message: 'cancelled',
});
spanData.span.end();
}
spanMap.clear();
}

spanMap.clear();
return pendingCount;
}
29 changes: 16 additions & 13 deletions packages/core/src/integrations/mcp-server/sessionExtraction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,34 +152,37 @@ export function extractClientInfo(extra: ExtraHandlerData): {
* @returns Transport type mapping for span attributes
*/
export function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } {
const transportName = transport.constructor?.name?.toLowerCase() || '';

if (transportName.includes('stdio')) {
return { mcpTransport: 'stdio', networkTransport: 'pipe' };
}

if (transportName.includes('streamablehttp') || transportName.includes('streamable')) {
return { mcpTransport: 'http', networkTransport: 'tcp' };
if (!transport?.constructor) {
return { mcpTransport: 'unknown', networkTransport: 'unknown' };
}

if (transportName.includes('sse')) {
return { mcpTransport: 'sse', networkTransport: 'tcp' };
const transportName = typeof transport.constructor?.name === 'string' ? transport.constructor.name : 'unknown';
let networkTransport = 'unknown';

const lowerTransportName = transportName.toLowerCase();
if (lowerTransportName.includes('stdio')) {
networkTransport = 'pipe';
} else if (lowerTransportName.includes('http') || lowerTransportName.includes('sse')) {
networkTransport = 'tcp';
}

return { mcpTransport: 'unknown', networkTransport: 'unknown' };
return {
mcpTransport: transportName,
networkTransport,
};
}

/**
* Build transport and network attributes
* @param transport - MCP transport instance
* @param extra - Optional extra handler data
* @returns Transport attributes for span instrumentation
* @note sessionId may be undefined during initial setup - session should be established by client during initialize flow
*/
export function buildTransportAttributes(
transport: MCPTransport,
extra?: ExtraHandlerData,
): Record<string, string | number> {
const sessionId = transport.sessionId;
const sessionId = transport && 'sessionId' in transport ? transport.sessionId : undefined;
const clientInfo = extra ? extractClientInfo(extra) : {};
const { mcpTransport, networkTransport } = getTransportTypes(transport);
const clientAttributes = getClientAttributes(transport);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ describe('MCP Server Semantic Conventions', () => {
'mcp.session.id': 'test-session-123',
'client.address': '192.168.1.100',
'client.port': 54321,
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'mcp.request.argument.location': '"Seattle, WA"',
Expand Down Expand Up @@ -93,7 +93,7 @@ describe('MCP Server Semantic Conventions', () => {
'mcp.resource.uri': 'file:///docs/api.md',
'mcp.request.id': 'req-2',
'mcp.session.id': 'test-session-123',
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'mcp.request.argument.uri': '"file:///docs/api.md"',
Expand Down Expand Up @@ -125,7 +125,7 @@ describe('MCP Server Semantic Conventions', () => {
'mcp.prompt.name': 'analyze-code',
'mcp.request.id': 'req-3',
'mcp.session.id': 'test-session-123',
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'mcp.request.argument.name': '"analyze-code"',
Expand Down Expand Up @@ -154,7 +154,7 @@ describe('MCP Server Semantic Conventions', () => {
attributes: {
'mcp.method.name': 'notifications/tools/list_changed',
'mcp.session.id': 'test-session-123',
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'sentry.op': 'mcp.notification.client_to_server',
Expand Down Expand Up @@ -193,7 +193,7 @@ describe('MCP Server Semantic Conventions', () => {
'mcp.request.id': 'req-4',
'mcp.session.id': 'test-session-123',
// Transport attributes
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
// Sentry-specific
Expand Down Expand Up @@ -227,7 +227,7 @@ describe('MCP Server Semantic Conventions', () => {
attributes: {
'mcp.method.name': 'notifications/message',
'mcp.session.id': 'test-session-123',
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'mcp.logging.level': 'info',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as currentScopes from '../../../../src/currentScopes';
import { wrapMcpServerWithSentry } from '../../../../src/integrations/mcp-server';
import {
buildTransportAttributes,
extractSessionDataFromInitializeRequest,
extractSessionDataFromInitializeResponse,
getTransportTypes,
} from '../../../../src/integrations/mcp-server/sessionExtraction';
import {
cleanupSessionDataForTransport,
Expand Down Expand Up @@ -214,7 +216,7 @@ describe('MCP Server Transport Instrumentation', () => {
'mcp.tool.name': 'process-file',
'mcp.request.id': 'req-stdio-1',
'mcp.session.id': 'stdio-session-456',
'mcp.transport': 'stdio', // Should be stdio, not http
'mcp.transport': 'StdioServerTransport',
'network.transport': 'pipe', // Should be pipe, not tcp
'network.protocol.version': '2.0',
'mcp.request.argument.path': '"/tmp/data.txt"',
Expand Down Expand Up @@ -245,7 +247,7 @@ describe('MCP Server Transport Instrumentation', () => {
attributes: expect.objectContaining({
'mcp.method.name': 'notifications/message',
'mcp.session.id': 'stdio-session-456',
'mcp.transport': 'stdio',
'mcp.transport': 'StdioServerTransport',
'network.transport': 'pipe',
'mcp.logging.level': 'debug',
'mcp.logging.message': 'Processing stdin input',
Expand Down Expand Up @@ -286,7 +288,7 @@ describe('MCP Server Transport Instrumentation', () => {
attributes: expect.objectContaining({
'mcp.method.name': 'resources/read',
'mcp.resource.uri': 'https://api.example.com/data',
'mcp.transport': 'sse', // Deprecated but supported
'mcp.transport': 'SSEServerTransport',
'network.transport': 'tcp',
'mcp.session.id': 'sse-session-789',
}),
Expand Down Expand Up @@ -361,7 +363,7 @@ describe('MCP Server Transport Instrumentation', () => {
'mcp.session.id': 'test-session-direct',
'client.address': '127.0.0.1',
'client.port': 8080,
'mcp.transport': 'http',
'mcp.transport': 'StreamableHTTPServerTransport',
'network.transport': 'tcp',
'network.protocol.version': '2.0',
'mcp.request.argument.input': '"test"',
Expand Down Expand Up @@ -500,4 +502,86 @@ describe('MCP Server Transport Instrumentation', () => {
expect(getSessionDataForTransport(transportWithoutSession)).toBeUndefined();
});
});

describe('Transport Type Detection', () => {
it('extracts HTTP transport name correctly', () => {
const transport = createMockTransport();
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('StreamableHTTPServerTransport');
expect(result.networkTransport).toBe('tcp');
});

it('extracts stdio transport and maps to pipe network', () => {
const transport = createMockStdioTransport();
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('StdioServerTransport');
expect(result.networkTransport).toBe('pipe');
});

it('extracts SSE transport name', () => {
const transport = createMockSseTransport();
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('SSEServerTransport');
expect(result.networkTransport).toBe('tcp');
});

it('handles transport without constructor', () => {
const transport = Object.create(null);
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('unknown');
expect(result.networkTransport).toBe('unknown');
});

it('handles transport with null/undefined constructor name', () => {
const transport = {
constructor: { name: null },
onmessage: () => {},
send: async () => {},
};
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('unknown');
expect(result.networkTransport).toBe('unknown');
});

it('returns unknown network transport for unrecognized transport types', () => {
const transport = {
constructor: { name: 'CustomTransport' },
onmessage: () => {},
send: async () => {},
};
const result = getTransportTypes(transport);

expect(result.mcpTransport).toBe('CustomTransport');
expect(result.networkTransport).toBe('unknown');
});
});

describe('buildTransportAttributes sessionId handling', () => {
it('includes sessionId when present', () => {
const transport = createMockTransport();
const attributes = buildTransportAttributes(transport);

expect(attributes['mcp.session.id']).toBe('test-session-123');
});

it('excludes sessionId when undefined', () => {
const transport = createMockTransport();
transport.sessionId = undefined;
const attributes = buildTransportAttributes(transport);

expect(attributes['mcp.session.id']).toBeUndefined();
});

it('excludes sessionId when not present in transport', () => {
const transport = { onmessage: () => {}, send: async () => {} };
const attributes = buildTransportAttributes(transport);

expect(attributes['mcp.session.id']).toBeUndefined();
});
});
});
Loading