Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ping/Pong Support #113

Open
justin0mcateer opened this issue Jun 14, 2024 · 2 comments · May be fixed by #120
Open

Ping/Pong Support #113

justin0mcateer opened this issue Jun 14, 2024 · 2 comments · May be fixed by #120

Comments

@justin0mcateer
Copy link

Because the actual WebSocket object from 'ws' is not returned in the emitted value, there doesn't appear to be anyway for the consumer to implement WebSocket ping/pong functionality.

We are using this via the Libp2p WebSocket transport and we sometimes have issues with WebSockets hanging open when the remote side fails in certain ways.

Would there be any opposition to adding an optional 'pingTime' parameter in the ServerOptions that would default to 0 (for disabled)? And implement the ping/pong in this library as described in the 'ws' documentation here:
https://www.npmjs.com/package/ws#how-to-detect-and-close-broken-connections

achingbrain added a commit that referenced this issue Jul 30, 2024
Adds an optional `heartbeatMs` key to the server config that will
ping each connected client on that interval - if they have not
responded with a pong since the last interval, the client will be
disconnected.

Fixes #113
@achingbrain achingbrain linked a pull request Jul 30, 2024 that will close this issue
achingbrain added a commit that referenced this issue Jul 30, 2024
Adds an optional `heartbeatMs` key to the server config that will
ping each connected client on that interval - if they have not
responded with a pong since the last interval, the client will be
disconnected.

Fixes #113
@achingbrain
Copy link
Collaborator

achingbrain commented Jul 31, 2024

I think this is something worth adding to this module for completeness (see #120) but the use of this in libp2p is a bit wonky as browsers don't present an API to detect if the server stopped sending pings so it's hard to do this in a peer to peer fashion - that is, only the server end can test if the client has gone away, the client can't test that the server is still available (unless it's a node.js client).

Can you take a look at libp2p/js-libp2p#2644 - it adds a connection monitor to libp2p that allows both ends of the connection to periodically assert that the other end hasn't disconnected. I figure this will be useful for more than just websockets (e.g. for UDP based transports like WebTransport, and one day, QUIC) so it's worth doing it at a higher level.

@justin0mcateer
Copy link
Author

I think both solutions are helpful. We would likely use both in a 'belt and suspenders' approach. The WebSocket layer 'pings' are probably lower overhead and could possibly be run more frequently than the application layer ping/pong.

As an aside, we have seen quite reliable and fast disconnect detection from the WebRTC transport, so I assume the browsers are constantly sending some connectivity checks there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants