Skip to content

[v1.x] Resolve a failed SSE message POST instead of hanging the caller - #3785

Closed
Mart22052006 wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
Mart22052006:fix/v1.x-sse-post-error
Closed

Mart22052006 wants to merge 1 commit into
modelcontextprotocol:v1.xfrom
Mart22052006:fix/v1.x-sse-post-error

Conversation

@Mart22052006

@Mart22052006 Mart22052006 commented Oct 28, 2026 •

Copy link
Copy Markdown

Fixes #2110 (the SSE part, on v1.x; #3278 covers main)

Backports the SSE half of #3278 to v1.x: a failed message POST now resolves its request with a JSON-RPC error instead of leaving the caller hanging.

Motivation and Context

sse_client's post_writer called raise_for_status() on every message POST and swallowed the error in a catch-all. A request whose POST failed (a 429 from a rate limiter, a 401, a refused connection) was never resolved: its caller waited for its own timeout, or forever without one. The write loop died with it, so every later message was dropped. We hit this in production: a 429 from a rate limiter in front of an MCP server left an agent's tool call waiting 90 minutes for an unrelated outer timeout.

How Has This Been Tested?

10 new tests in tests/shared/test_sse.py, all of which fail on unpatched v1.x (7 hang, 3 end with a closed stream or the wrong error):

  • non-2xx answers (302, 401, 429, 500) and a refused connection reach the caller, and the session keeps working;
  • a 404 maps to "Session terminated" only while the endpoint carries a session id;
  • a failed notification is only logged;
  • a server request answered while a request's POST is failing (the test times out if the error is delivered inline);
  • a failure whose error can no longer be delivered, after the SSE stream ended, is contained.

The full suite passes with 100% coverage, along with pyright and ruff. The SSE tests also pass on 3.10 with lowest-direct deps and on 3.14.

Breaking Changes

None. A caller that used to hang now gets an McpError, and later messages keep flowing.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I am assigned to the linked issue (or it is labeled help wanted, or I'm a maintainer): asked on HTTP transport swallows non-2xx status codes causing client to hang #2110
  • I have disclosed any AI assistance and can explain the change in my own words
  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

The outcomes match #3278 on main: a non-2xx status gives INTERNAL_ERROR "Server returned an error response", or INVALID_REQUEST "Session terminated" for a 404 while the endpoint carries a session id; an error before any response gives CONNECTION_CLOSED "Failed to send message: …".

One v1.x-specific difference: the error is delivered from a task of its own. v1.x ClientSession answers server requests inline in its receive loop, so a reply can be waiting on the write stream just as the error needs the receive loop, and delivering it from the write loop deadlocks.

AI assistance: written with Claude Code, as the commit's Co-Authored-By trailer notes.

The SSE client's post_writer called raise_for_status() on every message POST
and swallowed the error, so a request whose POST failed (a 429 from a rate
limiter, a 401, a refused connection) was never resolved: its caller waited
for its own timeout, or forever without one. The write loop died with it, so
every later message was dropped.

A failed POST now resolves its request with a JSON-RPC error, with the
outcomes modelcontextprotocol#3278 gives on main: a non-2xx status gives INTERNAL_ERROR, or
INVALID_REQUEST "Session terminated" for a 404 while the endpoint carries a
session id; an error before any response gives CONNECTION_CLOSED. A failed
notification is only logged, and the write loop keeps going.

Unlike on main, the error is delivered from a task of its own: v1.x
ClientSession answers server requests inline in its receive loop, so a reply
can be waiting on the write stream just as the error needs the receive loop,
and delivering it from the write loop deadlocks. A test covers that
interleaving.

Github-Issue: modelcontextprotocol#2110
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added the missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md) label Oct 28, 2026
@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

This PR has been closed automatically. This repo only keeps pull requests open when they come from a maintainer, or from a contributor a maintainer has assigned to the linked issue, and you aren't currently assigned to #2110.

If a maintainer assigns you to #2110, this PR reopens on its own and there's nothing more you need to do here. Assignment is a maintainer call based on capacity; comments that only ask to be assigned don't factor in. What does help is engaging on the issue itself by confirming the repro, explaining why it matters for your use case, or describing the approach you'd take.

You're welcome to keep pushing commits here (just avoid force-pushing, since GitHub can't reopen a rewritten branch), but that on its own won't get the PR reviewed or the issue assigned, and realistically most auto-closed PRs stay closed. There's no need to open a new PR either way.

CONTRIBUTING.md has the full reasoning, but in short:

  • We're a small team with very little capacity to review community PRs right now.
  • Many recent PRs are AI-generated with little human review, and reviewing one carefully still costs a maintainer as much time as it ever did. A well-described issue is usually more useful to us than the code.

Maintainers: reopen, remove missing-issue-link, or add bypass-issue-check to override.

@github-actions github-actions Bot closed this Oct 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

missing-issue-link Auto-closed: PR needs a linked issue assigned to its author (see CONTRIBUTING.md)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant