Skip to content

Commit 7f68a56

Browse files
codydeclaude
andcommitted
fix(mcp-server): Add defensive patches for StreamableHTTPServerTransport edge cases
This patch adds defensive handling for edge cases in MCP server instrumentation when working with StreamableHTTPServerTransport and similar transport implementations. Changes: - Add transport constructor null checks in getTransportTypes() - Graceful sessionId undefined handling in buildTransportAttributes() - WeakMap correlation fallback system for invalid transport objects - Type validation for WeakMap operations in correlation.ts The patches prevent runtime errors during MCP initialization and session establishment while maintaining backward compatibility and proper instrumentation functionality. Includes comprehensive test coverage for all edge cases and transport scenarios. Fixes edge cases where: - Transport objects have undefined/null constructors - sessionId is undefined during initialization phases - Transport objects cannot be used as WeakMap keys - Invalid transport objects are passed to correlation functions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
1 parent 8f4d56f commit 7f68a56

File tree

4 files changed

+407
-2
lines changed

4 files changed

+407
-2
lines changed

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,11 @@ import type {
4545
* @returns Transport type mapping for span attributes
4646
*/
4747
export function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } {
48-
const transportName = transport.constructor?.name?.toLowerCase() || '';
48+
// Handle undefined transport gracefully while preserving type detection
49+
if (!transport || !transport.constructor) {
50+
return { mcpTransport: 'unknown', networkTransport: 'unknown' };
51+
}
52+
const transportName = transport.constructor.name?.toLowerCase() || '';
4953

5054
if (transportName.includes('stdio')) {
5155
return { mcpTransport: 'stdio', networkTransport: 'pipe' };
@@ -265,7 +269,9 @@ export function buildTransportAttributes(
265269
transport: MCPTransport,
266270
extra?: ExtraHandlerData,
267271
): Record<string, string | number> {
268-
const sessionId = transport.sessionId;
272+
// Gracefully handle undefined sessionId during MCP initialization
273+
// Respects client-provided sessions and waits for proper session establishment
274+
const sessionId = transport && 'sessionId' in transport ? transport.sessionId : undefined;
269275
const clientInfo = extra ? extractClientInfo(extra) : {};
270276
const { mcpTransport, networkTransport } = getTransportTypes(transport);
271277
const clientAttributes = getClientAttributes(transport);

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,13 +19,25 @@ import type { MCPTransport, RequestId, RequestSpanMapValue } from './types';
1919
*/
2020
const transportToSpanMap = new WeakMap<MCPTransport, Map<RequestId, RequestSpanMapValue>>();
2121

22+
/**
23+
* Fallback span map for invalid transport objects
24+
* @internal Used when transport objects cannot be used as WeakMap keys
25+
*/
26+
const fallbackSpanMap = new Map<RequestId, RequestSpanMapValue>();
27+
2228
/**
2329
* Gets or creates the span map for a specific transport instance
2430
* @internal
2531
* @param transport - MCP transport instance
2632
* @returns Span map for the transport
2733
*/
2834
function getOrCreateSpanMap(transport: MCPTransport): Map<RequestId, RequestSpanMapValue> {
35+
// Handle invalid transport values for WeakMap while preserving correlation
36+
if (!transport || typeof transport !== 'object') {
37+
// Return persistent fallback Map to maintain correlation across calls
38+
return fallbackSpanMap;
39+
}
40+
2941
let spanMap = transportToSpanMap.get(transport);
3042
if (!spanMap) {
3143
spanMap = new Map();
Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
import { describe, expect, it } from 'vitest';
2+
import {
3+
buildTransportAttributes,
4+
getTransportTypes,
5+
} from '../../../../src/integrations/mcp-server/attributeExtraction';
6+
import type { MCPTransport } from '../../../../src/integrations/mcp-server/types';
7+
8+
describe('attributeExtraction edge cases', () => {
9+
describe('getTransportTypes', () => {
10+
it('handles undefined transport gracefully', () => {
11+
const result = getTransportTypes(undefined as any);
12+
expect(result).toEqual({
13+
mcpTransport: 'unknown',
14+
networkTransport: 'unknown',
15+
});
16+
});
17+
18+
it('handles null transport gracefully', () => {
19+
const result = getTransportTypes(null as any);
20+
expect(result).toEqual({
21+
mcpTransport: 'unknown',
22+
networkTransport: 'unknown',
23+
});
24+
});
25+
26+
it('handles transport with null constructor', () => {
27+
const transport = {
28+
constructor: null,
29+
} as any;
30+
31+
const result = getTransportTypes(transport);
32+
expect(result).toEqual({
33+
mcpTransport: 'unknown',
34+
networkTransport: 'unknown',
35+
});
36+
});
37+
38+
it('handles transport with undefined constructor', () => {
39+
const transport = {
40+
constructor: undefined,
41+
} as any;
42+
43+
const result = getTransportTypes(transport);
44+
expect(result).toEqual({
45+
mcpTransport: 'unknown',
46+
networkTransport: 'unknown',
47+
});
48+
});
49+
50+
it('correctly identifies StreamableHTTPServerTransport', () => {
51+
class StreamableHTTPServerTransport {
52+
sessionId = 'test-session';
53+
}
54+
55+
const transport = new StreamableHTTPServerTransport() as MCPTransport;
56+
const result = getTransportTypes(transport);
57+
58+
expect(result).toEqual({
59+
mcpTransport: 'http',
60+
networkTransport: 'tcp',
61+
});
62+
});
63+
64+
it('correctly identifies SSE transport', () => {
65+
class SSEServerTransport {
66+
sessionId = 'sse-session';
67+
}
68+
69+
const transport = new SSEServerTransport() as MCPTransport;
70+
const result = getTransportTypes(transport);
71+
72+
expect(result).toEqual({
73+
mcpTransport: 'sse',
74+
networkTransport: 'tcp',
75+
});
76+
});
77+
78+
it('correctly identifies stdio transport', () => {
79+
class StdioServerTransport {
80+
sessionId = 'stdio-session';
81+
}
82+
83+
const transport = new StdioServerTransport() as MCPTransport;
84+
const result = getTransportTypes(transport);
85+
86+
expect(result).toEqual({
87+
mcpTransport: 'stdio',
88+
networkTransport: 'pipe',
89+
});
90+
});
91+
});
92+
93+
describe('buildTransportAttributes', () => {
94+
it('handles undefined sessionId gracefully', () => {
95+
const transport = {
96+
constructor: { name: 'StreamableHTTPServerTransport' },
97+
// No sessionId property
98+
} as MCPTransport;
99+
100+
const attributes = buildTransportAttributes(transport);
101+
102+
// Should not include sessionId in attributes when undefined
103+
expect(attributes['mcp.session.id']).toBeUndefined();
104+
expect(attributes['mcp.transport']).toBe('http');
105+
});
106+
107+
it('handles transport without sessionId property', () => {
108+
const transport = {
109+
constructor: { name: 'StreamableHTTPServerTransport' },
110+
// sessionId property doesn't exist at all
111+
} as any;
112+
113+
const attributes = buildTransportAttributes(transport);
114+
115+
// Should not include sessionId in attributes
116+
expect(attributes['mcp.session.id']).toBeUndefined();
117+
expect(attributes['mcp.transport']).toBe('http');
118+
});
119+
120+
it('includes sessionId when properly set', () => {
121+
const transport = {
122+
constructor: { name: 'StreamableHTTPServerTransport' },
123+
sessionId: 'test-session-123',
124+
} as MCPTransport;
125+
126+
const attributes = buildTransportAttributes(transport);
127+
128+
expect(attributes['mcp.session.id']).toBe('test-session-123');
129+
expect(attributes['mcp.transport']).toBe('http');
130+
});
131+
132+
it('handles null sessionId gracefully', () => {
133+
const transport = {
134+
constructor: { name: 'StreamableHTTPServerTransport' },
135+
sessionId: null,
136+
} as any;
137+
138+
const attributes = buildTransportAttributes(transport);
139+
140+
// Should not include null sessionId in attributes
141+
expect(attributes['mcp.session.id']).toBeUndefined();
142+
expect(attributes['mcp.transport']).toBe('http');
143+
});
144+
145+
it('handles empty string sessionId', () => {
146+
const transport = {
147+
constructor: { name: 'StreamableHTTPServerTransport' },
148+
sessionId: '',
149+
} as MCPTransport;
150+
151+
const attributes = buildTransportAttributes(transport);
152+
153+
// Empty string is falsy, so should not be included
154+
expect(attributes['mcp.session.id']).toBeUndefined();
155+
expect(attributes['mcp.transport']).toBe('http');
156+
});
157+
158+
it('preserves all other attributes when sessionId is undefined', () => {
159+
const transport = {
160+
constructor: { name: 'StreamableHTTPServerTransport' },
161+
// No sessionId
162+
} as MCPTransport;
163+
164+
const attributes = buildTransportAttributes(transport, {
165+
clientAddress: '127.0.0.1',
166+
clientPort: 8080,
167+
});
168+
169+
expect(attributes).toMatchObject({
170+
'mcp.transport': 'http',
171+
'network.transport': 'tcp',
172+
'network.protocol.version': '2.0',
173+
'client.address': '127.0.0.1',
174+
'client.port': 8080,
175+
});
176+
177+
// sessionId should not be present
178+
expect(attributes['mcp.session.id']).toBeUndefined();
179+
});
180+
});
181+
});

0 commit comments

Comments
 (0)