Open justin0mcateer opened 4 months ago
I think this is something worth adding to this module for completeness (see https://github.com/alanshaw/it-ws/pull/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 https://github.com/libp2p/js-libp2p/pull/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.
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.
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