[websocket] Implement ping/pong for maintaining websockets through proxies.

[Zate]

I'd like to raise a few issues around the websocket implementation and the troubles it causes systems behind a proxy.

It's highly likely people will end up running Theia behind some kind of proxy, be that nginx or something like traefik or similar at the head of a k8s cluster etc. In those situations the proxies general enforce a timeout for sockets not in use. The websocket RFC contains a mechanism called ping/pong that maintains traffic through a websocket to keep it open. Those calls are included in websockets/ws that is being used by Theia but they are not implemented in the server/client to keep sockets open.

There does seem to be a lib being used on the client side to reopen closed sockets that may have been implemented to help deal with this issue. Ideally a websocket should remain open as long as the program is in use if ping/pong is done correctly unless some kind of network interruption is encountered. In that case being able to re-open the socket is useful, but it should only happen if the socket is closed. As it stands right now, after a period of inactivity the sockets are closed and re-opened and in my testing this can sometimes lead to functionality in the client being broken.

I've also noticed that the terminal instance does not use keep alives and doesn't get re-opened as it seems to be using a different mechanism to do it's sockets (not vscode-ws-jsonrpc).

I think the appropriate place to solve this issue might be in this file and do a setInterval() to handle sending a ping every 30-60s (configurable) across all open sockets. I've tried playing with this in my own fork but I am very new to typescript and javascript and thought it would perhaps be faster to open an issue on it. I can certainly learn from the implementation used if this is addressed.

End result is I think the site should continue to use the same sockets (and not open new ones) until forced to by a disconnect/breakage and the keep alive time be something that can be configurable ( to adjust for differing proxy timeout values you might not have control over). A default of 30s would be appropriate I fee.

I think this is related to #979

[kittaakos]

There is a package out there that seems to do what we need: it's called wska.

[Zate]

That looks like it might do the trick, would that replace reconnecting-websocket on the client side? I'm also unsure whether the ping/pong needs to be delivered through the connection as jsonrpc2 or not, or if I am just not grokking how its working.

[simark]

Hi @Zate. I'll start looking into the issue, but I'll also need to learn the technical details of websockets. If you want to submit a patch as a learning experience, you are very welcome. It doesn't have to be perfect :)

[Zate]

I will give that a shot thanks :)

[Zate]

I managed to replace reconnecting-websocket with wska and resolve a couple of issues but I'm stuck at something to do with crypto and the browser when it goes to create the websocket. Trying to work out how to use crypto-browserify to perhaps fix it.

[kittaakos]

I managed to replace reconnecting-websocket with wska and resolve a couple of issues

👍

but I'm stuck at something to do with crypto and the browser when it goes to create the websocket.

Could this thread help to resolve your issue? It recommends marking crypto as an external module.

If we you want to modify the generated webpack configuration, you can look into the generator here.

[Zate]

Thank you! I think that is exactly what I am after. Will test that here in a bit.

[svenefftinge]

There does seem to be a lib being used on the client side to reopen closed sockets that may have been implemented to help deal with this issue.

It is there to reconnect on connection loss. So I think the ping/pong and reconnecting are both needed. It would be nice if the terminal would auto-reconnect, too.

[Zate]

It is there to reconnect on connection loss. So I think the ping/pong and reconnecting are both needed. It would be nice if the terminal would auto-reconnect, too.

Agreed, the wska module claims to do both. I'll see once I can get it to run heh :)

[Zate]

I have tried many different things. Tried to add crypto to the "externals" in webpack-generator.ts as in that link above, tried using browserify / crypto-browserify in many different ways. Back to just doing the externals and getting this :

Error: TypeError: crypto.randomBytes is not a function
    at createWebSocket (index.js:45)
    at new WebSocketKeepAlive (index.js:37)
    at WebSocketConnectionProvider.createWebSocket (connection.ts:72)
    at WebSocketConnectionProvider.listen (connection.ts:42)
    at WebSocketConnectionProvider.createProxy (connection.ts:30)
    at Binding.dynamicValue (logger-frontend-module.ts:27)
    at resolver.js:63
    at invokeFactory (resolver.js:10)
    at resolver.js:63
    at Array.map (<anonymous>)

Going to keep trying and looking but there is so much about how node/npm/yarn/typescript works that I do not understand. I'm not able to have vscode on my chromebook so being able to host this myself and write Golang in a browser would be great :) (do not want to use dev mode).

[kittaakos]

@Zate, please create a PR with your changes (as is), add the [WIP] prefix to the PR name (so that people know the work is still in progress) and we can check.

If that is a build/config issue, I can take a look.

[Zate]

Created a PR with the changes I ended up on. I removed a great deal of cruft out of it as I tried a whole bunch of things but this is ultimately where I have it now. #1431

[kittaakos]

Did not work for me either. I have opened a follow-up here.

Maybe, we should try to implement our own ping/pong mechanism either built on the top of ws or pladaria/reconnecting-websocket.

[Zate]

ws has the functions for ping/pong existing in it, I just couldn't find the right place to put them. It feels like we'd need a setInterval() loop on socket creation to send a ping from the server every 30s (or what ever we decide). I was also not sure if vscode-ws-jsonrpc was getting in the way in some way. Sorry I understand what needs to happen but not enough about js/ts to implement it.

we could leave the current reconnecting-websocket on the client side and simply implement ping on the server side I think.

[akosyakov]

Why don't do https://github.com/websockets/ws#how-to-detect-and-close-broken-connections here: https://github.com/theia-ide/theia/blob/592c54b90ebcf1357f1e6bbf7ac395f062ea50ba/packages/core/src/node/messaging/connection.ts#L43

It is used by json-rpc as well as by terminals.

[Zate]

I had tried that before and couldn't get it to work. It wasn't erroring, it just wasnt emitting pings. I will give it another shot and paste back what I come up with so someone can see if I am making some kind of silly mistake.

[Zate]

So a couple of updates. I got ping/pong working for all the websockets, even terminal. I just need to clean up the code a little and I can submit a PR. On the other hand, I worked out my specific issue is Cloudflare. They have a hard limit on websocket connections of 100 seconds. It disconnects and then the reconnecting-websockets reconnects. The terminal socket doesn't reconnect.

There are some issues with the code I've put in, I think it's not good from a performance/resource usage perspective and it seems to ping/pong too often, or has multiple of them running per websocket. It does help with keeping sockets open with no deliberate tampering such as cloudflare in the way, but I've yet to test if that was happening anyhow.

So I plan to redo the swarm I am hosting these docker containers on and move cloudflare out of the way to see if it still happens. If it does in code without my fix, I'll clean up my code and do a PR, if it doesn't actually happen with CF out of the way and the traefik proxy in place then I can assume the issue is Cloudflare and only related to that and no fix is required.

[simark]

So a couple of updates. I got ping/pong working for all the websockets, even terminal. I just need to clean up the code a little and I can submit a PR. On the other hand, I worked out my specific issue is Cloudflare. They have a hard limit on websocket connections of 100 seconds. It disconnects and then the reconnecting-websockets reconnects. The terminal socket doesn't reconnect.

There are some issues with the code I've put in, I think it's not good from a performance/resource usage perspective and it seems to ping/pong too often, or has multiple of them running per websocket. It does help with keeping sockets open with no deliberate tampering such as cloudflare in the way, but I've yet to test if that was happening anyhow.

So I plan to redo the swarm I am hosting these docker containers on and move cloudflare out of the way to see if it still happens. If it does in code without my fix, I'll clean up my code and do a PR, if it doesn't actually happen with CF out of the way and the traefik proxy in place then I can assume the issue is Cloudflare and only related to that and no fix is required.

In our case we're using the traefik proxy too and suspect it to drop the inactive websocket connections. Please keep us updated with what you find!

[Zate]

New PR for this : #1534

[Zate]

This has been completed and merged.