fix(core): use sessionId for MCP transport correlation #19172

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

Draft

betegon wants to merge 15 commits into develop from bete/fix/mcp-wrapper-transport-correlation

+1,149 −32

dev-packages/e2e-tests/test-applications/node-express-v5/package.json

-Original file line number
+Diff line change
@@ Expand Up / @@ -11,7 +11,7 @@ @@
         "test:assert": "pnpm test"
       },
       "dependencies": {
-        "@modelcontextprotocol/sdk": "^1.10.2",
+        "@modelcontextprotocol/sdk": "^1.26.0",
         "@sentry/node": "latest || *",
         "@trpc/server": "10.45.4",
         "@trpc/client": "10.45.4",
@@ Expand Down @@

dev-packages/e2e-tests/test-applications/node-express-v5/src/mcp.ts

-Original file line number
+Diff line change
@@ -1,9 +1,16 @@
+    import { randomUUID } from 'node:crypto';
     import express from 'express';
     import { McpServer, ResourceTemplate } from '@modelcontextprotocol/sdk/server/mcp.js';
     import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js';
+    import { StreamableHTTPServerTransport } from '@modelcontextprotocol/sdk/server/streamableHttp.js';
     import { z } from 'zod';
     import { wrapMcpServerWithSentry } from '@sentry/node';
+    // Helper to check if request is an initialize request (compatible with all MCP SDK versions)
+    function isInitializeRequest(body: unknown): boolean {
+      return typeof body === 'object' && body !== null && (body as { method?: string }).method === 'initialize';
+    }
     const mcpRouter = express.Router();
     const server = wrapMcpServerWithSentry(
@@ Expand Down Expand Up / @@ -61,4 +68,134 @@ mcpRouter.post('/messages', async (req, res) => { @@
       }
     });
+    // =============================================================================
+    // Streamable HTTP Transport Endpoints
+    // This uses StreamableHTTPServerTransport which wraps WebStandardStreamableHTTPServerTransport
+    // and exercises the wrapper transport pattern that was fixed in the sessionId-based correlation
+    // See: https://github.com/getsentry/sentry-mcp/issues/767
+    // =============================================================================
+    // Create a separate wrapped server for streamable HTTP (to test independent of SSE)
+    const streamableServer = wrapMcpServerWithSentry(
+      new McpServer({
+        name: 'Echo-Streamable',
+        version: '1.0.0',
+      }),
+    );
+    // Register the same handlers on the streamable server
+    streamableServer.resource(
+      'echo',
+      new ResourceTemplate('echo://{message}', { list: undefined }),
+      async (uri, { message }) => ({
+        contents: [
+          {
+            uri: uri.href,
+            text: `Resource echo: ${message}`,
+          },
+        ],
+      }),
+    );
+    streamableServer.tool('echo', { message: z.string() }, async ({ message }) => {
+      return {
+        content: [{ type: 'text', text: `Tool echo: ${message}` }],
+      };
+    });
+    streamableServer.prompt('echo', { message: z.string() }, ({ message }) => ({
+      messages: [
+        {
+          role: 'user',
+          content: {
+            type: 'text',
+            text: `Please process this message: ${message}`,
+          },
+        },
+      ],
+    }));
+    // Map to store streamable transports by session ID
+    const streamableTransports: Record<string, StreamableHTTPServerTransport> = {};
+    // POST endpoint for streamable HTTP (handles both initialization and subsequent requests)
+    mcpRouter.post('/mcp', express.json(), async (req, res) => {
+      const sessionId = req.headers['mcp-session-id'] as string | undefined;
+      try {
+        let transport: StreamableHTTPServerTransport;
+        if (sessionId && streamableTransports[sessionId]) {
+          // Reuse existing transport for session
+          transport = streamableTransports[sessionId];
+        } else if (!sessionId && isInitializeRequest(req.body)) {
+          // New initialization request - create new transport
+          transport = new StreamableHTTPServerTransport({
+            sessionIdGenerator: () => randomUUID(),
+            onsessioninitialized: sid => {
+              // Store transport when session is initialized
+              streamableTransports[sid] = transport;
+            },
+          });
+          // Clean up on close
+          transport.onclose = () => {
+            const sid = transport.sessionId;
+            if (sid && streamableTransports[sid]) {
+              delete streamableTransports[sid];
+            }
+          };
+          // Connect to server before handling request
+          await streamableServer.connect(transport);
+          await transport.handleRequest(req, res, req.body);
+          return;
+        } else {
+          // Invalid request
+          res.status(400).json({
+            jsonrpc: '2.0',
+            error: { code: -32000, message: 'Bad Request: No valid session ID provided' },
+            id: null,
+          });
+          return;
+        }
+        // Handle request with existing transport
+        await transport.handleRequest(req, res, req.body);
+      } catch (error) {
+        console.error('Error handling streamable HTTP request:', error);
+        if (!res.headersSent) {
+          res.status(500).json({
+            jsonrpc: '2.0',
+            error: { code: -32603, message: 'Internal server error' },
+            id: null,
+          });
+        }
+      }
+    });
+    // GET endpoint for SSE streams (server-initiated messages)
+    mcpRouter.get('/mcp', async (req, res) => {
+      const sessionId = req.headers['mcp-session-id'] as string | undefined;
+      if (!sessionId || !streamableTransports[sessionId]) {
+        res.status(400).send('Invalid or missing session ID');
+        return;
+      }
+      const transport = streamableTransports[sessionId];
+      await transport.handleRequest(req, res);
+    });
+    // DELETE endpoint for session termination
+    mcpRouter.delete('/mcp', async (req, res) => {
+      const sessionId = req.headers['mcp-session-id'] as string | undefined;
+      if (!sessionId || !streamableTransports[sessionId]) {
+        res.status(400).send('Invalid or missing session ID');
+        return;
+      }
+      const transport = streamableTransports[sessionId];
+      await transport.handleRequest(req, res);
+    });
     export { mcpRouter };

dev-packages/e2e-tests/test-applications/node-express-v5/tests/mcp.test.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -2,6 +2,7 @@ import { expect, test } from '@playwright/test'; @@
     import { waitForTransaction } from '@sentry-internal/test-utils';
     import { Client } from '@modelcontextprotocol/sdk/client/index.js';
     import { SSEClientTransport } from '@modelcontextprotocol/sdk/client/sse.js';
+    import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js';
     test('Should record transactions for mcp handlers', async ({ baseURL }) => {
       const transport = new SSEClientTransport(new URL(`${baseURL}/sse`));
@@ Expand Down Expand Up @@
         // 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
       });
     });
+    /**
+     * Tests for StreamableHTTPServerTransport (wrapper transport pattern)
+     *
+     * StreamableHTTPServerTransport wraps WebStandardStreamableHTTPServerTransport via getters/setters.
+     * This causes different `this` values in onmessage vs send, which was breaking span correlation.
+     *
+     * The fix uses sessionId as the correlation key instead of transport object reference.
+     * This test verifies that spans are correctly recorded when using the wrapper transport.
+     *
+     * @see https://github.com/getsentry/sentry-mcp/issues/767
+     */
+    test('Should record transactions for streamable HTTP transport (wrapper transport pattern)', async ({ baseURL }) => {
+      const transport = new StreamableHTTPClientTransport(new URL(`${baseURL}/mcp`));
+      const client = new Client({
+        name: 'test-client-streamable',
+        version: '1.0.0',
+      });
+      const initializeTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
+        return (
+          transactionEvent.transaction === 'initialize' &&
+          transactionEvent.contexts?.trace?.data?.['mcp.server.name'] === 'Echo-Streamable'
+        );
+      });
+      await client.connect(transport);
+      await test.step('initialize handshake', async () => {
+        const initializeTransaction = await initializeTransactionPromise;
+        expect(initializeTransaction).toBeDefined();
+        expect(initializeTransaction.contexts?.trace?.op).toEqual('mcp.server');
+        expect(initializeTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('initialize');
+        expect(initializeTransaction.contexts?.trace?.data?.['mcp.client.name']).toEqual('test-client-streamable');
+        expect(initializeTransaction.contexts?.trace?.data?.['mcp.server.name']).toEqual('Echo-Streamable');
+        // Verify it's using a StreamableHTTP transport (may be wrapper or inner depending on environment)
+        expect(initializeTransaction.contexts?.trace?.data?.['mcp.transport']).toMatch(/StreamableHTTPServerTransport/);
+      });
+      await test.step('tool handler (tests wrapper transport correlation)', async () => {
+        // This is the critical test - without the sessionId fix, the span would not be completed
+        // because onmessage and send see different transport instances (wrapper vs inner)
+        const toolTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
+          const transport = transactionEvent.contexts?.trace?.data?.['mcp.transport'] as string | undefined;
+          return transactionEvent.transaction === 'tools/call echo' && transport?.includes('StreamableHTTPServerTransport');
+        });
+        const toolResult = await client.callTool({
+          name: 'echo',
+          arguments: {
+            message: 'wrapper-transport-test',
+          },
+        });
+        expect(toolResult).toMatchObject({
+          content: [
+            {
+              text: 'Tool echo: wrapper-transport-test',
+              type: 'text',
+            },
+          ],
+        });
+        const toolTransaction = await toolTransactionPromise;
+        expect(toolTransaction).toBeDefined();
+        expect(toolTransaction.contexts?.trace?.op).toEqual('mcp.server');
+        expect(toolTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('tools/call');
+        expect(toolTransaction.contexts?.trace?.data?.['mcp.tool.name']).toEqual('echo');
+        // This attribute proves the span was completed with results (sessionId correlation worked)
+        expect(toolTransaction.contexts?.trace?.data?.['mcp.tool.result.content_count']).toEqual(1);
+      });
+      await test.step('resource handler', async () => {
+        const resourceTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
+          const transport = transactionEvent.contexts?.trace?.data?.['mcp.transport'] as string | undefined;
+          return (
+            transactionEvent.transaction === 'resources/read echo://streamable-test' &&
+            transport?.includes('StreamableHTTPServerTransport')
+          );
+        });
+        const resourceResult = await client.readResource({
+          uri: 'echo://streamable-test',
+        });
+        expect(resourceResult).toMatchObject({
+          contents: [{ text: 'Resource echo: streamable-test', uri: 'echo://streamable-test' }],
+        });
+        const resourceTransaction = await resourceTransactionPromise;
+        expect(resourceTransaction).toBeDefined();
+        expect(resourceTransaction.contexts?.trace?.op).toEqual('mcp.server');
+        expect(resourceTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('resources/read');
+      });
+      await test.step('prompt handler', async () => {
+        const promptTransactionPromise = waitForTransaction('node-express-v5', transactionEvent => {
+          const transport = transactionEvent.contexts?.trace?.data?.['mcp.transport'] as string | undefined;
+          return (
+            transactionEvent.transaction === 'prompts/get echo' && transport?.includes('StreamableHTTPServerTransport')
+          );
+        });
+        const promptResult = await client.getPrompt({
+          name: 'echo',
+          arguments: {
+            message: 'streamable-prompt',
+          },
+        });
+        expect(promptResult).toMatchObject({
+          messages: [
+            {
+              content: {
+                text: 'Please process this message: streamable-prompt',
+                type: 'text',
+              },
+              role: 'user',
+            },
+          ],
+        });
+        const promptTransaction = await promptTransactionPromise;
+        expect(promptTransaction).toBeDefined();
+        expect(promptTransaction.contexts?.trace?.op).toEqual('mcp.server');
+        expect(promptTransaction.contexts?.trace?.data?.['mcp.method.name']).toEqual('prompts/get');
+      });
+      // Clean up - close the client connection
+      await client.close();
+    });

dev-packages/e2e-tests/test-applications/node-express/package.json

-Original file line number
+Diff line change
@@ Expand Up / @@ -11,7 +11,7 @@ @@
         "test:assert": "pnpm test"
       },
       "dependencies": {
-        "@modelcontextprotocol/sdk": "^1.10.2",
+        "@modelcontextprotocol/sdk": "^1.26.0",
         "@sentry/node": "latest || *",
         "@trpc/server": "10.45.4",
         "@trpc/client": "10.45.4",
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(core): use sessionId for MCP transport correlation #19172

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!

Uh oh!

fix(core): use sessionId for MCP transport correlation #19172

Are you sure you want to change the base?

fix(core): use sessionId for MCP transport correlation #19172

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!