Skip to content

Conversation

EiffL
Copy link

@EiffL EiffL commented Aug 28, 2025

Propagates transport disconnects (e.g. server killed mid tool call) as CONNECTION_CLOSED errors instead of hanging indefinitely. Adds regression test.

Motivation and Context

A call_tool over the StreamableHTTP transport would hang forever if the server closed the SSE/HTTP connection mid-response (e.g. process crash/kill). The underlying RemoteProtocolError was logged but in‑flight requests never completed. This change converts that low-level failure into a deterministic JSON-RPC error for every pending request, matching expected resilience semantics.

How Has This Been Tested?

  • Added integrated test test_streamable_http_mid_call_disconnect:
    • Starts real server process.
    • Invokes long-running tool (wait_for_lock_with_notification).
    • Kills server mid-call.
    • Asserts an McpError with code CONNECTION_CLOSED (previously would hang).
  • All existing tests pass locally.

Breaking Changes

None. Behavior only changes for abnormal transport termination; normal flows unaffected. No public API changes.

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

Implementation details:

In _handle_sse_response (client transport) on exception: send the exception into the session read stream then close the writer to signal termination.

In BaseSession._receive_loop: on receiving an Exception, broadcast a synthesized JSONRPCError (code CONNECTION_CLOSED) to all pending request streams and clear them.

Integrated test placed alongside existing StreamableHTTP tests for consistency. Potential follow-up: introduce optional reconnection/resumption logic for recoverable disconnects.

EiffL and others added 2 commits August 29, 2025 01:23
fix: handle transport exceptions and ensure proper cleanup of in-flig…
@EiffL EiffL requested a review from a team as a code owner August 28, 2025 23:53
@EiffL EiffL requested a review from felixweinberger August 28, 2025 23:53
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.

1 participant