feat: track node connection state #1719
Merged
Conversation
This file contains 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
size-limit report 📦
|
adklempner
force-pushed
the
adklempner/node-connection-state
branch
2 times, most recently
from
a8d198f to
4c0a743
Compare
adklempner
commented
Nov 16, 2023
| })(); | ||
| }, | ||
| "peer:disconnect": () => { | ||
| return (evt: CustomEvent<PeerId>): void => { | ||
| "peer:disconnect": (evt: CustomEvent<PeerId>): void => { |
There was a problem hiding this comment.
When I tested locally, this event handler was not being called at all until I changed it to use the same pattern as the two above
adklempner
force-pushed
the
adklempner/node-connection-state
branch
from
4c0a743 to
1bebcbc
Compare
adklempner
force-pushed
the
adklempner/node-connection-state
branch
from
1bebcbc to
2e34b07
Compare
danisharora099
requested changes
Nov 16, 2023
weboko
approved these changes
Nov 16, 2023
weboko
reviewed
Nov 16, 2023
adklempner
force-pushed
the
adklempner/node-connection-state
branch
from
2e34b07 to
b1e7a63
Compare
adklempner
force-pushed
the
adklempner/node-connection-state
branch
from
b1e7a63 to
73f4126
Compare
danisharora099
approved these changes
Nov 22, 2023
Comment on lines
+51
to
+71
| private toggleOnline(): void { | ||
| if (!this.online) { | ||
| this.online = true; | ||
| this.dispatchEvent( | ||
| new CustomEvent<boolean>(EConnectionStateEvents.CONNECTION_STATUS, { | ||
| detail: this.online | ||
| }) | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| private toggleOffline(): void { | ||
| if (this.online && this.libp2p.getConnections().length == 0) { | ||
| this.online = false; | ||
| this.dispatchEvent( | ||
| new CustomEvent<boolean>(EConnectionStateEvents.CONNECTION_STATUS, { | ||
| detail: this.online | ||
| }) | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
nit: maybe this particular dispatch event can be written as a private function for the class: dispatchConnectionStatus() since it's being reused twice.
There was a problem hiding this comment.
This is a usual concern with these type of functions. I like current one as it is explicitly mentions when it becomes online and when offline.
@adklempner can follow up if thinks it is necessary
Merged
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.
Problem
There is currently no easy way to check if a waku node is connected to any other peers.
Solution
Adds a function
isConnectedto theWakuinterface which returns a boolean indicating whether or not the node has any active connections.The node initially starts offline. Once the first
peer:connectevent is detected by theConnectionManager, it will now be considered offline. If theConnectionManagerdetects apeer:disconnectevent, and there are no timers for either relay or ping in theKeepAliveManager, then the node will be considered offline.Users can either call
isConnectedon the Waku node instance or add an event listener to the connection manager forwaku:onlineorwaku:offlineeventsNotes