Repository navigation
feat: emit peer-disconnect from $sync with unfinished sync counts - #1357
Open
gmaclennan wants to merge 2 commits into
Open
gmaclennan wants to merge 2 commits into
gmaclennan wants to merge 2 commits into
Conversation
When a peer connection closes, SyncApi emits 'peer-disconnect' with the peer's device id, whether the connection was opened by connectServers(), and the want/wanted counts for that peer at the moment of disconnect, read before the peer's remote state is cleared.
A throwing peer-disconnect listener would propagate into hypercore's synchronous peer teardown and surface as an uncaught exception, so the emit is now wrapped and the error logged. State, RemoteDeviceSyncState and PeerDisconnectInfo are exported from the package root under a SyncApi namespace, and the stale @internal tags on the remote sync state types are removed.
This branch has not been deployed
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.
Adds a
peer-disconnectevent toproject.$sync, emitted when a peer connection closes. The payload is the peer's device id,isServer(true when the connection was opened byconnectServers()), and the peer'ssyncState(initialanddata, each withwantandwanted) as it stood at the moment of disconnect. The state is read in#handlePeerDisconnectbeforedisconnectPeer()clears it, which is the one place that knows both that the connection closed and how much sync with that peer was left unfinished.The motivation is in digidem/comapeo-core-react-native#303: we want a metric for "a connection closed while sync with that peer was unfinished". That PR approximated it for servers by watching sync state from the outside. With this event, comapeo-core-react-native can forward disconnects to Sentry in a few lines, and comapeo-cloud can report the same event from its side, so we can see both ends of a disconnect and tell phone-initiated closes from network ones.
The event is emitted from inside hypercore's synchronous peer teardown, so a listener that throws is caught and logged rather than surfacing as an uncaught exception. The payload types (
State,RemoteDeviceSyncState,PeerDisconnectInfo) are exported from the package root under aSyncApinamespace, so consumers don't have to derive them.Tests: a new e2e test in
test-e2e/sync.jsdisconnects two peers with known outstanding data and checks the reported counts, and the existing "data can be synced via a server" test now checks that a server disconnect is reported withisServer: true. The new test asserts only the counts, notisSyncEnabled, because that can flip as the other side tears down first. It also registers a throwing listener, and fails without the guard.