Skip to content

Commit a5af768

Browse files
benbrandtclaude
andauthored
feat(acp)!: answer dropped request responders with an error (#462)
A request handler that dropped its `Responder` without responding left the peer waiting forever. On a v2 connection, dropping the `initialize` responder also left the connection stuck initializing, so a retry was rejected as "already in progress". The SDK now answers such a request itself: with -32800 (Request cancelled) if the peer had cancelled it via `$/cancel_request`, and otherwise with -32603 (Internal error) whose data names the method. Nothing is sent when a response was already sent or handed off (e.g. `forward_response_to`), while another owner still holds the responder, or once the connection is shutting down or terminated. If the handler that dropped the responder returns an error, that error is the response instead of the fallback. The fallback goes through the normal response path, so a dropped `initialize` responder fails initialization like any other error: a first `initialize` leaves the connection uninitialized and can be retried, and a failed repeat keeps the previous initialization. BREAKING CHANGE: dropping a `Responder` without responding now sends an error response for the request (-32603, or -32800 if the peer cancelled the request) instead of sending nothing. Handlers that intentionally dropped a responder and answered the request some other way must now keep the responder and respond through it. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 768a12b commit a5af768

9 files changed

Lines changed: 665 additions & 24 deletions

File tree

‎md/request-cancellation.md‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,29 @@ an unknown or already-completed request ID is silently ignored. A
4343
not a string, number, or null) is logged and ignored without a reply, like any
4444
other malformed notification.
4545

46+
## Unanswered Requests
47+
48+
The SDK makes sure a request it received is always answered, even when the
49+
handler never responds. If a `Responder` is dropped without a response having
50+
been sent through it, the SDK sends an error response for the request:
51+
52+
- `-32800` (Request cancelled) if the peer had cancelled the request with
53+
`$/cancel_request`, as ACP requires for a cancelled request whose method
54+
defines no cancellation result;
55+
- otherwise `-32603` (Internal error), with data naming the method.
56+
57+
If the handler that dropped the responder returns an error, that error is the
58+
response instead. No automatic response is sent when a response was already
59+
sent or handed off (for example with `forward_response_to`), when the
60+
responder was moved into another task or wrapper that still owns it, or when
61+
the connection is shutting down or has terminated. Notifications have no
62+
responder and never get a reply.
63+
64+
The automatic error is an ordinary error response. A dropped `initialize`
65+
responder fails initialization like any other `initialize` error: a first
66+
`initialize` leaves the connection uninitialized, so the client can retry, and
67+
a failed repeat keeps the previous initialization.
68+
4669
## Interoperability
4770

4871
Protocol-level (`$/`-prefixed) notifications are optional by design. The SDK

‎md/transport-architecture.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,8 +178,9 @@ protocol v2 removed batches, and v1 support was experimental and best-effort.
178178
Since the removal, `TransportFrame` has only the `Single` and `Malformed`
179179
variants, and `TransportBatch` and `TransportBatchEntry` no longer exist. Code
180180
that matched `TransportFrame::Batch` or built batches should send each message
181-
as its own `TransportFrame::Single`. Dropping a `Responder` never sends an
182-
automatic reply.
181+
as its own `TransportFrame::Single`. A dropped `Responder` is answered with an
182+
automatic error; see
183+
[Request Cancellation](./request-cancellation.md#unanswered-requests).
183184

184185
Lifecycle-sensitive routing is unaffected: `ProxyProtocolRouter` requires an
185186
exact v1 or v2 `proxy/initialize` match and preserves the initial frame without

‎src/agent-client-protocol/src/concepts/callbacks.rs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@
8686
//!
8787
//! Send at most one response per request. If a handler responds and then
8888
//! returns an error, the first response wins and the duplicate completion is
89-
//! ignored. Dropping a responder sends no response.
89+
//! ignored. Dropping a responder without responding answers the request with
90+
//! an error; see [error handling](super::error_handling#dropped-responders).
9091
//!
9192
//! # Multiple Handlers
9293
//!

‎src/agent-client-protocol/src/concepts/error_handling.rs‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,20 @@
4646
//! Malformed JSON and invalid request envelopes are handled at the SDK's transport
4747
//! boundary. Applications do not need to construct uncorrelated Error Responses.
4848
//!
49+
//! # Dropped Responders
50+
//!
51+
//! Every request gets a response. If a [`Responder`][crate::Responder] is
52+
//! dropped without responding (and was not moved somewhere that responds,
53+
//! such as [`SentRequest::forward_response_to`][crate::SentRequest::forward_response_to]),
54+
//! the SDK answers the request with `-32603` (Internal error), with data naming
55+
//! the method, or with `-32800` (Request cancelled) if the peer had cancelled
56+
//! the request with `$/cancel_request`. If the handler that dropped the
57+
//! responder returns an error, that error is sent instead. Nothing is sent if
58+
//! a response was already sent or the connection is shutting down.
59+
//!
60+
//! The automatic error is an ordinary error response: a dropped `initialize`
61+
//! responder fails initialization, so the client can retry.
62+
//!
4963
//! # The `into_internal_error` Helper
5064
//!
5165
//! When working with external libraries that return their own error types,

0 commit comments

Comments
 (0)