Skip to content

test: add tests for non-2xx HTTP status handling (fixes #3091)#3093

Open
zsxh1990 wants to merge 4 commits into
modelcontextprotocol:mainfrom
zsxh1990:test/non-2xx-status-handling
Open

test: add tests for non-2xx HTTP status handling (fixes #3091)#3093
zsxh1990 wants to merge 4 commits into
modelcontextprotocol:mainfrom
zsxh1990:test/non-2xx-status-handling

Conversation

@zsxh1990

Copy link
Copy Markdown

Summary

Add tests verifying that StreamableHTTPTransport._handle_post_request properly handles non-2xx HTTP status codes.

Tests Added

  • 401 Unauthorized → JSONRPCError with request ID (not timeout)
  • 403 Forbidden → JSONRPCError with request ID
  • 500 Internal Server Error → JSONRPCError with request ID
  • JSON error body → Parsed and used directly

Verification

The current implementation already handles non-2xx correctly (lines 342-373 in streamable_http.py). These tests document the expected behavior and prevent regressions.

Key Behavior

  • Non-2xx status → JSONRPCError sent to caller with original request ID
  • JSON-RPC error body → Parsed and forwarded directly
  • Non-JSON body → Fallback to generic INTERNAL_ERROR

Closes #3091

/claim #3091

…rotocol#3091)

Verify that StreamableHTTPTransport._handle_post_request properly
handles 401/403/5xx by sending JSONRPCError to the caller instead
of letting the request timeout.

Tests:
- 401 → JSONRPCError with INTERNAL_ERROR code
- 403 → JSONRPCError with INTERNAL_ERROR code
- 500 → JSONRPCError with INTERNAL_ERROR code
- JSON error body → parsed and used directly

Closes modelcontextprotocol#3091

@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.

1 issue 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_non2xx_status.py">

<violation number="1" location="tests/client/test_non2xx_status.py:27">
P1: These tests fail before exercising status handling because `AsyncMock.stream()` returns a coroutine, but `_handle_post_request` uses `client.stream()` directly as an async context manager. Making `client` a `MagicMock` lets its `stream` method return the configured `mock_stream_ctx`, matching `httpx.AsyncClient.stream()`'s interface.</violation>
</file>

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

Re-trigger cubic

Comment thread tests/client/test_non2xx_status.py Outdated
ctx.session_message.message = MagicMock(spec=JSONRPCRequest)
ctx.session_message.message.id = request_id
ctx.read_stream_writer = AsyncMock()
ctx.client = AsyncMock()

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: These tests fail before exercising status handling because AsyncMock.stream() returns a coroutine, but _handle_post_request uses client.stream() directly as an async context manager. Making client a MagicMock lets its stream method return the configured mock_stream_ctx, matching httpx.AsyncClient.stream()'s interface.

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

<comment>These tests fail before exercising status handling because `AsyncMock.stream()` returns a coroutine, but `_handle_post_request` uses `client.stream()` directly as an async context manager. Making `client` a `MagicMock` lets its `stream` method return the configured `mock_stream_ctx`, matching `httpx.AsyncClient.stream()`'s interface.</comment>

<file context>
@@ -0,0 +1,146 @@
+        ctx.session_message.message = MagicMock(spec=JSONRPCRequest)
+        ctx.session_message.message.id = request_id
+        ctx.read_stream_writer = AsyncMock()
+        ctx.client = AsyncMock()
+        ctx.metadata = None
+        return ctx
</file context>

zsxh1990 added 3 commits July 14, 2026 20:44
- SessionMessage is in mcp.shared.message, not mcp_types
- Format with ruff
Use StreamableHTTPTransport(url) instead of __new__() to avoid
AttributeError on uninitialized attributes.
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.

streamable_http: HTTP status errors (401/403/5xx) on tools/list, tools/call never reach the caller — indistinguishable from a timeout

1 participant