fix(transport): notify every subscriber and expose failureSignal when a confirmed subscription dies - #93
Merged
Merged
Conversation
… a confirmed subscription dies _failSubscription used to notify exactly one confirmed lease per listener registration (first-live-owner) and only when that caller had passed options.onError - every other subscriber sharing the payload, and any subscriber without onError, lost its feed silently while unsubscribe() still appeared to work. - _failSubscription now notifies EVERY live confirmed lease's onError, each call wrapped in its own try/catch so one throwing callback cannot silence the rest; unconfirmed leases still observe the failure solely through their subscribe() rejection, and voluntarily retired leases are never notified. - ISubscription gains an optional failureSignal?: AbortSignal that aborts with the failure TransportError as its reason when an already confirmed subscription fails, and never on a voluntary unsubscribe(). It is optional for back-compat with external ISubscriptionTransport implementations, but the WebSocket transport always provides it: the handle returned by WebSocketSubscriptionManager.subscribe() carries a lazily materialized per-call AbortController - accessed before the failure it aborts at failure time, first accessed after the failure it comes back already aborted with the same recorded reason, and the common path that never reads the signal allocates nothing. Subscription and explorer API methods return the transport handle unchanged, so the signal reaches the public API without threading. - Updated the invalidated comments (first-live-owner ownership text, RegistrationHandle.confirmed, _failSubscription, subscribe() option docs in the manager, transport, and ISubscriptionTransport) plus docs/clients.md and docs/transports.md failure semantics. - Tests: shared-payload failures fan out to every onError (same and different listeners), failureSignal aborts with the WebSocketRequestError, stays inert on voluntary unsubscribe() and for leases retired before the failure, and post-failure first access returns an already-aborted signal with the identical reason. Fixes #89
Merged
joeblau
added a commit
that referenced
this pull request
Aug 4, 2026
Four fixes, each cross-verified to agreement by three independent reviewers (Claude, Kimi K3 Max, Codex gpt-5.6-sol xhigh): - #89 subscription failures notify every subscriber + failureSignal (#93) - #90 WebSocket message pacing on by default, flush charging fixed (#96) - #91 200-OK { type: "error" } envelopes throw HttpRequestError (#94) - #92 wallet shape detection by member presence, not arity (#95) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Summary
_failSubscriptionnow notifies every live confirmed lease (previously: first-live-owner only, and only withonErrorpassed) — eachonErrorisolated in its own try/catchfailureSignal?: AbortSignalonISubscription— aborts with the failureTransportErroras reason when a confirmed subscription dies, never on voluntaryunsubscribe(); lazily allocated (common path pays nothing), always provided byWebSocketTransportunsubscribe()on a sibling lease cannot rob it of its notificationVerification (3-agent agreement)
bun run check+ 1965 offline tests, 0 failFixes #89
🤖 Generated with Claude Code