From 3a98694a975e3608ab54c9fe3b52b9fd18b6bd05 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 13:54:14 -0700 Subject: [PATCH 01/11] parity with ts sdk --- src/mcp/client/streamable_http.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index b1ab2c079..0a3d1e4c5 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -281,15 +281,16 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: content_type = response.headers.get(CONTENT_TYPE, "").lower() - if content_type.startswith(JSON): - await self._handle_json_response(response, ctx.read_stream_writer, is_initialization) - elif content_type.startswith(SSE): - await self._handle_sse_response(response, ctx, is_initialization) - else: - await self._handle_unexpected_content_type( - content_type, - ctx.read_stream_writer, - ) + if isinstance(message.root, JSONRPCRequest): + if content_type.startswith(JSON): + await self._handle_json_response(response, ctx.read_stream_writer, is_initialization) + elif content_type.startswith(SSE): + await self._handle_sse_response(response, ctx, is_initialization) + else: + await self._handle_unexpected_content_type( + content_type, + ctx.read_stream_writer, + ) async def _handle_json_response( self, From e6cdb23aab60c442c8ebbc0513e3f87efd5fdfdb Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 14:48:31 -0700 Subject: [PATCH 02/11] test --- tests/client/test_notification_response.py | 169 +++++++++++++++++++++ 1 file changed, 169 insertions(+) create mode 100644 tests/client/test_notification_response.py diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py new file mode 100644 index 000000000..c40ada9ac --- /dev/null +++ b/tests/client/test_notification_response.py @@ -0,0 +1,169 @@ +""" +Tests for StreamableHTTP client transport with non-SDK servers. + +These tests verify client behavior when interacting with servers +that don't follow SDK conventions. +""" + +import json +import multiprocessing +import socket +import time +from collections.abc import Generator + +import pytest +import uvicorn +from starlette.applications import Starlette +from starlette.requests import Request +from starlette.responses import JSONResponse, Response +from starlette.routing import Route + +from mcp.client.session import ClientSession +from mcp.client.streamable_http import streamablehttp_client +from mcp.types import ClientNotification, Implementation, RootsListChangedNotification + + +def create_non_sdk_server_app() -> Starlette: + """Create a minimal server that doesn't follow SDK conventions.""" + + async def handle_mcp_request(request: Request) -> Response: + """Handle MCP requests with non-standard responses.""" + try: + body = await request.body() + data = json.loads(body) + + # Handle initialize request normally + if data.get("method") == "initialize": + response_data = { + "jsonrpc": "2.0", + "id": data["id"], + "result": { + "serverInfo": { + "name": "test-non-sdk-server", + "version": "1.0.0" + }, + "protocolVersion": "2024-11-05", + "capabilities": {} + } + } + return JSONResponse(response_data) + + # For notifications, return 204 No Content (non-SDK behavior) + if "id" not in data: + return Response(status_code=204) + + # Default response for other requests + return JSONResponse({ + "jsonrpc": "2.0", + "id": data.get("id"), + "error": { + "code": -32601, + "message": "Method not found" + } + }) + + except Exception as e: + return JSONResponse( + {"error": f"Server error: {str(e)}"}, + status_code=500 + ) + + app = Starlette( + debug=True, + routes=[ + Route("/mcp", handle_mcp_request, methods=["POST"]), + ], + ) + return app + + +def run_non_sdk_server(port: int) -> None: + """Run the non-SDK server in a separate process.""" + app = create_non_sdk_server_app() + config = uvicorn.Config( + app=app, + host="127.0.0.1", + port=port, + log_level="error", # Reduce noise in tests + ) + server = uvicorn.Server(config=config) + server.run() + + +@pytest.fixture +def non_sdk_server_port() -> int: + """Get an available port for the test server.""" + with socket.socket() as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +@pytest.fixture +def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: + """Start a non-SDK server for testing.""" + proc = multiprocessing.Process( + target=run_non_sdk_server, + kwargs={"port": non_sdk_server_port}, + daemon=True + ) + proc.start() + + # Wait for server to be ready + start_time = time.time() + while time.time() - start_time < 10: + try: + with socket.create_connection(("127.0.0.1", non_sdk_server_port), timeout=0.1): + break + except (TimeoutError, ConnectionRefusedError): + time.sleep(0.1) + else: + proc.kill() + proc.join(timeout=2) + pytest.fail("Server failed to start within 10 seconds") + + yield + + proc.kill() + proc.join(timeout=2) + + +@pytest.mark.anyio +async def test_notification_with_204_response( + non_sdk_server: None, + non_sdk_server_port: int +) -> None: + """Test that client handles 204 responses to notifications correctly. + + This test verifies the fix for the issue where non-SDK servers + might return 204 No Content for notifications instead of 202 Accepted. + The client should handle this gracefully without trying to parse + the response body. + """ + server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" + + async with streamablehttp_client(server_url) as (read_stream, write_stream, get_session_id): + async with ClientSession( + read_stream, + write_stream, + client_info=Implementation(name="test-client", version="1.0.0") + ) as session: + # Initialize should work normally + await session.initialize() + + # Send a notification - this should not raise an error + # even though the server returns 204 instead of 202 + notification_sent = False + try: + await session.send_notification( + ClientNotification( + RootsListChangedNotification( + method="notifications/roots/list_changed", + params={} + ) + ) + ) + notification_sent = True + except Exception as e: + pytest.fail(f"Notification failed with 204 response: {e}") + + assert notification_sent, "Notification should have been sent successfully" From 1f5e0f3389add6096ec5b22ac932c881de31f488 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 18:33:07 -0700 Subject: [PATCH 03/11] fixup test --- tests/client/test_notification_response.py | 41 +++++++++++++--------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index c40ada9ac..0b28db96f 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -13,12 +13,13 @@ import pytest import uvicorn +from mcp.shared.session import RequestResponder from starlette.applications import Starlette from starlette.requests import Request from starlette.responses import JSONResponse, Response from starlette.routing import Route -from mcp.client.session import ClientSession +from mcp import ClientSession, types from mcp.client.streamable_http import streamablehttp_client from mcp.types import ClientNotification, Implementation, RootsListChangedNotification @@ -50,7 +51,10 @@ async def handle_mcp_request(request: Request) -> Response: # For notifications, return 204 No Content (non-SDK behavior) if "id" not in data: - return Response(status_code=204) + return Response( + status_code=204, + headers={"Content-Type": "application/json"} + ) # Default response for other requests return JSONResponse({ @@ -140,30 +144,33 @@ async def test_notification_with_204_response( the response body. """ server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" - + returned_exception = None + async def message_handler(message: RequestResponder[types.ServerRequest, types.ClientResult] | types.ServerNotification | Exception): + nonlocal returned_exception + if isinstance(message, Exception): + returned_exception = message + async with streamablehttp_client(server_url) as (read_stream, write_stream, get_session_id): async with ClientSession( read_stream, write_stream, - client_info=Implementation(name="test-client", version="1.0.0") + message_handler=message_handler, ) as session: # Initialize should work normally await session.initialize() # Send a notification - this should not raise an error # even though the server returns 204 instead of 202 - notification_sent = False - try: - await session.send_notification( - ClientNotification( - RootsListChangedNotification( - method="notifications/roots/list_changed", - params={} - ) + # Without the fix, this would fail with a JSON parsing error + # because the client would try to parse the empty 204 response body + await session.send_notification( + ClientNotification( + RootsListChangedNotification( + method="notifications/roots/list_changed", + params={} ) ) - notification_sent = True - except Exception as e: - pytest.fail(f"Notification failed with 204 response: {e}") - - assert notification_sent, "Notification should have been sent successfully" + ) + + if returned_exception: + pytest.fail(f"Server encountered an exception: {returned_exception}") From 1d8464abdbcfe3e28eee9ec6ed93247f5cfb9a26 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 18:43:14 -0700 Subject: [PATCH 04/11] ruff --- tests/client/test_notification_response.py | 75 ++++++++-------------- 1 file changed, 26 insertions(+), 49 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index 0b28db96f..c49e6b790 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -26,52 +26,38 @@ def create_non_sdk_server_app() -> Starlette: """Create a minimal server that doesn't follow SDK conventions.""" - + async def handle_mcp_request(request: Request) -> Response: """Handle MCP requests with non-standard responses.""" try: body = await request.body() data = json.loads(body) - + # Handle initialize request normally if data.get("method") == "initialize": response_data = { "jsonrpc": "2.0", "id": data["id"], "result": { - "serverInfo": { - "name": "test-non-sdk-server", - "version": "1.0.0" - }, + "serverInfo": {"name": "test-non-sdk-server", "version": "1.0.0"}, "protocolVersion": "2024-11-05", - "capabilities": {} - } + "capabilities": {}, + }, } return JSONResponse(response_data) - + # For notifications, return 204 No Content (non-SDK behavior) if "id" not in data: - return Response( - status_code=204, - headers={"Content-Type": "application/json"} - ) - + return Response(status_code=204, headers={"Content-Type": "application/json"}) + # Default response for other requests - return JSONResponse({ - "jsonrpc": "2.0", - "id": data.get("id"), - "error": { - "code": -32601, - "message": "Method not found" - } - }) - - except Exception as e: return JSONResponse( - {"error": f"Server error: {str(e)}"}, - status_code=500 + {"jsonrpc": "2.0", "id": data.get("id"), "error": {"code": -32601, "message": "Method not found"}} ) - + + except Exception as e: + return JSONResponse({"error": f"Server error: {str(e)}"}, status_code=500) + app = Starlette( debug=True, routes=[ @@ -105,13 +91,9 @@ def non_sdk_server_port() -> int: @pytest.fixture def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: """Start a non-SDK server for testing.""" - proc = multiprocessing.Process( - target=run_non_sdk_server, - kwargs={"port": non_sdk_server_port}, - daemon=True - ) + proc = multiprocessing.Process(target=run_non_sdk_server, kwargs={"port": non_sdk_server_port}, daemon=True) proc.start() - + # Wait for server to be ready start_time = time.time() while time.time() - start_time < 10: @@ -124,20 +106,17 @@ def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: proc.kill() proc.join(timeout=2) pytest.fail("Server failed to start within 10 seconds") - + yield - + proc.kill() proc.join(timeout=2) @pytest.mark.anyio -async def test_notification_with_204_response( - non_sdk_server: None, - non_sdk_server_port: int -) -> None: +async def test_notification_with_204_response(non_sdk_server: None, non_sdk_server_port: int) -> None: """Test that client handles 204 responses to notifications correctly. - + This test verifies the fix for the issue where non-SDK servers might return 204 No Content for notifications instead of 202 Accepted. The client should handle this gracefully without trying to parse @@ -145,31 +124,29 @@ async def test_notification_with_204_response( """ server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" returned_exception = None - async def message_handler(message: RequestResponder[types.ServerRequest, types.ClientResult] | types.ServerNotification | Exception): + + async def message_handler( + message: RequestResponder[types.ServerRequest, types.ClientResult] | types.ServerNotification | Exception, + ): nonlocal returned_exception if isinstance(message, Exception): returned_exception = message async with streamablehttp_client(server_url) as (read_stream, write_stream, get_session_id): async with ClientSession( - read_stream, + read_stream, write_stream, message_handler=message_handler, ) as session: # Initialize should work normally await session.initialize() - + # Send a notification - this should not raise an error # even though the server returns 204 instead of 202 # Without the fix, this would fail with a JSON parsing error # because the client would try to parse the empty 204 response body await session.send_notification( - ClientNotification( - RootsListChangedNotification( - method="notifications/roots/list_changed", - params={} - ) - ) + ClientNotification(RootsListChangedNotification(method="notifications/roots/list_changed", params={})) ) if returned_exception: From 34c782d3b73810600819dc0360a9ee8572a6b5a6 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 18:44:17 -0700 Subject: [PATCH 05/11] ruff ruff --- tests/client/test_notification_response.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index c49e6b790..ed94b4d41 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -13,7 +13,6 @@ import pytest import uvicorn -from mcp.shared.session import RequestResponder from starlette.applications import Starlette from starlette.requests import Request from starlette.responses import JSONResponse, Response @@ -21,7 +20,8 @@ from mcp import ClientSession, types from mcp.client.streamable_http import streamablehttp_client -from mcp.types import ClientNotification, Implementation, RootsListChangedNotification +from mcp.shared.session import RequestResponder +from mcp.types import ClientNotification, RootsListChangedNotification def create_non_sdk_server_app() -> Starlette: From 8fc8d3f8ff7b18031830d21faaa8c9ea1d2f56a0 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 18:46:32 -0700 Subject: [PATCH 06/11] cleanup comment --- tests/client/test_notification_response.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index ed94b4d41..d66e93bb6 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -141,10 +141,7 @@ async def message_handler( # Initialize should work normally await session.initialize() - # Send a notification - this should not raise an error - # even though the server returns 204 instead of 202 - # Without the fix, this would fail with a JSON parsing error - # because the client would try to parse the empty 204 response body + # The test server returns a 204 instead of the expected 202 await session.send_notification( ClientNotification(RootsListChangedNotification(method="notifications/roots/list_changed", params={})) ) From 9f6f0b15e630da4fd83913133f11fa48b09152e3 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Wed, 20 Aug 2025 18:48:48 -0700 Subject: [PATCH 07/11] pyright --- tests/client/test_notification_response.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index d66e93bb6..2e95fb9b8 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -132,7 +132,7 @@ async def message_handler( if isinstance(message, Exception): returned_exception = message - async with streamablehttp_client(server_url) as (read_stream, write_stream, get_session_id): + async with streamablehttp_client(server_url) as (read_stream, write_stream, _): async with ClientSession( read_stream, write_stream, @@ -143,7 +143,7 @@ async def message_handler( # The test server returns a 204 instead of the expected 202 await session.send_notification( - ClientNotification(RootsListChangedNotification(method="notifications/roots/list_changed", params={})) + ClientNotification(RootsListChangedNotification(method="notifications/roots/list_changed")) ) if returned_exception: From b51026c6880c024b55fa5d3fc1e5bd4fb4fe6d9d Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Thu, 21 Aug 2025 10:25:37 -0700 Subject: [PATCH 08/11] comment abt notifications --- src/mcp/client/streamable_http.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index 0a3d1e4c5..0889c56b4 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -281,6 +281,8 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: content_type = response.headers.get(CONTENT_TYPE, "").lower() + # Per https://modelcontextprotocol.io/specification/2025-06-18/basic#notifications: + # The server MUST NOT send a response to notifications. if isinstance(message.root, JSONRPCRequest): if content_type.startswith(JSON): await self._handle_json_response(response, ctx.read_stream_writer, is_initialization) From 4048bfc0045fc9ed5ec162740888c611fc10e0f8 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Thu, 21 Aug 2025 10:33:53 -0700 Subject: [PATCH 09/11] comment for test --- tests/client/test_notification_response.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index 2e95fb9b8..9e87cc2c0 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -115,12 +115,10 @@ def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: @pytest.mark.anyio async def test_notification_with_204_response(non_sdk_server: None, non_sdk_server_port: int) -> None: - """Test that client handles 204 responses to notifications correctly. - - This test verifies the fix for the issue where non-SDK servers - might return 204 No Content for notifications instead of 202 Accepted. - The client should handle this gracefully without trying to parse - the response body. + """ + This test verifies that the client does not parse responses to non-JsonRPCRequests, which matches the + behavior of the TypeScript SDK. The test uses a 204 No Content (commonly seen from servers), but in reality + any 2xx response should be handled the same way. """ server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" returned_exception = None From c6c2cc5e947d775f63845d9cc45f21f966f420e8 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Fri, 22 Aug 2025 09:10:11 -0700 Subject: [PATCH 10/11] address comments --- src/mcp/client/streamable_http.py | 3 +-- tests/client/test_notification_response.py | 10 ++++++---- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/mcp/client/streamable_http.py b/src/mcp/client/streamable_http.py index 0889c56b4..57df64705 100644 --- a/src/mcp/client/streamable_http.py +++ b/src/mcp/client/streamable_http.py @@ -279,11 +279,10 @@ async def _handle_post_request(self, ctx: RequestContext) -> None: if is_initialization: self._maybe_extract_session_id_from_response(response) - content_type = response.headers.get(CONTENT_TYPE, "").lower() - # Per https://modelcontextprotocol.io/specification/2025-06-18/basic#notifications: # The server MUST NOT send a response to notifications. if isinstance(message.root, JSONRPCRequest): + content_type = response.headers.get(CONTENT_TYPE, "").lower() if content_type.startswith(JSON): await self._handle_json_response(response, ctx.read_stream_writer, is_initialization) elif content_type.startswith(SSE): diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index 9e87cc2c0..718e9bb17 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -114,11 +114,13 @@ def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: @pytest.mark.anyio -async def test_notification_with_204_response(non_sdk_server: None, non_sdk_server_port: int) -> None: +async def test_non_compliant_notification_response(non_sdk_server: None, non_sdk_server_port: int) -> None: """ - This test verifies that the client does not parse responses to non-JsonRPCRequests, which matches the - behavior of the TypeScript SDK. The test uses a 204 No Content (commonly seen from servers), but in reality - any 2xx response should be handled the same way. + This test verifies that the client ignores unexpected responses to notifications: the spec states they should + either be 202 + no response body, or 4xx + optional error body + (https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#sending-messages-to-the-server), + but some servers wrongly return other 2xx codes (e.g. 204). For now we simply ignore unexpected responses + (aligning behaviour w/ the TS SDK). """ server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" returned_exception = None From 954db097c443b1672c5f2afd8e46bdefa5292930 Mon Sep 17 00:00:00 2001 From: Justin Wang Date: Fri, 22 Aug 2025 09:12:42 -0700 Subject: [PATCH 11/11] ruff --- tests/client/test_notification_response.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/client/test_notification_response.py b/tests/client/test_notification_response.py index 718e9bb17..88e64711b 100644 --- a/tests/client/test_notification_response.py +++ b/tests/client/test_notification_response.py @@ -116,11 +116,11 @@ def non_sdk_server(non_sdk_server_port: int) -> Generator[None, None, None]: @pytest.mark.anyio async def test_non_compliant_notification_response(non_sdk_server: None, non_sdk_server_port: int) -> None: """ - This test verifies that the client ignores unexpected responses to notifications: the spec states they should - either be 202 + no response body, or 4xx + optional error body - (https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#sending-messages-to-the-server), - but some servers wrongly return other 2xx codes (e.g. 204). For now we simply ignore unexpected responses - (aligning behaviour w/ the TS SDK). + This test verifies that the client ignores unexpected responses to notifications: the spec states they should + either be 202 + no response body, or 4xx + optional error body + (https://modelcontextprotocol.io/specification/2025-06-18/basic/transports#sending-messages-to-the-server), + but some servers wrongly return other 2xx codes (e.g. 204). For now we simply ignore unexpected responses + (aligning behaviour w/ the TS SDK). """ server_url = f"http://127.0.0.1:{non_sdk_server_port}/mcp" returned_exception = None