Add heartbeat support

[davoclavo]

The heartbeat interval can be configured in the socket_opts, it is measured in milliseconds. If set to nil, heartbeat messages will not be sent.

[sasa1977]

Thank you for this pull!

I think we should check the purpose of heartbeat with Chris first, because I'm not really sure if it's needed.

You can already have long-running idle connections today by providing transport_opts: [keepalive: :timer.seconds(30)] as the socket_opts argument of GenSocketClient.start_link/5 (i.e. the 4th argument). This is admittedly not documented, so I think that's the first thing we should do. Perhaps a 30 seconds keepalive should also be a default, so idle connections are long-running out of the box, and then, if a different behaviour is called for, it can be configured by explicitly passing the keepalive setting.

The keepalive mechanism uses the standard WebSocket ping/pong feature. IMO, that serves exactly the same purpose as Phoenix heartbeating, but requires a bit less code in this library, and doesn't rely on a custom Phoenix protocol. CPU usage on the server should also be slightly reduced, since the ping message doesn't even hit Phoenix, but is instead handled by Cowboy.

On the other hand, there's no doubt that this pull makes client behave more like other clients (e.g. the canonical JS one). However, it's still not exactly the same behaviour. The code in this PR always uses heartbeats, regardless of the transport. In contrast, Phoenix JS client skips heartbeats in long-polling, because it doesn't make sense there. So IMO, if we decide to use Phoenix heartbeating, we should probably make it transport specific.

But if a heartbeat is transport specific, and the only transport which uses it is websocket, then I don't see why standard WS ping/pong wouldn't suffice. Perhaps Phoenix team uses heartbeats because some browsers/clients don't send WS pings themselves, so heartbeating ensures consistent behaviour. If that's the case, then I think we don't have that problem here, because there aren't so many different transports, and I think it's sensible to make ping/heartbeat a transport responsibility.

I'm going to discuss this with Chris, and then we can decide what's the best way forward.

[sasa1977]

OK, I just chatted with Chris, and he says that the reason for the custom heartbeat protocol is because in browsers you can't configure the WS ping interval. In fact after a quick (and shallow) search, my impression is that in browsers WS pinging either works implicitly with a hardcoded interval, or doesn't work at all, depending on the implementation. That explains why a browser client needs to use the custom Phoenix heartbeat protocol.

However, in Elixir client we don't face this issue, since the WS client library we use supports configurable WS pings. So I'd suggest using the existing mechanism (keepalive mentioned in the previous comment). We actually use that in our own system to establish a long-running mostly idle connection, and it works fine for us.

Moreover we should document the keepalive option, and also consider turning it on by default.

If we ever reach a situation where Phoenix heartbeat is really needed, then we can reconsider using that mechanism. If we're doing that, I think usage of heartbeat should be transport-specific. Since transports are currently Phoenix agnostic, there is a certain conflict here which would complicate the code to some extent. This is yet another reason why I'd like to avoid introducing Phoenix heartbeats as long as current mechanisms (such as WS ping) can solve the problem.

If the proposed solution doesn't work for you, or you have some pro-heartbeat arguments, let us know.

[sasa1977]

@davoclavo do you have some comments on my proposals? If not, I'll close this pull.

[davoclavo]

@sasa1977 Thanks a lot for your detailed answer! Sorry I didn't reply earlier, had a bunch of stuff going on at work. I think your suggestion is the way to go, we should not have the client handle the hearbeats, but instead rely on the keepalive option supported by the underlying websocket client.