Skip to content

test: regression test for initialize hang on unexpected content-type#2472

Open
Christian-Sidak wants to merge 2 commits into
modelcontextprotocol:mainfrom
Christian-Sidak:fix/issue-2432
Open

test: regression test for initialize hang on unexpected content-type#2472
Christian-Sidak wants to merge 2 commits into
modelcontextprotocol:mainfrom
Christian-Sidak:fix/issue-2432

Conversation

@Christian-Sidak

Copy link
Copy Markdown

Summary

The underlying fix is already present on main_handle_post_request sends a proper JSONRPCError (with the matching request ID) when it encounters an unexpected content type, which lets ClientSession resolve the pending request instead of waiting indefinitely. This PR adds the missing test that exercises that code path for the initialize call specifically.

Test plan

  • tests/client/test_notification_response.py::test_initialize_does_not_hang_on_unexpected_content_type — new test passes
  • All existing test_notification_response.py tests continue to pass

Fixes #2432

…ype (modelcontextprotocol#2432)

Add a test that verifies initialize() raises MCPError immediately when
the server returns an unexpected Content-Type (e.g. text/plain) instead
of hanging forever waiting for a response that never arrives.

Fixes modelcontextprotocol#2432
@Christian-Sidak

Copy link
Copy Markdown
Author

Friendly bump -- let me know if anything needs changing.

1 similar comment
@Christian-Sidak

Copy link
Copy Markdown
Author

Friendly bump -- let me know if anything needs changing.

@MukundaKatta

Copy link
Copy Markdown

Does the same fix path also cover a server returning 200 with an empty body, or 200 with valid JSON for a different request id? Both have the same "initialize never resolves" shape, so if the fix is at the right layer they should unblock without new code. If those cases are tested elsewhere already, ignore.

@Christian-Sidak

Copy link
Copy Markdown
Author

No — this test is scoped to the unexpected content-type path only. The hang in issue #2432 is triggered when the SDK reaches the if content_type not in ("application/json", "text/event-stream") guard and raises McpError; the initialize() future is resolved by that error propagation.

The two cases you describe follow different paths:

  • 200 with empty body: content-type would still be application/json (or absent), so it bypasses the content-type check. The body JSON-parse would fail or produce an empty response, which is a separate code path — likely a different hang or error class.
  • 200 with valid JSON for a different request id: the response matcher would never complete the outstanding initialize future since the id doesn't match, so the session would hang differently — at the response-matching layer, not the content-type layer.

Both are valid "initialize never resolves" failure modes but would need separate tests against the relevant code paths. Happy to add them here or in a follow-up PR if that's useful.

@Christian-Sidak

Copy link
Copy Markdown
Author

Friendly bump -- let me know if anything needs changing.

@Christian-Sidak

Copy link
Copy Markdown
Author

Rebased onto current main: the streamable-HTTP test module has since moved from httpx/mcp.types to httpx2/mcp_types (repo-wide rename), so the new regression test needed the same update to avoid a NameError once merged. Content/behavior of the added test is unchanged.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

3 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="tests/client/test_notification_response.py">

<violation number="1" location="tests/client/test_notification_response.py:9">
P1: Test collection now fails because the project provides `httpx`, not `httpx2`. Keep the existing `httpx` import and corresponding `httpx.AsyncClient`/`httpx.ASGITransport` references so this regression test module can run.</violation>

<violation number="2" location="tests/client/test_notification_response.py:10">
P1: This changes SDK type imports to an uninstalled `mcp_types` module, so pytest cannot import this file. Import `types` from `mcp` and `RootsListChangedNotification` from `mcp.types` instead.</violation>

<violation number="3" location="tests/client/test_notification_response.py:146">
P2: When the initialization-hang regression occurs, this test will hang the test run instead of producing a bounded failure, and it does not actually assert the claimed “immediately” behavior. Wrap the `initialize()` await in a short `anyio.fail_after(...)` scope (while retaining `pytest.raises`) so a regression fails deterministically.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic


import httpx
import httpx2
import mcp_types as types

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1: This changes SDK type imports to an uninstalled mcp_types module, so pytest cannot import this file. Import types from mcp and RootsListChangedNotification from mcp.types instead.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/client/test_notification_response.py, line 10:

<comment>This changes SDK type imports to an uninstalled `mcp_types` module, so pytest cannot import this file. Import `types` from `mcp` and `RootsListChangedNotification` from `mcp.types` instead.</comment>

<file context>
@@ -6,17 +6,18 @@
 
-import httpx
+import httpx2
+import mcp_types as types
 import pytest
+from mcp_types import RootsListChangedNotification
</file context>

import json

import httpx
import httpx2

Copy link
Copy Markdown

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, not httpx2. Keep the existing httpx import and corresponding httpx.AsyncClient/httpx.ASGITransport references so this regression test module can run.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/client/test_notification_response.py, line 9:

<comment>Test collection now fails because the project provides `httpx`, not `httpx2`. Keep the existing `httpx` import and corresponding `httpx.AsyncClient`/`httpx.ASGITransport` references so this regression test module can run.</comment>

<file context>
@@ -6,17 +6,18 @@
 import json
 
-import httpx
+import httpx2
+import mcp_types as types
 import pytest
</file context>

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
await session.initialize()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: When the initialization-hang regression occurs, this test will hang the test run instead of producing a bounded failure, and it does not actually assert the claimed “immediately” behavior. Wrap the initialize() await in a short anyio.fail_after(...) scope (while retaining pytest.raises) so a regression fails deterministically.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/client/test_notification_response.py, line 146:

<comment>When the initialization-hang regression occurs, this test will hang the test run instead of producing a bounded failure, and it does not actually assert the claimed “immediately” behavior. Wrap the `initialize()` await in a short `anyio.fail_after(...)` scope (while retaining `pytest.raises`) so a regression fails deterministically.</comment>

<file context>
@@ -116,6 +132,20 @@ async def test_unexpected_content_type_sends_jsonrpc_error() -> None:
+        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
+                    await session.initialize()
+
+
</file context>

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.

Initialize call hangs forever if MCP server does not return a Content-Type: text/plain

2 participants