Skip to content

Ping Behavior Clarification #226

Closed
Closed

Description

@JoshStrobl

Hey there,

I'm currently having some issues understanding what the expected implementation is for handling pings and pongs in a repeated fashion, primarily being used as a "heartbeat" indicator so I can free specific connections so I'm not sending data to socket connections which aren't active. I read over #186 and took a look at both of the examples in the repo and am trying to determine a few things:

I'm using wsjson.Read to read in requested data, as opposed to using the Reader / Read functions. This data is a JSON marshalled form of the struct listed below:

type WebSocketEvent struct {
	IDs  SocketKeys `json:"ids"`
	Type string     `json:"type"`
}

This is being read in as follows:

var request WebSocketEvent
if readErr := wsjson.Read(r.Context(), socketConn, &request); readErr != nil { // Read contents
	return
}

The type has a value, whether that's "init", "register", or "unregister". During "init", we'll perform some mapping and then call a go routine called Ping, providing the pointer the websocket.Conn. The desired implementation is basically to ping every 30 seconds or so and then after a few failures it'll unlisted it.

Is the pong response supposed to be automatically handled by the call to the connection's Ping function? Is this something that would be in the response handler (e.g. func SocketHandler(w http.ResponseWriter, r *http.Request)) instead? Or am I just missing something or going about it the wrong way.

Here's something I was testing, not actually functional but should give some idea on the context:

func Ping(c *websocket.Conn) {
	for {
		ctx, _ := context.WithTimeout(context.Background(), time.Second*15)
		trunk.LogInfo("Pinging client")

		pingErr := c.Ping(ctx)

		if pingErr == nil {
			trunk.LogSuccess("Pinged client")
			time.Sleep(time.Second * 30)
		} else {
			trunk.LogErr("Failed to ping client")
			RemoveConn(socketConn, "all", "all")
			return
		}
	}
}

Thanks in advance for your help!

Activity

  1. nhooyr commented on Apr 22, 2020

    @nhooyr
    Contributor

    Hi,

    Is the pong response supposed to be automatically handled by the call to the connection's Ping function

    Ping will block until the goroutine in wjson.Read reads the pong from the peer or the deadline is hit.

    I've opened #227 to add an example to make this more clear.

  2. nhooyr commented on Apr 22, 2020

    @nhooyr
    Contributor

    So your code should work as long as Ping runs in a different goroutine than the one in wsjson.Read.

  3. JoshStrobl commented on Apr 22, 2020

    @JoshStrobl
    Author

    Holy smokes that was a quick response, thanks for the additional information. Here are the two functions, do you see anything obvious that would indicate why at least in Firefox it isn't indicating ping / pong responses in its Messages tab (it seems to receive a ping once, send a pong, then nothing after that).

    Screenshot from 2020-04-23 01-06-11

    // Ping the client will attempt to regularly ping the client
    func Ping(c *websocket.Conn) {
    	for {
    		ctx, _ := context.WithTimeout(context.Background(), time.Second*15)
    		trunk.LogInfo("Pinging client")
    
    		pingErr := c.Ping(ctx)
    
    		if pingErr == nil {
    			trunk.LogSuccess("Pinged client")
    			time.Sleep(time.Second * 30)
    		} else {
    			trunk.LogErr("Failed to ping client")
    			RemoveConn(socketConn, "all", "all")
    		}
    	}
    }
    
    // SocketHandler will handle any request to registration and unregistration of a WebSocket Connection
    func SocketHandler(w http.ResponseWriter, r *http.Request) {
    	socketConn, acceptErr := websocket.Accept(w, r, &websocket.AcceptOptions{
    		Subprotocols: []string{
    			"sbd-v0.1", // 0.1
    		},
    		InsecureSkipVerify: core.Conf.Servers.WebSockets.DisableOriginVerification,
    	})
    
    	if acceptErr != nil {
    		return
    	}
    
    	var request WebSocketEvent
    
    	if readErr := wsjson.Read(r.Context(), socketConn, &request); readErr != nil { // Read contents
    		return
    	}
    
    	if request.Type == "init" {
    		initErr := SocketInitHandler(socketConn, request)
    
    		if initErr == nil {
    			go Ping(socketConn)
    		}
    	} else if request.Type == "register" { // Register
    		SocketRegisterHandler(socketConn, request)
    	} else if request.Type == "unregister" { // Unregister
    		SocketUnregisterHandler(socketConn, request) // Call to unregister
    	} else { // Neither type
    		socketConn.Close(websocket.StatusUnsupportedData, "Type specified is invalid")
    	}
    }

    So is wsjson.Read expected to fail unmarshalling the pong given it isn't actually that struct?

  4. nhooyr commented on Apr 22, 2020

    @nhooyr
    Contributor

    Holy smokes that was a quick response, thanks for the additional information.

    🚀

    Here are the two functions, do you see anything obvious that would indicate why at least in Firefox it isn't indicating ping / pong responses in its Messages tab (it seems to receive a ping once, send a pong, then nothing after that).

    You need to keep reading from the connection in SocketHandler. You can use CloseRead if you don't expect any data messages as is your case.

    Also, not sure if it's just the example code you've provided but there are codepaths where the connection isn't closed. I'd recommend using CloseRead after you read the JSON and then calling Ping in the SocketHandler goroutine instead of a separate one.

    So is wsjson.Read expected to fail unmarshalling the pong given it isn't actually that struct?

    wsjson.Read just tells the Ping goroutine it's received a pong and then continues reading data messages. It doesn't unmarshal the pong.

  5. JoshStrobl commented on Apr 23, 2020

    @JoshStrobl
    Author

    Hey, not to necro this, but after some tinkering I was able to get pinging and the use of the CloseRead to work as I expected thanks to your advise and pointers. So, thank you. Your time is genuinely much appreciated, both working on this as well as providing support.

  6. nhooyr commented on Apr 23, 2020

    @nhooyr
    Contributor

    Thank you Joshua, I really appreciate that <3

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions