Repository navigation
Keep HTTP requests separate during cancellation - #298
EhsanAzish80 wants to merge 6 commits into
Conversation
Signed-off-by: King Star <mcxin.y@gmail.com>
…essHTTPServerTransport A request cancelled via notifications/cancelled correctly receives no JSON-RPC response from Server, but nothing resumed the transport's HTTP response waiter, so the original POST hung until transport termination. The transport now detects notifications/cancelled, forwards it to the Server unchanged (its existing cancellation path still cancels the handler task), and resumes the matching waiter with a synthesized JSON-RPC error response (code -32002, "Request cancelled"), satisfying the Streamable HTTP requirement that a request POST receive one JSON object. Unknown, already-completed, and requestId-less cancellations are ignored per spec. Fixes modelcontextprotocol#255 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The updated head still completes a cancelled MCP request by synthesizing a JSON-RPC error: AnyMethod.response(
id: waiter.originalID,
error: .serverError(code: -32800, message: message)
)That looks inconsistent with the cancellation contract. For I agree the HTTP exchange itself must not hang, but those are separable requirements. The transport can terminate the POST without producing a JSON-RPC response associated with the cancelled request — e.g. 204/no body when nothing has been committed — while still forwarding the cancellation so the handler stops. Could AI-assisted review; re-checked this against the updated 2c3e81b head before posting. |
|
I pushed a follow-up for the cancellation review: the original POST now ends as a 200 empty SSE stream, without a synthetic JSON-RPC error. This changes our earlier -32800 choice, so I’d like a maintainer call on the wire behavior before merge. All 565 Swift tests passed locally, plus a real HTTP client/server cancellation check. AI helped write the new tests; I ran them. Could a maintainer review this and approve CI? |
…ests without a reply (upstream modelcontextprotocol#298, EhsanAzish80) Supersedes the contributions of modelcontextprotocol#264, modelcontextprotocol#260 and modelcontextprotocol#270 (manifest entries 1-3), which modelcontextprotocol#298 combines. Conflicts resolved in favour of modelcontextprotocol#298 in HTTPServerTypes.swift, StatelessHTTPServerTransport.swift, Server.swift, CancellationTests.swift and HTTPServerTransportTests.swift. Fork changes re-applied on top: - modelcontextprotocol#258's Server.init description/websiteUrl/icons parameters - modelcontextprotocol#221's MockTransport receiveCallCount / finishStream - entry 1a's collision regression tests and their makeStatelessPOSTRequest authorization overload Behavior: a cancelled original POST now ends as HTTP 200 with an empty text/event-stream and no JSON-RPC body (previously -32002 from modelcontextprotocol#260). Follow-up on when to use -32800: fork issue #41. Full suite: 589 tests in 43 suites passed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ede entries 1-3 Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…rder Entry 3a broke two conventions: letter suffixes mark fork additions, and the Order column claims merge order, in which modelcontextprotocol#298 comes after modelcontextprotocol#258. The maintenance procedure now says to skip superseded rows. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
298-delta: bring stateless HTTP cancellation to upstream modelcontextprotocol#298's behavior
This brings together #264, #260, and #270. Stateless HTTP requests get separate internal IDs, while each response keeps the client's original ID. Cancellation reaches the matching handler before the HTTP exchange ends, and a late handler result cannot send a second response.
Following the review discussion, an ambiguous cancellation ID leaves the active requests alone. A duplicate outstanding ID is rejected only when it is a genuine conflict. For the 2025 HTTP path, the cancellation POST returns 202 and the original POST ends as HTTP 200 with an empty SSE stream, without a JSON-RPC response. This wire behavior needs maintainer review before merge. This change does not add a default SDK timeout.
The HTTP adapter owns the result when it cancels an exchange, including timeouts and disconnects. In-flight requests ended by server shutdown now complete with HTTP 503 instead of a generic 500.
Local checks passed: 565 Swift tests in 41 suites, plus a real HTTP loopback check for cancellation, cleanup, and server shutdown. CI is awaiting maintainer approval on this revision.
Tests were written with AI help.