search

interledgerjs / ilp-plugin-btp

This has been moved to the monorepo https://github.com/interledgerjs/interledgerjs
8 stars 7 forks source link

Add heartbeat to websocket connection #60

Closed rhuairahrighairidh closed 5 years ago

rhuairahrighairidh commented 5 years ago

Adds a heartbeat to the websocket connection to keep the connection alive.

spearl commented 5 years ago

What websocket service are you using that does keep-alive based on websocket activity?

If you're using say a GCP load balancer, the timeout for websockets is the same as the HTTP timeout which is tunable but defaults to 30s. And as @sentientwaffle said above a heartbeat does not change this behavior as it does not inspect websocket activity.

kincaidoneil commented 5 years ago

@spearl @sentientwaffle Right now we're just using nginx with no load balancer, so it would help us.

As an aside, Heroku and other providers I think do inspect WebSocket activity:

https://devcenter.heroku.com/articles/websockets#timeouts

The normal Heroku HTTP routing timeout rules apply to WebSocket connections. Either client or server can prevent the connection from idling by sending an occasional ping packet over the connection.

WebSocket capable endpoints support HTTP keep-alive. Refer to the documentation for the Heroku router for complete details.

https://devcenter.heroku.com/articles/http-routing#timeouts

After the initial response, each byte sent (either from the client or from your app process) resets a rolling 55 second window. If no data is sent during this 55 second window then the connection is terminated and a H15 or H28 error is logged.

They also offer session affinity using HTTP cookies so each the websocket is sticky to the same instance.

kincaidoneil commented 5 years ago

Also, if the client's network goes down, but the plugin isn't sending any messages, sometimes it doesn't actually know the connection dropped. We've been encountering this issue with Switch.

A plugin receiving packets cannot be reached, which causes all packets sent to it to be rejected (since the plugin itself wasn't sending any messages, no reconnect would be triggered).

I just tested that this PR resolves that issue, terminating the connection when the ping fails, which triggers a reconnect.

@sentientwaffle Can we merge this?

kincaidoneil commented 5 years ago

Thanks for the ack, and sorry for the oversight on my part; upped the interval to 20 seconds.