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

no doc related to hearbeat use case when connection is for push only events #492

Closed
jjuraszek opened this issue Mar 19, 2019 · 7 comments
Closed

Comments

@jjuraszek
Copy link

jjuraszek commented Mar 19, 2019

right now according to doc you have to read from connection to fire SetPongHandler event. But if the websocket is used for push only it won't be fired. Solution is to use:

go ws.NextReader()

But the doc is not very straight forward about that. In my opinion there should be description of how to setup heartbeat in case of push only usage. Let me know if you need help with that.

@ghost
Copy link

ghost commented Mar 19, 2019

This is covered in the section about control message. The section includes the snippet of code to use.

@jjuraszek
Copy link
Author

The read operation is blocking so c.Close() might not ever happen for unresponsive client. Therefore I think it's not right way to implement heartbeat procedure. I think it should be explicitly described procedure of using:

  • websocket.Conn.SetPongHandler for registration of last PONG time from client
  • websocket.Conn.NextReader() for reassurance that PONG is even obtained
  • websocket.Conn.WriteMessage(websocket.PingMessage, ...) for sending PING and checking if there is no socket issue

I'm not saying that there is no documentation. I'm just saying that this common use case is not covered as whole neither by examples nor by documentation. All breadcrumbs are separated ;)

It takes me at least 2h to figure out how to do it right. So I can create example to save time of others working on similar case. Does it sound reasonable?

@ghost
Copy link

ghost commented Mar 19, 2019

All of the examples except the echo example show how to use ping/pong.

The filewatch example covers the push only scenario (code here).

@garyburd
Copy link
Contributor

The filewatch example covers the scenario. Do we need to make the examples more visible? Does the filewatch example need improvement?

@elithrar
Copy link
Contributor

Closing this one out for now.

@ozgurakcali
Copy link

A dummy reader should not be necessary to be able to respond to ping messages IMO. Even if it is documented, it doesn't feel natural.

@ghost
Copy link

ghost commented Nov 4, 2020

@ozgurakcali That would be nice, but the package does not know if the application will read the connection or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants