✨ Add ping/pong #576
Merged
✨ Add ping/pong #576
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
alecgibson
force-pushed
the
ping-pong
branch
from
4f6a556 to
5cb2dc5
Compare
|
NB This would be a perfect candidate for a plugin: #337 |
ericyhwang
reviewed
Sep 27, 2022
| @@ -476,6 +478,12 @@ Connection.prototype.send = function(message) { | |||
| this.socket.send(JSON.stringify(message)); | |||
| }; | |||
|
|
|||
| Connection.prototype.ping = function() { | |||
There was a problem hiding this comment.
From discussion today - let's throw an error if the connection state is not connected.
This is to guard against a bunch of ping requests queueing up at the socket layer if the socket is currently disconnected or connecting, and having them suddenly flood the socket when eventually connected.
There was a problem hiding this comment.
Done — added ERR_CANNOT_PING_OFFLINE
This change adds the ability for a client to send a "ping" to the server and receive a "pong" response. The motivation for this is that sometimes the socket underlying the `Connection` may not reliably report its connection state (for example if using a wrapper around a "vanilla" websocket that handles reconnection). The most bullet-proof way for a client to determine its connection state is to actually make a request to the server and assert that it receives a timely response. The implementation of ping/pong is arguably a websocket concern, especially since it's already [defined by the spec][1]. However, the browser JavaScript [`WebSocket`][2] does not expose this functionality, so we have to add our own ping/pong layer on top anyway. It's also worth noting that consumers of this library can't easily send their own ping messages down the socket, since ShareDB will [error][3] for unknown messages. Note that this change only adds the ability for a client to ping the server, and not the other way around. This is because: - the `Agent` is not an `Emitter`, and emitting a `pong` on the `Backend` is pretty meaningless - server-side implementations of WebSockets, such as `ws`, _do_ [expose a `ping` API][4] [1]: https://www.rfc-editor.org/rfc/rfc6455#section-5.5.2 [2]: https://developer.mozilla.org/en-US/docs/Web/API/WebSocket [3]: https://github.com/share/sharedb/blob/8b531bedf19860ffe0b37a3e1c8da7a32b5005bd/lib/agent.js#L451 [4]: https://github.com/websockets/ws/blob/966f9d47cd0ff5aa9db0b2aa262f9819d3f4d414/lib/websocket.js#L351
alecgibson
force-pushed
the
ping-pong
branch
from
5cb2dc5 to
56ac7a8
Compare
ericyhwang
approved these changes
Sep 27, 2022
|
Very nice! |
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.
This change adds the ability for a client to send a "ping" to the server
and receive a "pong" response.
The motivation for this is that sometimes the socket underlying the
Connectionmay not reliably report its connection state (for exampleif using a wrapper around a "vanilla" websocket that handles
reconnection).
The most bullet-proof way for a client to determine its connection state
is to actually make a request to the server and assert that it receives
a timely response.
The implementation of ping/pong is arguably a websocket concern,
especially since it's already defined by the spec. However, the
browser JavaScript
WebSocketdoes not expose this functionality,so we have to add our own ping/pong layer on top anyway.
It's also worth noting that consumers of this library can't easily
send their own ping messages down the socket, since ShareDB will
error for unknown messages.
Note that this change only adds the ability for a client to ping the
server, and not the other way around. This is because:
Agentis not anEmitter, and emitting apongon theBackendis pretty meaninglessws, doexpose a
pingAPI