Skip to content

fix: isolate streamable HTTP POST errors - #2613

Closed
pragnyanramtha wants to merge 2 commits into
modelcontextprotocol:v1.xfrom
pragnyanramtha:pragnyan/v1x-2604-post-error-isolation
Closed

fix: isolate streamable HTTP POST errors#2613
pragnyanramtha wants to merge 2 commits into
modelcontextprotocol:v1.xfrom
pragnyanramtha:pragnyan/v1x-2604-post-error-isolation

Conversation

@pragnyanramtha

Copy link
Copy Markdown

Summary

  • convert non-404 POST HTTP status failures into request-scoped JSON-RPC errors for JSON-RPC requests
  • avoid synthesizing responses for notification/no-id messages
  • add a regression test proving one failed concurrent POST request no longer cancels an unrelated sibling request

Fixes#2604.

This targets v1.x because the released 1.x branch still raises through response.raise_for_status() for POST HTTP errors. main already has equivalent scoped >=400 handling; this PR backports that behavior and adds the concurrency regression coverage for the reported failure mode.

Validation

  • uv run --frozen pytest tests/shared/test_streamable_http.py::test_streamable_http_client_http_error_does_not_cancel_concurrent_request
  • uv run --frozen pytest tests/shared/test_streamable_http.py -> 52 passed
  • uv run --frozen pytest tests/shared/test_streamable_http.py::test_streamable_http_client_http_error_does_not_cancel_concurrent_request tests/shared/test_streamable_http.py::test_streamable_http_client_session_termination tests/shared/test_streamable_http.py::test_streamable_http_client_session_termination_204 tests/client/test_notification_response.py::test_non_compliant_notification_response -> 4 passed
  • uv run --frozen ruff check src/mcp/client/streamable_http.py tests/shared/test_streamable_http.py
  • uv sync --frozen --all-extras --dev
  • uv run --frozen pyright -> 0 errors
  • git diff --check

CopilotAI review requested due to automatic review settings May 16, 2026 00:47

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

Pull request overview

Backports main’s request-scoped handling of HTTP POST failures for the Streamable HTTP transport in the v1.x line, preventing a single request’s non-2xx POST response from escaping the per-request task and tearing down concurrent sibling requests.

Changes:

  • Convert POST HTTP >=400 (non-404) responses into request-scoped JSON-RPC errors for JSON-RPC requests (and avoid synthesizing responses for notifications).
  • Add a concurrency regression test ensuring one failing POST does not cancel an unrelated in-flight request.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

FileDescription
src/mcp/client/streamable_http.pyScopes POST HTTP error handling to the originating JSON-RPC request instead of raising through the shared task group.
tests/shared/test_streamable_http.pyAdds a regression test covering concurrent requests where one receives an HTTP 400 and the other still completes successfully.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment threadsrc/mcp/client/streamable_http.py
Comment threadtests/shared/test_streamable_http.py
@pragnyanramtha
pragnyanramthaforce-pushed the pragnyan/v1x-2604-post-error-isolation branch 3 times, most recently from 8d17351 to ab917f9CompareMay 16, 2026 01:05
@pragnyanramtha
pragnyanramthaforce-pushed the pragnyan/v1x-2604-post-error-isolation branch from ab917f9 to cca10c2CompareMay 16, 2026 01:12
@BossChaos

This comment was marked as abuse.

@BossChaos

This comment was marked as abuse.

@Kludex

Copy link
Copy Markdown
Member

You've opened a duplicated pull request, please search opened PRs before creating new ones. Duplicated from #1998.

@KludexKludex closed this Jun 25, 2026
@modelcontextprotocolmodelcontextprotocol locked as resolved and limited conversation to collaborators Jun 25, 2026
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pragnyanramtha@BossChaos@Kludex