support persistent websocket

[DanboDuan]

Signed-off-by: bob bob170731@gmail.com

What it does

Support persistent websocket connection, which allow network to be weak and will not fire close event for shortly disconnected.

How to test

just open an ide in browser and fake network weakness by turning the computer with network-off mode, then turn on the network and the ide should continue to work fine.

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[DanboDuan]

for #11271

[DanboDuan]

would you like to help review this @msujew @paul-marechal

[paul-marechal]

I think these kind of changes require a much more thorough explanation of how it works.

I don't want to reverse engineer code to guess things like: How are connections identified? How are "backend sessions" persisted? How long may a session persist? Is there anything worth mentioning regarding the approach taken?

I agree that the connection infrastructure is in need of more robustness, but please help us understand your solution.

[DanboDuan]

I think these kind of changes require a much more thorough explanation of how it works.

I don't want to reverse engineer code to guess things like: How are connections identified? How are "backend sessions" persisted? How long may a session persist? Is there anything worth mentioning regarding the approach taken?

I agree that the connection infrastructure is in need of more robustness, but please help us understand your solution.

The reason for the unreliable message is that the message layer close the socket once the socket.io disconnects.

    protected toIWebSocket(socket: Socket): IWebSocket {
        return {
            close: () => {
                socket.removeAllListeners('disconnect');
                socket.removeAllListeners('error');
                socket.removeAllListeners('message');
                socket.disconnect();
            },
            isConnected: () => socket.connected,
            onClose: cb => socket.on('disconnect', reason => cb(reason)),
            onError: cb => socket.on('error', error => cb(error)),
            onMessage: cb => socket.on('message', data => cb(data)),
            send: message => socket.emit('message', message)
        };
    }

so if we do not close the socket at once and try to reconnect from frontend to backend, we can keep the message reliable.

So we do as following:

1. Generate a unique persistent_key for socket.io URL
2. The backend keep a map of socket.io. once a socket connected, check if it's reconnection or first connecting using the persistent_key
3. Once the socket.io disconnected, wait for a certain timeout to emit the close event while the frontend might re-connect

So we build a PersistentWebSocket to hold the underlying socket.

[paul-marechal]

Given the current ReconnectionGraceTime set at 10 min, do I understand correctly that we will leak backend-scoped inversify containers that will live for 10 min each time the user reloads the browser? This means any connection-scoped process like the plugin host process, language server processes, etc? If so I'm not a fan.

[DanboDuan]

Given the current ReconnectionGraceTime set at 10 min, do I understand correctly that we will leak backend-scoped inversify containers that will live for 10 min each time the user reloads the browser? This means any connection-scoped process like the plugin host process, language server processes, etc? If so I'm not a fan.

that is why we could close other no-connected connections while some connection re-connect to stop waiting

[safisa]

Hi,
Any updates on this PR?

This change could help us a lot with disconnections issues since today after reconnecting all the plugins are reloaded/reactivated again and this takes a lot of time (e.g. RedHat Java plugins) also we have a vscode plugin that uses many webviews and custom editors that are fully reloaded cause bad UX.

[tortmayr]

I'm happy to take another look at this PR.
@DanboDuan I know that this PR has been stale for a while, nevertheless I hope that you are still able/willing to work on this. If so could you please rebase this PR to resolve the conflicts?

Given the current ReconnectionGraceTime set at 10 min, do I understand correctly that we will leak backend-scoped inversify containers that will live for 10 min each time the user reloads the browser? This means any connection-scoped process like the plugin host process, language server processes, etc? If so I'm not a fan.

I have to take a deeper look into this but I'm pretty confident that we can find a way to let us distinguish between graceful/intended websocket connection closes and actual connection loses. This way we could avoid leaking of backend-scoped containers on reload.

@safisa
Could you please eloborate on the disconnection issues that you are encoutering recently? This PR is definitely a big step into the right direction but I also want to make sure that there are no additional underlying problems that should be addressed.

[DanboDuan]

I'm happy to take another look at this PR. @DanboDuan I know that this PR has been stale for a while, nevertheless I hope that you are still able/willing to work on this. If so could you please rebase this PR to resolve the conflicts?

Given the current ReconnectionGraceTime set at 10 min, do I understand correctly that we will leak backend-scoped inversify containers that will live for 10 min each time the user reloads the browser? This means any connection-scoped process like the plugin host process, language server processes, etc? If so I'm not a fan.

I have to take a deeper look into this but I'm pretty confident that we can find a way to let us distinguish between graceful/intended websocket connection closes and actual connection loses. This way we could avoid leaking of backend-scoped containers on reload.

@safisa Could you please eloborate on the disconnection issues that you are encoutering recently? This PR is definitely a big step into the right direction but I also want to make sure that there are no additional underlying problems that should be addressed.

@tortmayr I'm now away. But @FinnChen would continue to help it.

[safisa]

Hi @tortmayr

This disconnection can cause several issues, but I am not sure about all of them. I will try to mention the most annoying issues we faced:

1. biggest issue we faced, is the web views and custom editors we are using, those web views are reloaded (on online), and this is a very bad UX.
2. vscode plugins are activated again on online. Some of the plugins have issues after this re-activation. E.g. we have a log-viewer (berublan.vscode-log-viewer-0.13.1), that stopped working after re-connection.
3. re-activations of vscode plugins take time and resources (bad UX), e.g try the RedHat java plugin. it is a heavy one and takes some time to be loaded (not sure about the side effects of this re-activation on its functionality).

Thanks

[safisa]

Hi @DanboDuan,

First thank you for this work in the PR.
I want to know if you want to continue working on it. We are wishing to have support for persistent websockets in Theia to help us resolve many issues like the ones mentioned above.

Thanks in advance

[msujew]

I am not totally sure whether this is the right approach to resolving the issues that have been brought up. I agree that something needs fixing and I appreciate the work that went into this PR, but keeping the connection alive (and everything associated with it, like the plugin host) seems a bit heavy handed for me.

I would much rather see a clean reconnection (including a restart of the plugin host) than trying to keep everything alive. I won't prevent other committers from reviewing/approving this PR, but I'm more inclined to close this to find another solution to the issues. Unless there are objections, I'll close this in the near future.