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

support set ping handler and pong handler #307

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

qcha0
Copy link

@qcha0 qcha0 commented Jun 11, 2021

first of all, thanks this good project, we are preparing a tool based on this project.

Sometimes i need do something while receive ping or pong message.
So I added this feature, is it alright

@qcha0 qcha0 requested a review from nhooyr as a code owner June 11, 2021 11:25
Copy link
Contributor

@nhooyr nhooyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for implementing this @qcha0

Related issue is #246

What's your usecase out of curiosity for needing to instrument ping/pongs?

@qcha0
Copy link
Author

qcha0 commented Mar 14, 2023

just like i want to count the number of ping,or set close the connection if there is no ping message whithin two cycle.

@schmidtw
Copy link

schmidtw commented Mar 28, 2024

@nhooyr I need pings/pongs at the application layer for an app we're building a replacement for. I know work is being done on the dev branch, but adding this one feature to a trusted, working code base prevents me from needing to fork the repo for just this one feature we're trying to ship in a week.

Could this PR be merged?

@nhooyr
Copy link
Contributor

nhooyr commented Mar 29, 2024

@schmidtw Sorry I won't get to this for a few weeks still. There's another change I wanted to make to let people handle close frames too. What feature are you trying to ship that relies on this?

@schmidtw
Copy link

I have a protocol that depends on sending & receiving websocket pings and pongs. Each side of the connection expect a ping/pong within an interval & if either doesn't get the ping or pong from the other side the connection is determined to be broken & application logic notes the reason, closes down immediately & the client side will reconnect. The pings and pongs are reported as metrics as well. This is the project: https://xmidt.io/

Since this is weeks out & behind other PRs and the dev branch, it makes sense for me to copy the code, apply this patch & use it locally in my project. When these features are available upstream I can delete the code from the project & move back to this code. I'm not interested in staying on a forked copy for any longer than is needed.

@nhooyr
Copy link
Contributor

nhooyr commented Mar 29, 2024

Makes sense. Yes, please fork the code for now.

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 this pull request may close these issues.

3 participants