Skip to content

fix(transport): cancel pending modern HTTP requests #1194

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lucarlig wants to merge 1 commit into
base: main
Choose a base branch
from
+180 −13
Draft

fix(transport): cancel pending modern HTTP requests #1194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/rmcp/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,9 @@ path = "tests/test_streamable_http_connection_reuse.rs"
[[test]]
name = "test_streamable_http_disconnect_cancel"
required-features = [
"client",
"server",
"transport-streamable-http-client-reqwest",
"transport-streamable-http-server",
"reqwest",
]
Expand Down
130 changes: 123 additions & 7 deletions crates/rmcp/src/transport/streamable_http_client.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ use crate::{

type BoxedSseStream = BoxStream<'static, Result<Sse, SseError>>;
type SseTaskResult<E> = (Option<RequestId>, Result<(), StreamableHttpError<E>>);

struct ModernPostTaskResult<E: std::error::Error + Send + Sync + 'static> {
request_id: Option<RequestId>,
responder: tokio::sync::oneshot::Sender<Result<(), StreamableHttpError<E>>>,
response: Option<Result<StreamableHttpPostResponse, StreamableHttpError<E>>>,
stream_ct: CancellationToken,
}

const SESSION_CLEANUP_TIMEOUT: Duration = Duration::from_secs(5);

fn build_request_headers(
Expand Down Expand Up @@ -949,19 +957,17 @@ impl<C: StreamableHttpClient> Worker for StreamableHttpClientWorker<C> {
))?;
let _ = initialized_notification.responder.send(Ok(()));
}
#[expect(
clippy::large_enum_variant,
reason = "the event is short-lived and boxing would add allocation in the event loop"
)]
enum Event<W: Worker, E: std::error::Error + Send + Sync + 'static> {
ClientMessage(WorkerSendRequest<W>),
ServerMessage(ServerJsonRpcMessage),
ModernPostResult(ModernPostTaskResult<E>),
StreamResult {
request_id: Option<RequestId>,
result: Result<(), StreamableHttpError<E>>,
},
}
let mut streams = tokio::task::JoinSet::new();
let mut modern_posts = tokio::task::JoinSet::new();
let mut pending_stream_response_ids = HashSet::new();
let mut request_stream_cancellations = HashMap::<RequestId, CancellationToken>::new();
let mut awaiting_fallback_initialized = false;
Expand Down Expand Up @@ -996,6 +1002,13 @@ impl<C: StreamableHttpClient> Worker for StreamableHttpClientWorker<C> {
};
Event::ServerMessage(message)
},
terminated_post = modern_posts.join_next(), if !modern_posts.is_empty() => {
match terminated_post {
Some(Ok(result)) => Event::ModernPostResult(result),
Some(Err(error)) => break 'main_loop Err(WorkerQuitReason::Join(error)),
None => continue,
}
}
terminated_stream = streams.join_next(), if !streams.is_empty() => {
match terminated_stream {
Some(result) => {
Expand Down Expand Up @@ -1125,9 +1138,6 @@ impl<C: StreamableHttpClient> Worker for StreamableHttpClientWorker<C> {
ClientNotification::InitializedNotification(_)
)
);
// Pass a clone to the first attempt so `message` is retained for a
// potential re-init retry. `post_message` takes ownership and the
// trait cannot be changed, so the clone is unavoidable.
let (request_version, request_headers) = request_version_headers(
&protocol_headers,
&message,
Expand All @@ -1144,6 +1154,40 @@ impl<C: StreamableHttpClient> Worker for StreamableHttpClientWorker<C> {
cleanup.protocol_headers = protocol_headers.clone();
}
}
if uses_modern_http {
let stream_ct = transport_task_ct.child_token();
if let Some(request_id) = request_id.as_ref() {
request_stream_cancellations
.insert(request_id.clone(), stream_ct.clone());
}
let client = self.client.clone();
let uri = config.uri.clone();
let auth_header = config.auth_header.clone();
let max_sse_event_size = config.max_sse_event_size;
modern_posts.spawn(async move {
let response = tokio::select! {
response = client.post_message_with_max_sse_event_size(
uri,
message,
None,
auth_header,
request_headers,
max_sse_event_size,
) => Some(response),
_ = stream_ct.cancelled() => None,
};
ModernPostTaskResult {
request_id,
responder,
response,
stream_ct,
}
});
continue;
}
// Pass a clone to the first attempt so `message` is retained for a
// potential re-init retry. `post_message` takes ownership and the
// trait cannot be changed, so the clone is unavoidable.
let response = self
.client
.post_message_with_max_sse_event_size(
Expand Down Expand Up @@ -1393,6 +1437,78 @@ impl<C: StreamableHttpClient> Worker for StreamableHttpClientWorker<C> {
}
let _ = responder.send(send_result);
}
Event::ModernPostResult(ModernPostTaskResult {
request_id,
responder,
response,
stream_ct,
}) => {
let send_result = match (stream_ct.is_cancelled(), response) {
(true, _) | (_, None) => Ok(()),
(false, Some(response)) => match response {
Err(error) => {
if let Some(request_id) = request_id.as_ref() {
request_stream_cancellations.remove(request_id);
}
Err(error)
}
Ok(StreamableHttpPostResponse::Accepted) => {
if let Some(request_id) = request_id.as_ref() {
request_stream_cancellations.remove(request_id);
}
Self::mark_stream_response_pending(
&mut pending_stream_response_ids,
request_id,
);
tracing::trace!("client message accepted");
Ok(())
}
Ok(StreamableHttpPostResponse::Json(mut message, ..)) => {
if let Some(request_id) = request_id.as_ref() {
request_stream_cancellations.remove(request_id);
}
cache_tools_from_response(
&mut tool_header_cache,
&mut message,
&negotiated_version,
);
context.send_to_handler(message).await?;
Ok(())
}
Ok(StreamableHttpPostResponse::Sse(stream, ..)) => {
Self::mark_stream_response_pending(
&mut pending_stream_response_ids,
request_id.clone(),
);
let sse_stream = Self::response_sse_to_jsonrpc(
stream,
None,
self.client.clone(),
config.uri.clone(),
config.auth_header.clone(),
protocol_headers.clone(),
config.max_sse_event_size,
self.config.retry_config.clone(),
);
let stream_tx = sse_worker_tx.clone();
let origin = match &request_id {
Some(id) => InboundStreamOrigin::OutboundRequest(id.clone()),
None => InboundStreamOrigin::Unassociated,
};
streams.spawn(async move {
let result = Self::execute_sse_stream(
sse_stream, stream_tx, origin, true, stream_ct,
)
.await;
(request_id, result)
});
tracing::trace!("got new sse stream");
Ok(())
}
},
};
let _ = responder.send(send_result);
}
Event::ServerMessage(mut json_rpc_message) => {
if let Some(response_id) = Self::server_response_id(&json_rpc_message)
&& let Some(stream_ct) = crate::service::remove_pending_request(
Expand Down
61 changes: 55 additions & 6 deletions crates/rmcp/tests/test_streamable_http_disconnect_cancel.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#![cfg(all(
feature = "client",
feature = "server",
feature = "transport-streamable-http-server",
feature = "reqwest",
Expand All @@ -16,14 +17,18 @@
use std::{sync::Arc, time::Duration};

use rmcp::{
ErrorData as McpError, RoleServer, ServerHandler,
ClientLifecycleMode, ClientServiceExt, ErrorData as McpError, RoleServer, ServerHandler,
model::{
CallToolRequestParams, CallToolResponse, CallToolResult, ContentBlock, ServerCapabilities,
ServerInfo,
CallToolRequestParams, CallToolResponse, CallToolResult, ClientInfo, ClientRequest,
ContentBlock, ProtocolVersion, Request, ServerCapabilities, ServerInfo,
},
service::RequestContext,
transport::streamable_http_server::{
StreamableHttpServerConfig, StreamableHttpService, session::local::LocalSessionManager,
service::{PeerRequestOptions, RequestContext},
transport::{
StreamableHttpClientTransport,
streamable_http_client::StreamableHttpClientTransportConfig,
streamable_http_server::{
StreamableHttpServerConfig, StreamableHttpService, session::local::LocalSessionManager,
},
},
};
use tokio::sync::Notify;
Expand Down Expand Up @@ -194,3 +199,47 @@ async fn stateless_json_client_disconnect_cancels_request() -> anyhow::Result<()
server.server_ct.cancel();
Ok(())
}

/// Modern RMCP clients must be able to cancel while the request POST is still
/// waiting for its first response event.
#[tokio::test]
async fn modern_rmcp_client_cancels_pending_request() -> anyhow::Result<()> {
let server = spawn_stateless_server(false).await?;
let transport = StreamableHttpClientTransport::from_config(
StreamableHttpClientTransportConfig::with_uri(server.url.clone()),
);
let client = ClientInfo::default()
.serve_with_lifecycle(
transport,
ClientLifecycleMode::Discover {
preferred_versions: vec![ProtocolVersion::V_2026_07_28],
},
)
.await?;

let request = client
.send_cancellable_request(
ClientRequest::CallToolRequest(Request::new(CallToolRequestParams::new(
"wait_for_cancel".to_owned(),
))),
PeerRequestOptions::no_options(),
)
.await?;

tokio::time::timeout(Duration::from_secs(5), server.started.notified())
.await
.expect("tool handler should start");
tokio::time::timeout(
Duration::from_secs(5),
request.cancel(Some("test cancellation".to_owned())),
)
.await
.expect("RequestHandle::cancel should not wait for the response stream")?;
tokio::time::timeout(Duration::from_secs(10), server.cancelled.notified())
.await
.expect("RequestContext::ct should fire after RequestHandle::cancel");

client.cancel().await?;
server.server_ct.cancel();
Ok(())
}