Skip to content

fix(streamable-http-client): handle error responses to initialize - #1361

Merged
DaleSeo merged 3 commits into
modelcontextprotocol:mainfrom
gocamille:fix/initialize-error-response
Oct 15, 2026
Merged

DaleSeo merged 3 commits into
modelcontextprotocol:mainfrom
gocamille:fix/initialize-error-response

Conversation

@gocamille

Copy link
Copy Markdown
Contributor

Fixes #1360

expect_initialized now stops draining an SSE response on an error as well as a result, so the error reaches the service layer and serve() returns ClientInitializeError::JsonRpcError, the same as it already does for application/json.

perform_reinitialization now returns an error when the re-sent initialize gets an error reply, before it sends notifications/initialized. The worker's existing recovery-failure path then fails the pending requests, so nothing is retried without a session. I reused UnexpectedServerResponse for this rather than adding a variant, since recovery errors reach callers as TransportSend anyway and the message carries the server's code and text.

The new tests in test_streamable_http_initialize_error.rs use an axum mock server:

  • initialize error over SSE, then the stream ends
  • initialize error over SSE, stream held open
  • re-initialize answered with a JSON error
  • re-initialize answered with an SSE error (stream held open)

The two re-init tests check the full list of POSTs the server receives, so they fail if notifications/initialized or the retried request goes out after the error.

Validation

  • All four new tests fail on main and pass with the change.
  • cargo test -p rmcp --all-features: passed.
  • cargo clippy --all-targets --all-features -- -D warnings and cargo +nightly fmt --all -- --check: clean.

expect_initialized only stopped draining an SSE response on a result, so
an error event for initialize was skipped. serve() then reported
ConnectionClosed, or hung if the server kept the stream open.

perform_reinitialization also ignored an error reply and went on to send
notifications/initialized and retry the expired request without a
session. It now returns the error, which fails the pending requests.

Fixes modelcontextprotocol#1360
@gocamille
gocamille requested a review from a team as a code owner October 14, 2026 19:09
@github-actions github-actions Bot added T-dependencies Dependencies related changes T-test Testing related changes T-config Configuration file changes T-core Core library changes T-transport Transport layer changes labels Oct 14, 2026
@github-actions
github-actions Bot requested a review from bolinfest October 14, 2026 19:10
@github-actions

Copy link
Copy Markdown
Contributor

@bolinfest is on triage rotation this week and is the reviewer for this one.

Labels come from the PR labeler. Please review the change.

Automated triage. Labels are a first pass and the assigned maintainer has the final say.

DaleSeo
DaleSeo previously approved these changes Oct 15, 2026

@DaleSeo DaleSeo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix, @gocamille! I only have a minor suggession.

Comment thread crates/rmcp/src/transport/streamable_http_client.rs Outdated
Comment thread crates/rmcp/Cargo.toml
@DaleSeo
DaleSeo merged commit 5d9935d into modelcontextprotocol:main Oct 15, 2026
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-config Configuration file changes T-core Core library changes T-dependencies Dependencies related changes T-test Testing related changes T-transport Transport layer changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

streamable-http client: an error response to initialize is lost or ignored (SSE, session re-init)

2 participants