Add ping and pong received callbacks by igolaizola · Pull Request #509 · coder/websocket · GitHub

Add ping and pong received callbacks

This change adds two optional callbacks to both DialOptions and
AcceptOptions. These callbacks are invoked synchronously when a ping
or pong frame is received, allowing advanced users to log or inspect
payloads for metrics or debugging. If the callback needs to perform more
complex work or reuse the payload outside the callback, it is
recommended to perform processing in a separate goroutine.

The boolean return value of OnPingReceived is used to determine if the
subsequent pong frame should be sent. If false is returned, the pong
frame is not sent.

Tests confirm that the ping/pong callbacks are invoked as expected.

Fixes #246
Closes #307

It seems the tests I added are flaky. I'm investigating this.

It seems the tests I added are flaky. I'm investigating this.

I've found the problem. newConnTest returns the two connections in a random order. I'm fixing the test to account for this.

pingPongReceived test is passing now, but it seems there is an unrelated test (TestWasm) that is failing here and in other PRs.

Thanks for the pull request! Looks great, I just had some questions/concerns about the ping received payload modifications.

This looks great and the docs are well written too, thanks! Once the data race in the tests is fixed, I believe this is good to go 👍🏻

This change adds two optional callbacks to both `DialOptions` and `AcceptOptions`. These callbacks are invoked synchronously when a ping or pong frame is received, allowing advanced users to log or inspect payloads for metrics or debugging. If the callback needs to perform more complex work or reuse the payload outside the callback, it is recommended to perform processing in a separate goroutine. The boolean return value of `OnPingReceived` is used to determine if the subsequent pong frame should be sent. If `false` is returned, the pong frame is not sent. Tests confirm that the ping/pong callbacks are invoked as expected. References coder#246

It looks like we are now encountering an issue related to wasmbrowsertest and timeouts. I'm trying to update it to the latest version to see if that resolves the problem.

This repo had a similar issue, and it looks like they fixed it by updating the version as well: hajimehoshi/ebiten#2982

[edit] Moved the fix attempt to a new PR: #514

This looks great and the docs are well written too, thanks! Once the data race in the tests is fixed, I believe this is good to go 👍🏻

CI is passing now. Feel free to merge this.

We can discuss the flaky TestWasm here (#514), as it is unrelated to this PR change.

Awesome, thanks for the contribution!

Hi,

This is great. Any info on when the next release will be? Thanks to all the maintainers and contributors!

@mafredri

@chrpwd the release is out now v1.8.13! 🎉

igolaizola mentioned this pull request Jan 28, 2025

support set ping handler and pong handler #307

Closed

mafredri self-requested a review January 28, 2025 09:27

igolaizola force-pushed the ping-pong-callbacks branch from a154a33 to 679b569 Compare January 28, 2025 09:35

mafredri reviewed Jan 29, 2025

View reviewed changes

Comment thread accept.go Outdated

Comment thread conn_test.go Outdated

Comment thread accept.go Outdated

mafredri reviewed Jan 29, 2025

View reviewed changes

Comment thread conn_test.go Outdated

igolaizola force-pushed the ping-pong-callbacks branch 2 times, most recently from 38c82dd to a9a6a1f Compare January 29, 2025 12:09

mafredri reviewed Jan 29, 2025

View reviewed changes

Comment thread conn_test.go

igolaizola force-pushed the ping-pong-callbacks branch from a9a6a1f to 45597ec Compare January 29, 2025 19:20

igolaizola force-pushed the ping-pong-callbacks branch from 45597ec to 85e8670 Compare January 29, 2025 19:38

igolaizola force-pushed the ping-pong-callbacks branch from 1c94ea4 to 85e8670 Compare January 30, 2025 08:26

igolaizola mentioned this pull request Jan 30, 2025

ci: update wasmbrowsertest to a specific commit #514

Merged

mafredri approved these changes Jan 30, 2025

View reviewed changes

mafredri merged commit 703784f into coder:master Jan 30, 2025

igolaizola deleted the ping-pong-callbacks branch January 30, 2025 12:22

yaslama mentioned this pull request Mar 18, 2025

Add support for websocket swap updates proxying breez/swapproxy#1

Merged

Add ping and pong received callbacks by igolaizola · Pull Request #509 · coder/websocket · GitHub

Uh oh!

Add ping and pong received callbacks - #509

Add ping and pong received callbacks#509mafredri merged 1 commit intocoder:mastercoder/websocket:masterfrom igolaizola:ping-pong-callbacksigolaizola/websocket:ping-pong-callbacksCopy head branch name to clipboard

Conversation

igolaizola commented Jan 28, 2025 • edited by mafredri Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igolaizola commented Jan 28, 2025

Uh oh!

igolaizola commented Jan 28, 2025

Uh oh!

igolaizola commented Jan 28, 2025

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

igolaizola commented Jan 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

igolaizola commented Jan 30, 2025

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

chrpwd commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mafredri commented Mar 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add ping and pong received callbacks#509
mafredri merged 1 commit into
coder:masterfrom
igolaizola:ping-pong-callbacks

igolaizola commented Jan 28, 2025 •
edited by mafredri
Loading

igolaizola commented Jan 30, 2025 •
edited
Loading

chrpwd commented Feb 26, 2025 •
edited
Loading