Add a lastFrameReceivedAt timestamp so that dead connections can be detected

[bridger]

This feature allows a server to detect when a websocket has gone stale. It is to fix #24.

Here is an example of how my server uses this API to cleanup old connections:

    func _connectionKeepAliveAndCleanup() {
        let pingInterval: TimeInterval = 60

        for connection in connections {
            connection.ping()
            if connection.lastFrameReceivedAt.numberOfSecondsOld > 2 * pingInterval {
                connection.close(reason: .normal, description: nil)
                self.disconnected(connection: connection, reason: .normal)
            }
        }

        readWriteQueue.asyncAfter(deadline: DispatchTime.now() + pingInterval) {
            self._connectionKeepAliveAndCleanup()
        }
    }

[CLAassistant]

All committers have signed the CLA.

[codecov-io]

Codecov Report

Merging #43 into master will increase coverage by 0.03%. The diff coverage is 100%.

@@            Coverage Diff             @@
##           master      #43      +/-   ##
==========================================
+ Coverage    92.4%   92.43%   +0.03%     
==========================================
  Files           9        9              
  Lines         500      502       +2     
==========================================
+ Hits          462      464       +2     
  Misses         38       38

Flag	Coverage Δ
#KituraWebSocket	92.43% <100%> (+0.03%)	:arrow_up:

Impacted Files	Coverage Δ
Sources/KituraWebSocket/WebSocketConnection.swift	92.46% <100%> (+0.07%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 0583f38...019dbdd. Read the comment docs.

[Andrew-Lees11]

Thank you @bridger for raising this pull request. I have just one comment regarding when lastFrameReceivedAt is set. Otherwise I think this is a good idea to allow clean up of stale connections.

[bridger]

Good question on whether we should count all frames or just pong frames.

I think either approach would work in my case. I went with all frames because the idea was to detect dead connections. If any data is still being received then the connection isn’t dead. Only counting pongs would make sure the connection is working both ways - the ping being sent and the pong being received.

I’m really in the fence on this one. 🤔 Want to make the final call and I’ll implement the change?

[Andrew-Lees11]

@bridger So after some thought, I think your original solution is actually the correct way to go. Having lastFrameReceivedAt on all frames allows you to check for stale connections and then send a verifying ping to connections with old lastFrameReceivedAt instead of pinging all connections. If you are happy with this then I will merge this PR

[bridger]

Cool! Let's merge 🙌

[bridger]

Also, fwiw the memory usage on my websocket server is much better now that dead connections are quickly being cleaned up.

It might be worth making this an easy config to turn on which would do the pinging automatically.

As a rough idea, maybe the API would allow a user to specify a pingKeepaliveInterval and/or a timeoutAfter parameter which would send pings automatically and close a connection that isn't responding.

On the other hand, it isn't difficult to implement this in the client code like I did. Maybe just an example in the docs would be the best way.

[Andrew-Lees11]

@bridger So we were looking at how to implement that and have made a gist to handle dead connections for our example chat server. This adds it's own "heartbeat" instead of having one within the websocket connection. Would you be able to have a look at this and see if it works for your case? Once this is neatened up we will make a blog post and add it to the readme so other people can see how to implement this.

[bridger]

The heartbeat timestamp in the gist doesn't get updated when a pong frame is received, so it will only keep a connection alive that has sent other data. In the gist it still sends a ping to the connection, but I don't think that will have any effect on keeping the connection alive.

In my case I don't want to close connections that have been idle - just ones that are actually dead. If the client hasn't sent data in a long time but still responds to a ping then I'd like to keep the connection open.

[Andrew-Lees11]

@bridger Implementation using timers and a tunable is implemented on this branch. Would you be able to have a look and see if this would fix your problem.

[Andrew-Lees11]

Thank you for working on this but we are closing this pr since this was fixed in this pr.