Skip to content

ClientHandler's Send + Sync bound is not gated by the local feature (unlike ServerHandler and Service) #1062

Description

@gold-silver-copper

ClientHandler appears to have been missed when the local feature was introduced: it is the only handler/service trait whose Send + Sync bound is not gated on it.

ServerHandler has both variants (src/handler/server.rs:431-438):

#[cfg(not(feature = "local"))]
pub trait ServerHandler: Sized + Send + Sync + 'static { ... }

#[cfg(feature = "local")]
pub trait ServerHandler: Sized + 'static { ... }

Service<R> — the trait ClientHandler blankets into via impl<H: ClientHandler> Service<RoleClient> for H — has both too (src/service.rs:136 and :151).

ClientHandler has only one (src/handler/client.rs:89):

pub trait ClientHandler: Sized + Send + Sync + 'static {

So with local enabled, a client handler still has to be Send + Sync, even though everything downstream of it no longer requires that.

Why it matters

This is the last blocker for a browser-wasm MCP client in our case. With features = ["client", "local"] on wasm32-unknown-unknown:

  • rmcp itself compiles cleanly for the target.
  • MaybeSendFuture correctly stops requiring Send on the handler's futures.
  • The build then fails only on this bound, because our handler owns a tool registry holding Arc<dyn Tool> values that are deliberately not Send/Sync on wasm — browser tools capture JsValue, which is !Send by design.

That is the same !Send JS-interop constraint described in #379. Worth noting relative to that thread: the conclusion there was that wasm support would be a very large changeset, but for the client path specifically the local feature already covers essentially all of it — this one bound is what remains. (#379 is currently marked stale and scheduled to auto-close, which is part of why I am filing this separately: it is a small, self-contained item rather than the umbrella effort.)

Reproduction

// Cargo.toml
// [target.'cfg(target_arch = "wasm32")'.dependencies]
// rmcp = { version = "2.2", features = ["client", "local"] }

use std::rc::Rc;

struct Handler {
    // Any !Send state — a JsValue, an Rc, a browser tool registry.
    _local: Rc<()>,
}

impl rmcp::handler::client::ClientHandler for Handler {
    fn get_info(&self) -> rmcp::model::ClientInfo {
        Default::default()
    }
}
$ cargo check --target wasm32-unknown-unknown
error[E0277]: `Rc<()>` cannot be shared between threads safely
  --> src/lib.rs:7:47
note: required by a bound in `ClientHandler`
  --> rmcp-2.2.0/src/handler/client.rs:89:41

error[E0277]: `Rc<()>` cannot be sent between threads safely
  --> src/lib.rs:7:47
note: required by a bound in `ClientHandler`
  --> rmcp-2.2.0/src/handler/client.rs:89:34

(Verified against rmcp 2.2.0 on rustc 1.94.0.)

Suggested fix

Mirror ServerHandler:

#[cfg(not(feature = "local"))]
pub trait ClientHandler: Sized + Send + Sync + 'static { ... }

#[cfg(feature = "local")]
pub trait ClientHandler: Sized + 'static { ... }

MaybeSend (src/service.rs:18) already expands to Send + Sync without local and to nothing with it, so Sized + MaybeSend + 'static would work as a single definition if you prefer avoiding the duplicated method bodies — ServerHandler uses a server_handler_methods!() macro for that reason, and ClientHandler's body is small enough to go either way.

No behaviour change without local; the bound is identical there.

Scope

I have only verified that this unblocks compiling a client handler. I have not yet checked whether the streamable-HTTP transports build for wasm32-unknown-unknown, so there may be further work behind this for an end-to-end browser client. This bound is simply the first thing in the way, and it looks like an oversight rather than a decision.

Metadata

Metadata

Assignees

Labels

P2Medium: important but non-blocking improvement

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions