Skip to content

Commit 347422c

Browse files
committed
refactor(mcp-server): improve transport type handling and add tests for stdio and SSE transports
1 parent e193118 commit 347422c

File tree

2 files changed

+172
-11
lines changed

2 files changed

+172
-11
lines changed

packages/core/src/utils/mcp-server/utils.ts

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export function isJsonRpcNotification(message: unknown): message is JsonRpcNotif
5555
);
5656
}
5757

58-
/** Extracts target info from method and params based on method type */
58+
/** Validates MCP server instance with comprehensive type checking */
5959
export function validateMcpServerInstance(instance: unknown): boolean {
6060
if (
6161
typeof instance === 'object' &&
@@ -156,11 +156,26 @@ function getRequestArguments(method: string, params: Record<string, unknown>): R
156156
function getTransportTypes(transport: MCPTransport): { mcpTransport: string; networkTransport: string } {
157157
const transportName = transport.constructor?.name?.toLowerCase() || '';
158158

159-
if (transportName.includes('sse')) return { mcpTransport: 'sse', networkTransport: 'tcp' };
160-
if (transportName.includes('websocket')) return { mcpTransport: 'websocket', networkTransport: 'tcp' };
161-
if (transportName.includes('stdio')) return { mcpTransport: 'stdio', networkTransport: 'pipe' };
159+
// Standard MCP transports per specification
160+
if (transportName.includes('stdio')) {
161+
return { mcpTransport: 'stdio', networkTransport: 'pipe' };
162+
}
163+
164+
// Streamable HTTP is the standard HTTP-based transport
165+
// The official SDK uses 'StreamableHTTPServerTransport' / 'StreamableHTTPClientTransport'
166+
if (transportName.includes('streamablehttp') || transportName.includes('streamable')) {
167+
return { mcpTransport: 'http', networkTransport: 'tcp' };
168+
}
169+
170+
// SSE is the deprecated HTTP+SSE transport (backwards compatibility)
171+
// Note: Modern Streamable HTTP can use SSE internally, but SSE transport is deprecated
172+
if (transportName.includes('sse')) {
173+
return { mcpTransport: 'sse', networkTransport: 'tcp' };
174+
}
162175

163-
return { mcpTransport: 'http', networkTransport: 'tcp' };
176+
// For custom transports, mark as unknown
177+
// TODO(bete): Add support for custom transports
178+
return { mcpTransport: 'unknown', networkTransport: 'unknown' };
164179
}
165180

166181
/** Extracts additional attributes for specific notification types */

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

Lines changed: 152 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,123 @@ describe('wrapMcpServerWithSentry', () => {
530530
);
531531
});
532532
});
533+
534+
describe('Stdio Transport Tests', () => {
535+
let mockMcpServer: ReturnType<typeof createMockMcpServer>;
536+
let wrappedMcpServer: ReturnType<typeof createMockMcpServer>;
537+
let mockStdioTransport: ReturnType<typeof createMockStdioTransport>;
538+
539+
beforeEach(() => {
540+
mockMcpServer = createMockMcpServer();
541+
wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer);
542+
mockStdioTransport = createMockStdioTransport();
543+
mockStdioTransport.sessionId = 'stdio-session-456';
544+
});
545+
546+
it('should detect stdio transport and set correct attributes', async () => {
547+
await wrappedMcpServer.connect(mockStdioTransport);
548+
549+
const jsonRpcRequest = {
550+
jsonrpc: '2.0',
551+
method: 'tools/call',
552+
id: 'req-stdio-1',
553+
params: { name: 'process-file', arguments: { path: '/tmp/data.txt' } },
554+
};
555+
556+
mockStdioTransport.onmessage?.(jsonRpcRequest, {});
557+
558+
expect(startSpanSpy).toHaveBeenCalledWith(
559+
{
560+
name: 'tools/call process-file',
561+
forceTransaction: true,
562+
attributes: {
563+
'mcp.method.name': 'tools/call',
564+
'mcp.tool.name': 'process-file',
565+
'mcp.request.id': 'req-stdio-1',
566+
'mcp.session.id': 'stdio-session-456',
567+
'mcp.transport': 'stdio', // Should be stdio, not http
568+
'network.transport': 'pipe', // Should be pipe, not tcp
569+
'network.protocol.version': '2.0',
570+
'mcp.request.argument.path': '"/tmp/data.txt"',
571+
'sentry.op': 'mcp.server',
572+
'sentry.origin': 'auto.function.mcp_server',
573+
'sentry.source': 'route',
574+
},
575+
},
576+
expect.any(Function),
577+
);
578+
});
579+
580+
it('should handle stdio transport notifications correctly', async () => {
581+
await wrappedMcpServer.connect(mockStdioTransport);
582+
583+
const notification = {
584+
jsonrpc: '2.0',
585+
method: 'notifications/message',
586+
params: {
587+
level: 'debug',
588+
data: 'Processing stdin input',
589+
},
590+
};
591+
592+
mockStdioTransport.onmessage?.(notification, {});
593+
594+
expect(startSpanSpy).toHaveBeenCalledWith(
595+
expect.objectContaining({
596+
name: 'notifications/message',
597+
attributes: expect.objectContaining({
598+
'mcp.method.name': 'notifications/message',
599+
'mcp.session.id': 'stdio-session-456',
600+
'mcp.transport': 'stdio',
601+
'network.transport': 'pipe',
602+
'mcp.logging.level': 'debug',
603+
'mcp.logging.message': 'Processing stdin input',
604+
}),
605+
}),
606+
expect.any(Function),
607+
);
608+
});
609+
});
610+
611+
describe('SSE Transport Tests (Backwards Compatibility)', () => {
612+
let mockMcpServer: ReturnType<typeof createMockMcpServer>;
613+
let wrappedMcpServer: ReturnType<typeof createMockMcpServer>;
614+
let mockSseTransport: ReturnType<typeof createMockSseTransport>;
615+
616+
beforeEach(() => {
617+
mockMcpServer = createMockMcpServer();
618+
wrappedMcpServer = wrapMcpServerWithSentry(mockMcpServer);
619+
mockSseTransport = createMockSseTransport();
620+
mockSseTransport.sessionId = 'sse-session-789';
621+
});
622+
623+
it('should detect SSE transport for backwards compatibility', async () => {
624+
await wrappedMcpServer.connect(mockSseTransport);
625+
626+
const jsonRpcRequest = {
627+
jsonrpc: '2.0',
628+
method: 'resources/read',
629+
id: 'req-sse-1',
630+
params: { uri: 'https://api.example.com/data' },
631+
};
632+
633+
mockSseTransport.onmessage?.(jsonRpcRequest, {});
634+
635+
expect(startSpanSpy).toHaveBeenCalledWith(
636+
expect.objectContaining({
637+
name: 'resources/read https://api.example.com/data',
638+
attributes: expect.objectContaining({
639+
'mcp.method.name': 'resources/read',
640+
'mcp.resource.uri': 'https://api.example.com/data',
641+
'mcp.transport': 'sse', // Deprecated but supported
642+
'network.transport': 'tcp',
643+
'mcp.session.id': 'sse-session-789',
644+
}),
645+
}),
646+
expect.any(Function),
647+
);
648+
});
649+
});
533650
});
534651

535652
// Test helpers
@@ -546,10 +663,39 @@ function createMockMcpServer() {
546663
}
547664

548665
function createMockTransport() {
549-
return {
550-
onmessage: vi.fn(),
551-
onclose: vi.fn(),
552-
send: vi.fn().mockResolvedValue(undefined),
553-
sessionId: 'test-session-123',
554-
};
666+
// exact naming pattern from the official SDK
667+
class StreamableHTTPServerTransport {
668+
onmessage = vi.fn();
669+
onclose = vi.fn();
670+
send = vi.fn().mockResolvedValue(undefined);
671+
sessionId = 'test-session-123';
672+
}
673+
674+
return new StreamableHTTPServerTransport();
675+
}
676+
677+
function createMockStdioTransport() {
678+
// Create a mock that mimics StdioServerTransport
679+
// Using the exact naming pattern from the official SDK
680+
class StdioServerTransport {
681+
onmessage = vi.fn();
682+
onclose = vi.fn();
683+
send = vi.fn().mockResolvedValue(undefined);
684+
sessionId = 'stdio-session-456';
685+
}
686+
687+
return new StdioServerTransport();
688+
}
689+
690+
function createMockSseTransport() {
691+
// Create a mock that mimics the deprecated SSEServerTransport
692+
// For backwards compatibility testing
693+
class SSEServerTransport {
694+
onmessage = vi.fn();
695+
onclose = vi.fn();
696+
send = vi.fn().mockResolvedValue(undefined);
697+
sessionId = 'sse-session-789';
698+
}
699+
700+
return new SSEServerTransport();
555701
}

0 commit comments

Comments
 (0)