Skip to content

Commit 14fc2f3

Browse files
committed
fix(transport): cancel in-flight request on streamable-http client disconnect (#857)
When a streamable-HTTP client closes its TCP connection while a tool handler is still running, the request-wise SSE response stream is dropped, but nothing fired the per-request cancellation token. Only a natural response completion or an explicit notifications/cancelled cancelled it, so a long-running handler ran to completion after the client had already disconnected. Wrap the request-wise response stream in a drop-guard. When the stream is dropped before completing (client disconnect), it injects a synthetic notifications/cancelled for the request, reusing the existing cancellation path so the service fires the handler's RequestContext::ct. Streams that end normally (response delivered, or a reconnection handoff) do not cancel. Adds a regression test covering both the disconnect-cancels and the normal-completion paths. No public API change.
1 parent 9e3de34 commit 14fc2f3

2 files changed

Lines changed: 314 additions & 4 deletions

File tree

crates/rmcp/src/transport/streamable_http_server/session/local.rs

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use std::{
22
collections::{HashMap, HashSet, VecDeque},
33
num::ParseIntError,
4+
pin::Pin,
5+
task::{Context, Poll},
46
time::{Duration, Instant},
57
};
68

@@ -16,9 +18,10 @@ use tracing::instrument;
1618
use crate::{
1719
RoleServer,
1820
model::{
19-
CancelledNotificationParam, ClientJsonRpcMessage, ClientNotification, ClientRequest,
20-
JsonRpcNotification, JsonRpcRequest, Notification, ProgressNotificationParam,
21-
ProgressToken, RequestId, ServerJsonRpcMessage, ServerNotification,
21+
CancelledNotification, CancelledNotificationMethod, CancelledNotificationParam,
22+
ClientJsonRpcMessage, ClientNotification, ClientRequest, JsonRpcNotification,
23+
JsonRpcRequest, Notification, ProgressNotificationParam, ProgressToken, RequestId,
24+
ServerJsonRpcMessage, ServerNotification,
2225
},
2326
transport::{
2427
WorkerTransport,
@@ -92,6 +95,13 @@ impl SessionManager for LocalSessionManager {
9295
let handle = sessions
9396
.get(id)
9497
.ok_or(LocalSessionManagerError::SessionNotFound(id.clone()))?;
98+
// Remember the request id so the in-flight request can be cancelled if
99+
// the client disconnects before its response is delivered (issue #857).
100+
// Only requests reach `create_stream`; other message kinds carry no id.
101+
let request_id = match &message {
102+
ClientJsonRpcMessage::Request(request) => Some(request.id.clone()),
103+
_ => None,
104+
};
95105
let receiver = handle.establish_request_wise_channel().await?;
96106
let http_request_id = receiver.http_request_id;
97107
handle.push_message(message, http_request_id).await?;
@@ -103,7 +113,8 @@ impl SessionManager for LocalSessionManager {
103113
};
104114
ServerSseMessage::priming(event_id, retry)
105115
});
106-
Ok(futures::stream::iter(priming).chain(ReceiverStream::new(receiver.inner)))
116+
let stream = futures::stream::iter(priming).chain(ReceiverStream::new(receiver.inner));
117+
Ok(CancelOnDisconnect::new(stream, handle.clone(), request_id))
107118
}
108119

109120
async fn create_standalone_stream(
@@ -159,6 +170,91 @@ impl SessionManager for LocalSessionManager {
159170
}
160171
}
161172

173+
/// SSE response body wrapper that cancels the in-flight request when the client
174+
/// disconnects before the response is delivered.
175+
///
176+
/// The streamable-HTTP server returns a request-wise SSE stream as the HTTP
177+
/// response body for each request. If the client drops the connection while a
178+
/// handler is still running (Ctrl-C, network drop, read timeout), the runtime
179+
/// drops this stream. Nothing else feeds that drop back into the per-request
180+
/// cancellation token (`RequestContext::ct`), so a long-running handler would
181+
/// otherwise run to completion. This guard detects the drop-before-completion
182+
/// and injects a synthetic `notifications/cancelled` for the request, reusing
183+
/// the same path as an explicit client cancellation.
184+
///
185+
/// See <https://github.com/modelcontextprotocol/rust-sdk/issues/857>.
186+
struct CancelOnDisconnect<S> {
187+
inner: S,
188+
handle: LocalSessionHandle,
189+
/// The request id to cancel on disconnect, taken once a cancellation is
190+
/// issued. `None` when there is nothing to cancel (non-request messages).
191+
request_id: Option<RequestId>,
192+
/// Set once the inner stream terminates (response delivered, or the stream
193+
/// was closed for a reconnection handoff), so drop does not cancel a request
194+
/// that already completed.
195+
completed: bool,
196+
}
197+
198+
impl<S> CancelOnDisconnect<S> {
199+
fn new(inner: S, handle: LocalSessionHandle, request_id: Option<RequestId>) -> Self {
200+
Self {
201+
inner,
202+
handle,
203+
request_id,
204+
completed: false,
205+
}
206+
}
207+
}
208+
209+
impl<S: Stream<Item = ServerSseMessage> + Unpin> Stream for CancelOnDisconnect<S> {
210+
type Item = ServerSseMessage;
211+
212+
fn poll_next(self: Pin<&mut Self>, cx: &mut Context<'_>) -> Poll<Option<Self::Item>> {
213+
let this = self.get_mut();
214+
let poll = Pin::new(&mut this.inner).poll_next(cx);
215+
if matches!(poll, Poll::Ready(None)) {
216+
// Stream ended normally: the response was delivered (the worker
217+
// closes the request-wise sender on completion) or the stream was
218+
// handed off for reconnection. Either way, do not cancel.
219+
this.completed = true;
220+
}
221+
poll
222+
}
223+
}
224+
225+
impl<S> Drop for CancelOnDisconnect<S> {
226+
fn drop(&mut self) {
227+
if self.completed {
228+
return;
229+
}
230+
let Some(request_id) = self.request_id.take() else {
231+
return;
232+
};
233+
// The response stream was dropped before completing, i.e. the client
234+
// disconnected. Inject a synthetic cancellation so the service fires the
235+
// request's `RequestContext::ct`, mirroring an explicit
236+
// `notifications/cancelled` from the client.
237+
let cancelled = CancelledNotification {
238+
params: CancelledNotificationParam::new(
239+
Some(request_id),
240+
Some("client disconnected".to_string()),
241+
),
242+
method: CancelledNotificationMethod,
243+
extensions: Default::default(),
244+
};
245+
let message =
246+
ClientJsonRpcMessage::notification(ClientNotification::CancelledNotification(cancelled));
247+
// `Drop` cannot await; deliver on the session's control channel without
248+
// blocking. If the channel is momentarily full the request is not
249+
// cancelled early, but the session keep-alive/idle timeout still reaps
250+
// it — no worse than before this fix.
251+
let _ = self.handle.event_tx.try_send(SessionEvent::ClientMessage {
252+
message,
253+
http_request_id: None,
254+
});
255+
}
256+
}
257+
162258
/// `<index>/request_id>`
163259
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
164260
pub struct EventId {
Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,214 @@
1+
#![cfg(not(feature = "local"))]
2+
//! Regression test for issue #857: when a streamable-HTTP client disconnects
3+
//! (TCP close) while a tool handler is still running, the in-flight request
4+
//! must be cancelled so the handler's `RequestContext::ct` fires, instead of
5+
//! letting the handler run to completion.
6+
7+
use std::{sync::Arc, time::Duration};
8+
9+
use rmcp::{
10+
ErrorData as McpError, RoleServer, ServerHandler,
11+
model::{CallToolRequestParams, CallToolResult, ContentBlock, ServerCapabilities, ServerInfo},
12+
service::RequestContext,
13+
transport::streamable_http_server::{
14+
StreamableHttpServerConfig, StreamableHttpService,
15+
session::{SessionId, local::LocalSessionManager},
16+
},
17+
};
18+
use tokio::sync::Notify;
19+
use tokio_util::sync::CancellationToken;
20+
21+
/// A minimal server exposing:
22+
/// - `long_sleep`: runs until its per-request cancellation token fires (then
23+
/// notifies `cancelled`) or 30s elapses.
24+
/// - `quick`: returns immediately.
25+
#[derive(Clone)]
26+
struct DisconnectServer {
27+
cancelled: Arc<Notify>,
28+
}
29+
30+
impl ServerHandler for DisconnectServer {
31+
fn get_info(&self) -> ServerInfo {
32+
ServerInfo {
33+
capabilities: ServerCapabilities::builder().enable_tools().build(),
34+
..Default::default()
35+
}
36+
}
37+
38+
async fn call_tool(
39+
&self,
40+
request: CallToolRequestParams,
41+
context: RequestContext<RoleServer>,
42+
) -> Result<CallToolResult, McpError> {
43+
match request.name.as_ref() {
44+
"long_sleep" => {
45+
tokio::select! {
46+
_ = context.ct.cancelled() => {
47+
self.cancelled.notify_one();
48+
Ok(CallToolResult::success(vec![ContentBlock::text("cancelled")]))
49+
}
50+
_ = tokio::time::sleep(Duration::from_secs(30)) => {
51+
Ok(CallToolResult::success(vec![ContentBlock::text(
52+
"ran_to_completion",
53+
)]))
54+
}
55+
}
56+
}
57+
"quick" => Ok(CallToolResult::success(vec![ContentBlock::text(
58+
"quick_done",
59+
)])),
60+
other => Err(McpError::invalid_params(
61+
format!("unknown tool: {other}"),
62+
None,
63+
)),
64+
}
65+
}
66+
}
67+
68+
async fn spawn_server(
69+
server: DisconnectServer,
70+
) -> (String, CancellationToken, tokio::task::JoinHandle<()>) {
71+
let ct = CancellationToken::new();
72+
let service: StreamableHttpService<DisconnectServer, LocalSessionManager> =
73+
StreamableHttpService::new(
74+
move || Ok(server.clone()),
75+
Default::default(),
76+
StreamableHttpServerConfig::default()
77+
// Short keep-alive so the server observes the client disconnect
78+
// (a failed keep-alive write) quickly.
79+
.with_sse_keep_alive(Some(Duration::from_millis(100)))
80+
.with_cancellation_token(ct.child_token()),
81+
);
82+
83+
let router = axum::Router::new().nest_service("/mcp", service);
84+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
85+
let addr = listener.local_addr().unwrap();
86+
let handle = tokio::spawn({
87+
let ct = ct.clone();
88+
async move {
89+
let _ = axum::serve(listener, router)
90+
.with_graceful_shutdown(async move { ct.cancelled_owned().await })
91+
.await;
92+
}
93+
});
94+
(format!("http://{addr}/mcp"), ct, handle)
95+
}
96+
97+
async fn init_session(client: &reqwest::Client, url: &str) -> SessionId {
98+
let response = client
99+
.post(url)
100+
.header("Content-Type", "application/json")
101+
.header("Accept", "application/json, text/event-stream")
102+
.body(
103+
r#"{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2025-06-18","capabilities":{},"clientInfo":{"name":"test","version":"1.0"}}}"#,
104+
)
105+
.send()
106+
.await
107+
.unwrap();
108+
assert_eq!(response.status(), 200);
109+
let session_id: SessionId = response.headers()["mcp-session-id"]
110+
.to_str()
111+
.unwrap()
112+
.into();
113+
114+
let status = client
115+
.post(url)
116+
.header("Content-Type", "application/json")
117+
.header("Accept", "application/json, text/event-stream")
118+
.header("mcp-session-id", session_id.to_string())
119+
.header("Mcp-Protocol-Version", "2025-06-18")
120+
.body(r#"{"jsonrpc":"2.0","method":"notifications/initialized"}"#)
121+
.send()
122+
.await
123+
.unwrap()
124+
.status();
125+
assert_eq!(status, 202);
126+
session_id
127+
}
128+
129+
#[tokio::test]
130+
async fn client_disconnect_cancels_in_flight_request() -> anyhow::Result<()> {
131+
let cancelled = Arc::new(Notify::new());
132+
let (url, ct, handle) = spawn_server(DisconnectServer {
133+
cancelled: cancelled.clone(),
134+
})
135+
.await;
136+
137+
// Disable connection pooling so dropping the response actually closes the
138+
// TCP connection (simulating a client disconnect).
139+
let client = reqwest::Client::builder()
140+
.pool_max_idle_per_host(0)
141+
.build()?;
142+
let session_id = init_session(&client, &url).await;
143+
144+
let response = client
145+
.post(&url)
146+
.header("Content-Type", "application/json")
147+
.header("Accept", "application/json, text/event-stream")
148+
.header("mcp-session-id", session_id.to_string())
149+
.header("Mcp-Protocol-Version", "2025-06-18")
150+
.body(
151+
r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"long_sleep","arguments":{}}}"#,
152+
)
153+
.send()
154+
.await?;
155+
assert_eq!(response.status(), 200);
156+
157+
// Let the handler start awaiting its cancellation token, then disconnect by
158+
// dropping the streaming response without reading it to completion.
159+
tokio::time::sleep(Duration::from_millis(200)).await;
160+
drop(response);
161+
162+
// The fix must fire the handler's cancellation token; `long_sleep` then
163+
// returns via its cancel branch and notifies us. Without the fix the
164+
// handler sleeps for 30s and this times out.
165+
tokio::time::timeout(Duration::from_secs(10), cancelled.notified())
166+
.await
167+
.expect("handler cancellation token should fire after client disconnect");
168+
169+
ct.cancel();
170+
let _ = handle.await;
171+
Ok(())
172+
}
173+
174+
#[tokio::test]
175+
async fn normal_tool_response_is_delivered() -> anyhow::Result<()> {
176+
let cancelled = Arc::new(Notify::new());
177+
let (url, ct, handle) = spawn_server(DisconnectServer {
178+
cancelled: cancelled.clone(),
179+
})
180+
.await;
181+
182+
let client = reqwest::Client::new();
183+
let session_id = init_session(&client, &url).await;
184+
185+
// A normal, fully-read tool call must still work end-to-end: the disconnect
186+
// guard must forward the response unchanged and not cancel a request that
187+
// completes normally.
188+
let body = client
189+
.post(&url)
190+
.header("Content-Type", "application/json")
191+
.header("Accept", "application/json, text/event-stream")
192+
.header("mcp-session-id", session_id.to_string())
193+
.header("Mcp-Protocol-Version", "2025-06-18")
194+
.body(
195+
r#"{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"quick","arguments":{}}}"#,
196+
)
197+
.send()
198+
.await?
199+
.text()
200+
.await?;
201+
202+
assert!(
203+
body.contains("quick_done"),
204+
"expected tool result in response, got: {body}"
205+
);
206+
assert!(
207+
body.contains(r#""id":2"#),
208+
"expected response id 2, got: {body}"
209+
);
210+
211+
ct.cancel();
212+
let _ = handle.await;
213+
Ok(())
214+
}

0 commit comments

Comments
 (0)