Skip to content

SSE example is broken #187

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

Open
TheBestMoshe opened this issue Mar 12, 2025 · 5 comments
Open

SSE example is broken #187

TheBestMoshe opened this issue Mar 12, 2025 · 5 comments
Labels
bug Something isn't working

Comments

@TheBestMoshe
Copy link

TheBestMoshe commented Mar 12, 2025

Describe the bug
The messages endpoint returns a 400 because body parsing fails

To Reproduce
Steps to reproduce the behavior:

import express from "express";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { SSEServerTransport } from "@modelcontextprotocol/sdk/server/sse.js";

const server = new McpServer({
  name: "example-server",
  version: "1.0.0"
});

// ... set up server resources, tools, and prompts ...

const app = express();

let transport: SSEServerTransport | undefined = undefined;

app.get("/sse", async (req, res) => {
  transport ??= new SSEServerTransport("/messages", res);
  await server.connect(transport);
});

app.post("/messages", async (req, res) => {
  await transport.handlePostMessage(req, res);
});

app.listen(3001);

Expected behavior
Should be able to connect to server via npx @modelcontextprotocol/inspector and setting sse endpoint to http://localhost:3001

Additional context
I was able to work around it by passing the body to handlePostMessage: await transport.handlePostMessage(req, res, req.body);

I'm unsure where the source of the error is from. Is the inspector sending the incorrect data to the messages endpoint (Cursor also has the same issue) or is the sdk not working correctly?

@TheBestMoshe TheBestMoshe added the bug Something isn't working label Mar 12, 2025
@cliffhall
Copy link
Contributor

cliffhall commented Mar 13, 2025

Observations

When running this example and connecting to it with the Inspector, I get the following in the server console output:

> node dist/index.cjs

node:_http_server:344
    throw new ERR_HTTP_HEADERS_SENT('write');
          ^

Error [ERR_HTTP_HEADERS_SENT]: Cannot write headers after they are sent to the client
    at ServerResponse.writeHead (node:_http_server:344:11)
    at SSEServerTransport.start (/Users/cliffhall/Projects/puzzlebox/dist/index.cjs:11673:14)
    at Server.connect (/Users/cliffhall/Projects/puzzlebox/dist/index.cjs:9627:27)
    at McpServer.connect (/Users/cliffhall/Projects/puzzlebox/dist/index.cjs:11341:30)
    at /Users/cliffhall/Projects/puzzlebox/dist/index.cjs:11772:16
    at Layer.handle [as handle_request] (/Users/cliffhall/Projects/puzzlebox/node_modules/express/lib/router/layer.js:95:5)
    at next (/Users/cliffhall/Projects/puzzlebox/node_modules/express/lib/router/route.js:149:13)
    at Route.dispatch (/Users/cliffhall/Projects/puzzlebox/node_modules/express/lib/router/route.js:119:3)
    at Layer.handle [as handle_request] (/Users/cliffhall/Projects/puzzlebox/node_modules/express/lib/router/layer.js:95:5)
    at /Users/cliffhall/Projects/puzzlebox/node_modules/express/lib/router/index.js:284:15 {
  code: 'ERR_HTTP_HEADERS_SENT'
}

Solution

There are problems in the example that must be fixed; Typescript will complain about the fact that transport is a const defined in the handler for the /sse endpoint and is referenced in the one for /messages . I've addressed this with #193.

I see that @TheBestMoshe has done this in his example by moving the declaration outside the handler and using let not const so it can be set in the /sse handler. Since his varied slightly, I started from the example and went from there.

Below is the example from the SDK page, but with that fixed.

As far as my tired eyes can see, it is the same as posted above except I don't define transport as being possibly undefined and don't use nullish coalescence operator when setting it inside the /sse handler. But it works as written below. I can run it and connect to it with the Inspector.

import express from "express";
import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js";
import { SSEServerTransport } from "@modelcontextprotocol/sdk/server/sse.js";

const server = new McpServer({
  name: "example-server",
  version: "1.0.0"
});

// ... set up server resources, tools, and prompts ...

const app = express();
let transport: SSEServerTransport;
app.get("/sse", async (req, res) => {
  transport = new SSEServerTransport("/messages", res);
  await server.connect(transport);
});

app.post("/messages", async (req, res) => {
  // Note: to support multiple simultaneous connections, these messages will
  // need to be routed to a specific matching transport. (This logic isn't
  // implemented here, for simplicity.)
  await transport.handlePostMessage(req, res);
});

app.listen(3001);

cliffhall added a commit to cliffhall/mcp-typescript-sdk that referenced this issue Mar 13, 2025
…red as a const inside the handler for /sse endpoint and referenced in the handler for the /messages endpoint.

This fixes modelcontextprotocol#187
@Symbolk
Copy link

Symbolk commented Mar 31, 2025

Same problem here! But as I add the param here:

await transport.handlePostMessage(req, res, req.body);

It works!
(MCP is popular but it really needs time to grow, now the doc, coding style, and sdk quality is really annoying)

@cliffhall
Copy link
Contributor

cliffhall commented Mar 31, 2025

Same problem here! But as I add the param here:

await transport.handlePostMessage(req, res, req.body);

It works! (MCP is popular but it really needs time to grow, now the doc, coding style, and sdk quality is really annoying)

Are you using the latest version of the Typescript SDK? And also, the above example, taken from the SDK README, was changed recently. Try with this setup.

@adcentury
Copy link

Same problem here! But as I add the param here:

await transport.handlePostMessage(req, res, req.body);

It works! (MCP is popular but it really needs time to grow, now the doc, coding style, and sdk quality is really annoying)

Same issue here, and this works for me. I'm using the latest version of the sdk (1.9.0).

Image

@cliffhall
Copy link
Contributor

cliffhall commented Apr 16, 2025

@adcentury @TheBestMoshe @Symbolk Here is what I did to test the current example.

In a local copy of the servers repo, in the everything server, there is an sse.ts file that does mostly what this example does.

  • In that project, I created example.ts and pasted in the current example from the docs.
  • Compiled and ran it and when I connected, I was told that the server didn't support any features. So I couldn't send a message to the /message endpoint to test.
  • I commented out the minimal server and instead use the factory from everything.ts
  • That just adds capabilities, doesn't have anything to do with whether the message body is handled properly.
  • I upgraded the typescript-sdk version in the package.json from 1.0.1 to 1.9.0.

What my code looks like

Image

The server responds just fine

Image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants