Add server tool timeouts (default & per-tool) and honor JSON-RPC $/cancelRequest (#922) #951
Conversation
I'm not understanding the purpose of this. Cancellation is already supported with |
bkarakaya01
changed the title
I see how my original wording made this look like a cancellation feature PR. The actual primary goal here is server-side tool timeout support. The I’ve updated the PR title/description to reflect the real scope (tool timeout) much clearer. Thanks for pointing it out! |
| && message is JsonRpcRequest cancelledReq | ||
| && IsUserInitiatedCancellation(oce, cancellationToken, combinedCts)) | ||
| { | ||
| await SendRequestCancelledErrorAsync(cancelledReq, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
If the timeout is configured server side can the server than cancel the client request? This seems odd to me.
If have a client and I get a 'cancelled error from the server' how am I to know this is a time-out?
| /// <remarks> | ||
| /// This error is returned when the CancellationToken passed with the request is cancelled before processing completes. | ||
| /// </remarks> | ||
| RequestCancelled = -32800, |
There was a problem hiding this comment.
If the timeout happens on the server, then this isn't a client cancellation?
|
I think this is conflating cancellation and server / client time-outs. A server-side timeout is, as I see it, an implementation detail of the server, and not really a protocol level concern. I can see how convenience to implement this is nice, but if it is added, I think it needs to be much more clear on this being a purely server side interaction, and the client also needs a better error message. This isn't a cancellation - it is the server deciding that long-running tool calls are an error and aborting the client call. |
…lation This introduces support for parsing and honoring JSON-RPC \$/cancelRequest\ notifications, making cancellation semantics compliant with MCP protocol expectations. Includes a new \JsonRpcCancelRequest\ constant in \NotificationMethods\ and integrates proper cancellation token linkage inside the server message loop. Additionally: - Added SlowTool test fixture to simulate long-running tools - Implemented full timeout vs external cancellation tests - Normalized English summaries / comments in related areas Fixes modelcontextprotocol#922
…ction, machine-readable Meta.TimeoutMs)
…er review feedback)
…ersion; ensure silent $/cancelRequest path
…path; no protocol error emitted)
bkarakaya01
force-pushed
the
feature/cancel-request-support-922
branch
from
fdb2157 to
48ad945
Compare
Thanks, that makes sense. I see now that this behavior blurred the line between user cancellation vs server-side timeouts. I’ve updated the PR so that:
This keeps the semantics clearly separated and avoids protocol confusion. Thanks again for the guidance, waiting for your review. |
…emit standard cancel; keep server timeouts in RunWithTimeoutAsync
3 participants
This PR introduces server-side tool timeout support and ensures proper JSON-RPC
$/cancelRequesthandling.Summary of Changes
DefaultToolTimeouttoMcpServerOptionsIMcpToolWithTimeoutinterface for per-tool overrideNotificationMethods.JsonRpcCancelRequestconstantSlowToolfixture for timeout scenarios$/cancelRequestcancellation pathFixes #922
Motivation and Context
Timeout control at the server/tool level is required for predictable behavior when dealing with long-running tool executions.
$/cancelRequesthandling complements this by ensuring JSON-RPC cancellation semantics correctly surface to the caller.How Has This Been Tested?
All new behavior is covered via unit tests under
McpServerToolTimeoutTestsusingTestServerTransport.Breaking Changes
No breaking changes. Existing server implementations continue to work without modification.
Types of Changes
Checklist
Additional Context
Server-side timeout is surfaced as a normal tool error with explicit
Meta.IsTimeoutmetadata - this is not modeled as cancellation and is distinct from$/cancelRequest.This PR also ensures the server does not respond on user-initiated
$/cancelRequestper JSON-RPC expectations.