Skip to content

sse_client blocks indefinitely when server has incorrect base URL #447

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.

Sign up for GitHub

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

Jump to bottom
Open
Kudryavkaz opened this issue Apr 7, 2025 · 10 comments · May be fixed by #500
Open

sse_client blocks indefinitely when server has incorrect base URL #447

Kudryavkaz opened this issue Apr 7, 2025 · 10 comments · May be fixed by #500

Comments

@Kudryavkaz
Copy link

Describe the bug
When using the sse_client function to connect to a remote MCP server, if the MCP server sets an incorrect base URL (like http://localhost:8080), the sse_client will raise an exception in the following code:

python-sdk/src/mcp/client/sse.py

Line 81 in c2ca8e0

raise ValueError(error_msg)

The read_stream_writer then sends this exception to the read_stream:

python-sdk/src/mcp/client/sse.py

Line 107 in c2ca8e0

await read_stream_writer.send(exc)

However, since the read_stream has not been yielded yet, the exception will never be received.
As a result, the sse_client will be blocked indefinitely.

To Reproduce

  1. Set up an MCP server with an incorrect base URL on a remote server, for example:
import express, { Request, Response } from 'express';
import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js';
import { SSEServerTransport } from '@modelcontextprotocol/sdk/server/sse.js';
import { z } from 'zod';

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

server.tool(
  "calculate-bmi",
  {
    weightKg: z.number(),
    heightM: z.number()
  },
  async ({ weightKg, heightM }) => ({
    content: [{
      type: "text",
      text: String(weightKg / (heightM * heightM))
    }]
  })
);

const app = express();

const transports: { [sessionId: string]: SSEServerTransport } = {};

app.get('/sse', async (_: Request, res: Response) => {
    // set baseUrl for the transport
    const baseUrl = 'http://localhost:8080';
    const transport = new SSEServerTransport(`${baseUrl}/messages`, res);
    transports[transport.sessionId] = transport;
    res.on('close', () => {
        delete transports[transport.sessionId];
    });
    await server.connect(transport);
});

app.post('/messages', async (req: Request, res: Response) => {
    const sessionId = req.query.sessionId as string;
    const transport = transports[sessionId];
    if (transport) {
        await transport.handlePostMessage(req, res);
    } else {
        res.status(400).send('No transport found for sessionId');
    }
});

app.listen(8080);
  1. Try to connect to the remote MCP server, for example:
import asyncio

from mcp.client.sse import sse_client


async def run():
    try:
        async with sse_client("http://{your_remote_server}:8080/sse"):
            print("Connected to SSE endpoint successfully.")
    except Exception as e:
        print(f"Error: {e}")


if __name__ == "__main__":
    asyncio.run(run())

Expected behavior
The sse_client should raise an exception and exit gracefully.

Logs
None

Additional context
None
@kavinkumar807
Copy link

kavinkumar807 commented Apr 10, 2025 •
edited
Loading

@Kudryavkaz I was able to replicate this issue

Image

The issue in the screenshot I've added must be fixed, as the exception is not handled properly as you've mentioned.

@Kudryavkaz But the above client code, when incorrect URL is added, quits with unhandled error as mentioned in this comment. It is not blocked indefinitely. Can please provide more information about this indefinite block?

Currently debugging this.
Image

@Kudryavkaz
Copy link
Author

@kavinkumar807 Thank you for your reply.

If you start the example MCP server locally and connect the MCP client to "localhost:xxxx", you will not be able to reproduce this issue.

You should start the example MCP server on a remote server, and then use the MCP client to connect to that remote server.

Like this:
Image

"zhangfish.top" is my remote server, and I ran this command on it:
Image

@kavinkumar807
Copy link

kavinkumar807 commented Apr 11, 2025 •
edited
Loading

@Kudryavkaz Debugging the unhandled exception issue. Shall I use your endpoint to test?

@Kudryavkaz
Copy link
Author

@kavinkumar807 Sure, endpoint is "http://zhangfish.top:8080/sse"

@kavinkumar807
Copy link

@Kudryavkaz Thanks

@kavinkumar807
Copy link

@Kudryavkaz I could see there are two issue when I was debugging,

  1. The issue mentioned here, when I try to connect to the above mentioned url the deadlock situation happens and client is blocked instead of graceful shutdown -> has to be fixed :: I'm working on this
  2. The main reason for this exception is that with the above url client is able to make SSE connection but when checked with endpoint_url and url (as in the below screenshot) there is a mismatch
Image

Logs
endpoint_url :: http://localhost:8080/messages?sessionId=18b0b6fb-b3e9-4faf-9350-e2b0d66d6549
url :: http://zhangfish.top:8080/sse
Endpoint origin does not match connection origin: http://localhost:8080/messages?sessionId=18b0b6fb-b3e9-4faf-9350-e2b0d66d6549
Error in sse_reader: Endpoint origin does not match connection origin: http://localhost:8080/messages?sessionId=18b0b6fb-b3e9-4faf-9350-e2b0d66d6549

@dsp-ant @jspahrsummers the endpoint_url has to be http://zhangfish.top:8080/messages?sessionId=18b0b6fb-b3e9-4faf-9350-e2b0d66d6549 right? is it an issue or am I missing something?

@kavinkumar807
Copy link

#410 seems like this issue is related to this. Adding here for reference

@kavinkumar807
Copy link

On further debugging as I've mentioned in the above comment the first point handling exception must be fixed, handling exception gracefully and quitting the client. But feel free to ignore the second point, that's my misunderstanding.

app.get('/sse', async (_: Request, res: Response) => {
    // set baseUrl for the transport
    const baseUrl = 'http://localhost:8080';
    const transport = new SSEServerTransport(`${baseUrl}/messages`, res);
    transports[transport.sessionId] = transport;
    res.on('close', () => {
        delete transports[transport.sessionId];
    });
    await server.connect(transport);
});

@Kudryavkaz Here the baseUrl for the transport is added intentionally to demonstrate the indefinite blocking scenario right?

@Kudryavkaz
Copy link
Author

@kavinkumar807 yes.

@kavinkumar807 kavinkumar807 linked a pull request Apr 13, 2025 that will close this issue
Open
9 tasks
@kavinkumar807
Copy link

@Kudryavkaz Raised PR #500 for the above issue.

Cc: @dsp-ant @jspahrsummers @nick-merrill @Kludex @jerome3o-anthropic

Note: adding the above people in cc from top contributors as I'm not sure who is the maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: implemented exception handling for client indefinite blocks kavinkumar807/mcp-python-sdk
2 participants
@kavinkumar807 @Kudryavkaz