-
Notifications
You must be signed in to change notification settings - Fork 3.7k
test: regression test for initialize hang on unexpected content-type #2472
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
Christian-Sidak
wants to merge
3
commits into
modelcontextprotocol:main
Choose a base branch
from
Christian-Sidak:fix/issue-2432
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+107
−9
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
34dc68a
test: add regression test for initialize hang on unexpected content-t…
b5ddbf4
Update httpx/mcp.types references to match renamed httpx2/mcp_types o…
Christian-Sidak 5ef8194
test: bound initialize() with anyio.fail_after to fail fast on regres…
Christian-Sidak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,17 +6,19 @@ | |
|
|
||
| import json | ||
|
|
||
| import httpx | ||
| import anyio | ||
| import httpx2 | ||
| import mcp_types as types | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P1: This changes SDK type imports to an uninstalled Prompt for AI agents |
||
| import pytest | ||
| from mcp_types import RootsListChangedNotification | ||
| from starlette.applications import Starlette | ||
| from starlette.requests import Request | ||
| from starlette.responses import JSONResponse, Response | ||
| from starlette.routing import Route | ||
|
|
||
| from mcp import ClientSession, MCPError, types | ||
| from mcp import ClientSession, MCPError | ||
| from mcp.client.streamable_http import streamable_http_client | ||
| from mcp.shared.session import RequestResponder | ||
| from mcp.types import RootsListChangedNotification | ||
|
|
||
| pytestmark = pytest.mark.anyio | ||
|
|
||
|
|
@@ -71,6 +73,21 @@ async def handle_mcp_request(request: Request) -> Response: | |
| return Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])]) | ||
|
|
||
|
|
||
| def _create_plain_text_server_app() -> Starlette: | ||
| """Create a server that returns text/plain for all requests, including initialize. | ||
|
|
||
| This reproduces the scenario from issue #2432 where a misconfigured server | ||
| returns an unexpected content type on the initialize call, causing the client | ||
| to hang forever. | ||
| """ | ||
|
|
||
| async def handle_mcp_request(request: Request) -> Response: | ||
| # Always return text/plain — never a valid MCP response. | ||
| return Response(content="this is not json", status_code=200, media_type="text/plain") | ||
|
|
||
| return Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])]) | ||
|
|
||
|
|
||
| async def test_non_compliant_notification_response() -> None: | ||
| """Verify the client ignores unexpected responses to notifications. | ||
|
|
||
|
|
@@ -88,7 +105,7 @@ async def message_handler( # pragma: no cover | |
| if isinstance(message, Exception): | ||
| returned_exception = message | ||
|
|
||
| async with httpx.AsyncClient(transport=httpx.ASGITransport(app=_create_non_sdk_server_app())) as client: | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=_create_non_sdk_server_app())) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream, message_handler=message_handler) as session: | ||
| await session.initialize() | ||
|
|
@@ -107,7 +124,7 @@ async def test_unexpected_content_type_sends_jsonrpc_error() -> None: | |
| the client should send a JSONRPCError so the pending request resolves immediately | ||
| instead of hanging until timeout. | ||
| """ | ||
| async with httpx.AsyncClient(transport=httpx.ASGITransport(app=_create_unexpected_content_type_app())) as client: | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=_create_unexpected_content_type_app())) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
|
|
@@ -116,6 +133,21 @@ async def test_unexpected_content_type_sends_jsonrpc_error() -> None: | |
| await session.list_tools() | ||
|
|
||
|
|
||
| async def test_initialize_does_not_hang_on_unexpected_content_type() -> None: | ||
| """Verify that initialize() raises MCPError immediately when server returns wrong content type. | ||
|
|
||
| Regression test for issue #2432: when a misconfigured server returns a content type | ||
| other than application/json or text/event-stream in response to the initialize request, | ||
| the client must raise MCPError right away instead of hanging forever. | ||
| """ | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=_create_plain_text_server_app())) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| with pytest.raises(MCPError, match="Unexpected content type: text/plain"): # pragma: no branch | ||
| with anyio.fail_after(5): | ||
| await session.initialize() | ||
|
|
||
|
|
||
| def _create_http_error_app(error_status: int, *, error_on_notifications: bool = False) -> Starlette: | ||
| """Create a server that returns an HTTP error for non-init requests.""" | ||
|
|
||
|
|
@@ -141,9 +173,9 @@ async def test_http_error_status_sends_jsonrpc_error() -> None: | |
|
|
||
| When a server returns a non-2xx status code (e.g. 500), the client should | ||
| send a JSONRPCError so the pending request resolves immediately instead of | ||
| raising an unhandled httpx.HTTPStatusError that causes the caller to hang. | ||
| raising an unhandled httpx2.HTTPStatusError that causes the caller to hang. | ||
| """ | ||
| async with httpx.AsyncClient(transport=httpx.ASGITransport(app=_create_http_error_app(500))) as client: | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=_create_http_error_app(500))) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
|
|
@@ -159,7 +191,7 @@ async def test_http_error_on_notification_does_not_hang() -> None: | |
| unblock, so the client should just return without sending a JSONRPCError. | ||
| """ | ||
| app = _create_http_error_app(500, error_on_notifications=True) | ||
| async with httpx.AsyncClient(transport=httpx.ASGITransport(app=app)) as client: | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
|
|
@@ -194,10 +226,76 @@ async def test_invalid_json_response_sends_jsonrpc_error() -> None: | |
| should send a JSONRPCError so the pending request resolves immediately | ||
| instead of hanging until timeout. | ||
| """ | ||
| async with httpx.AsyncClient(transport=httpx.ASGITransport(app=_create_invalid_json_response_app())) as client: | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=_create_invalid_json_response_app())) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
|
|
||
| with pytest.raises(MCPError, match="Failed to parse JSON response"): # pragma: no branch | ||
| await session.list_tools() | ||
|
|
||
|
|
||
| def _create_non_2xx_json_body_app(status: int, body: bytes) -> Starlette: | ||
| """Server that returns a fixed non-2xx status + ``application/json`` body for non-init requests. | ||
|
|
||
| The initialize response carries an ``mcp-session-id`` so the client treats subsequent | ||
| requests as part of an established session (needed for the 404 → session-terminated mapping). | ||
| """ | ||
|
|
||
| async def handle_mcp_request(request: Request) -> Response: | ||
| data = json.loads(await request.body()) | ||
| if data.get("method") == "initialize": | ||
| return JSONResponse( | ||
| {"jsonrpc": "2.0", "id": data["id"], "result": INIT_RESPONSE}, | ||
| headers={"mcp-session-id": "test-session"}, | ||
| ) | ||
| if "id" not in data: | ||
| return Response(status_code=202) | ||
| return Response(content=body, status_code=status, media_type="application/json") | ||
|
|
||
| return Starlette(debug=True, routes=[Route("/mcp", handle_mcp_request, methods=["POST"])]) | ||
|
|
||
|
|
||
| async def test_client_surfaces_jsonrpc_error_from_non_2xx_body_with_correlated_id() -> None: | ||
| """SDK-defined: a JSON-RPC error in a non-2xx body is surfaced verbatim even when the | ||
| server set ``id: null`` — the client rewraps it under the pending request's id, so | ||
| the awaiting call resolves with the server's error code instead of the generic fallback.""" | ||
| body = json.dumps( | ||
| {"jsonrpc": "2.0", "id": None, "error": {"code": types.METHOD_NOT_FOUND, "message": "nope"}} | ||
| ).encode() | ||
| app = _create_non_2xx_json_body_app(400, body) | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
| with pytest.raises(MCPError) as exc: | ||
| await session.list_tools() | ||
| assert exc.value.error.code == types.METHOD_NOT_FOUND | ||
|
|
||
|
|
||
| async def test_client_falls_back_to_generic_error_when_non_2xx_body_is_a_jsonrpc_result() -> None: | ||
| """SDK-defined: a non-2xx response whose JSON body parses as a JSON-RPC *result* (not an | ||
| error) falls through to the generic ``INTERNAL_ERROR`` fallback rather than being | ||
| treated as the request's reply.""" | ||
| app = _create_non_2xx_json_body_app(400, b'{"jsonrpc":"2.0","id":1,"result":{}}') | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
| with pytest.raises(MCPError) as exc: | ||
| await session.list_tools() | ||
| assert exc.value.error.code == types.INTERNAL_ERROR | ||
|
|
||
|
|
||
| async def test_client_falls_back_to_session_terminated_when_404_body_is_malformed_json() -> None: | ||
| """SDK-defined: an unparseable ``application/json`` body on a 404 response is swallowed | ||
| and the status-derived ``INVALID_REQUEST`` (session-terminated) fallback resolves the | ||
| pending request — the parse failure never propagates.""" | ||
| app = _create_non_2xx_json_body_app(404, b"not valid json{{{") | ||
| async with httpx2.AsyncClient(transport=httpx2.ASGITransport(app=app)) as client: | ||
| async with streamable_http_client("http://localhost/mcp", http_client=client) as (read_stream, write_stream): | ||
| async with ClientSession(read_stream, write_stream) as session: # pragma: no branch | ||
| await session.initialize() | ||
| with pytest.raises(MCPError) as exc: | ||
| await session.list_tools() | ||
| assert exc.value.error.code == types.INVALID_REQUEST | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P1: Test collection now fails because the project provides
httpx, nothttpx2. Keep the existinghttpximport and correspondinghttpx.AsyncClient/httpx.ASGITransportreferences so this regression test module can run.Prompt for AI agents