fix ping/pong handler in tapir websocket protocol #1458
Merged
+36
−8
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
|
Ah, just realised that this would cause issues on receiving the pong message from client when keepAlive config is set server-side. |
|
Ok fixed... |
|
Just made the ping hook accepting an optional payload, because one may want to run it independent from given payload, e.g. to perform something like health checks? (ping). |
|
Thanks for your work, I will have more time to review this in the weekend. @paulpdaniels in case you have time to take a look 😄 |
paulpdaniels
reviewed
Sep 9, 2022
interop/tapir/src/main/scala/caliban/interop/tapir/WebSocketHooks.scala
Outdated
Show resolved
Hide resolved
paulpdaniels
approved these changes
Sep 11, 2022
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.
Hi, I believe I found a bug in the protocol while investigating an issue we have with our websocket server using the graphql-ws client with keepAlive config (see config description). Currently It causes the server to close the websocket after it receives a ping message instead of responding with pong.
This update would make it respond with a "pong" message to "ping" by default, or after a successfully executed "onPing" hook if defined.