Skip to content

server/sse: return response in handle_post_message #83

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

Closed

Conversation

artmoskvin
Copy link

@artmoskvin artmoskvin commented Dec 1, 2024

Changes

  • Modified SseServerTransport.handle_post_message to return Response objects instead of sending them directly via ASGI

Motivation and Context

The current implementation directly sends ASGI responses, which breaks when used with Starlette because Starlette expects route handlers to return response objects. After the request is handled, Starlette tries to call the response object which currently fails because the handle_post_message method returns None:

ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/uvicorn/protocols/http/h11_impl.py", line 403, in run_asgi
    result = await app(  # type: ignore[func-returns-value]
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        self.scope, self.receive, self.send
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/uvicorn/middleware/proxy_headers.py", line 60, in __call__
    return await self.app(scope, receive, send)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/applications.py", line 113, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/middleware/errors.py", line 187, in __call__
    raise exc
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/middleware/errors.py", line 165, in __call__
    await self.app(scope, receive, _send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/middleware/exceptions.py", line 62, in __call__
    await wrap_app_handling_exceptions(self.app, conn)(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/routing.py", line 715, in __call__
    await self.middleware_stack(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/routing.py", line 735, in app
    await route.handle(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/routing.py", line 288, in handle
    await self.app(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/routing.py", line 76, in app
    await wrap_app_handling_exceptions(app, request)(scope, receive, send)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/_exception_handler.py", line 53, in wrapped_app
    raise exc
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/_exception_handler.py", line 42, in wrapped_app
    await app(scope, receive, sender)
  File "/Users/artemm/Code/mcp_proxy/.venv/lib/python3.13/site-packages/starlette/routing.py", line 74, in app
    await response(scope, receive, send)
          ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
TypeError: 'NoneType' object is not callable

How Has This Been Tested?

Tested the change with this code running locally:

# server.py
import logging
from mcp.server import Server
from mcp.server.sse import SseServerTransport
import mcp.types as types
from starlette.applications import Starlette
from starlette.routing import Route

logging.basicConfig(level=logging.DEBUG)
logger = logging.getLogger("sse-server")

# Create SSE transport with endpoint
app = Server("example-server")
sse = SseServerTransport("/messages")


@app.list_tools()
async def handle_list_tools():
    return [
        types.Tool(
            name="hello",
            description="Says hello",
            inputSchema={},
        )
    ]


# Handler for SSE connections
async def handle_sse(request):
    async with sse.connect_sse(
        request.scope, request.receive, request._send
    ) as streams:
        await app.run(streams[0], streams[1], app.create_initialization_options())


# Handler for client messages
async def handle_messages(request):
    await sse.handle_post_message(request.scope, request.receive, request._send)


# Create Starlette app with routes
starlette_app = Starlette(
    debug=True,
    routes=[
        Route("/sse", endpoint=handle_sse),
        Route("/messages", endpoint=handle_messages, methods=["POST"]),
    ],
)

# Run with any ASGI server
import uvicorn

uvicorn.run(starlette_app, host="0.0.0.0", port=8000)
# client.py
import asyncio
from mcp.client.sse import sse_client
from mcp.client.session import ClientSession

async def get_tools():
    async with sse_client("http://localhost:8000/sse") as streams:
        async with ClientSession(streams[0], streams[1]) as session:
            await session.initialize()

            # List available tools
            return await session.list_tools()

async def main():
    tools = await get_tools()
    print(tools.tools)

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

Breaking Changes

Technically, the change is breaking because it removes on of the unused parameter but it didn't work anyway.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

Make SseServerTransport.handle_post_message return Response objects
instead of sending them via ASGI, improving framework compatibility and
separation of concerns.
@dsp-ant dsp-ant requested a review from jspahrsummers December 2, 2024 11:39
@artmoskvin
Copy link
Author

@jspahrsummers hello! could you please review this PR? i found a workaround but it's kinda ugly:

async def handle_messages(request):
    # Create a dummy response that we'll return to Starlette
    from starlette.responses import Response

    response = Response("", status_code=202)

    async def send_wrapper(message):
        # Skip sending response since we're handling it at the Starlette level
        if (
            message["type"] != "http.response.start"
            and message["type"] != "http.response.body"
        ):
            await request._send(message)

    await sse.handle_post_message(request.scope, request.receive, send_wrapper)
    return response

Copy link
Member

@jspahrsummers jspahrsummers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We solved this internally by using a Mount instead of a Route—e.g.:

routes = [Mount("/message/", app=sse.handle_post_message)]

Would that work for your use case? It's nice for handle_post_message to support the full ASGI application interface, as that generalizes beyond Starlette. Returning a Response might make it harder to use in other contexts.

@dsp-ant
Copy link
Member

dsp-ant commented Dec 10, 2024

Closing this out since it seems we have a path forwrad that doesnt require a code change.

@sparfenyuk
Copy link

@dsp-ant will you update the documentation and code samples, then?

@allenporter
Copy link
Contributor

@dsp-ant will you update the documentation and code samples, then?

I've sent #121 to update

samihamine pushed a commit to samihamine/fastmcp that referenced this pull request Jan 2, 2025
This change follows the recommended approach from MCP SDK (modelcontextprotocol/python-sdk#83)
for handling SSE messages using Starlette's Mount instead of Route.

- Replace Route with Mount for /messages/ endpoint
- Update SseServerTransport path to include trailing slash
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants