Repository navigation
fix(acp): don't let peer errors match the SDK's termination predicates - #484
Merged
Merged
Conversation
`is_connection_terminated` and `is_incoming_transport_closed` only checked
`error.data.reason`. A peer error response is delivered with its data
unchanged, so a peer answering a live request with
`{"reason":"connection_terminated"}` (or a proxy forwarding a downstream
termination error) made the SDK report its own connection as terminated.
The SDK now marks the errors it produces for these conditions with a
private, never-serialized origin field on `Error`, and the predicates check
only that marker. The `data.reason` values are still set for display and
logging. The marker is ignored by `PartialEq` and `Debug`, so errors keep
comparing and printing as the JSON-RPC objects they hold.
Co-Authored-By: Claude <noreply@anthropic.com>
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
is_connection_terminatedandis_incoming_transport_closedonly checkederror.data.reason. Peer error responses keep theirdataunchanged, so a peer answering a live request with{"data":{"reason":"connection_terminated"}}made the SDK report that its own, still-running connection had terminated. A proxy forwarding a downstream termination error upstream has the same effect.Fix
Error(already#[non_exhaustive]) gets a privateoriginmarker that is never serialized (#[serde(skip)]). The SDK sets it only on the errors it produces for clean incoming EOF and connection termination, and both predicates check only that marker.data.reason,data.methodanddata.terminationare still set, for display only.PartialEqandDebugignore the marker, so errors keep comparing and printing as their code, message and data. No public API change.Rustdoc on the predicates and reason constants,
concepts::connections, andmd/design.mdnow say that only locally produced errors match.Tests
connection_termination::peer_errors_with_termination_reasons_do_not_match: a peer answers live requests with the exact errors the SDK produces locally (same code, message anddata, for both reasons). The errors are delivered unchanged, neither predicate matches, and the connection is still running. It fails onmain. The existing termination and EOF tests still pass, so locally produced errors still match.