77: BaseGLSPClient should handle reconnect

[CamilleLetavernier]

• update to Theia 1.45
• enable support for reconnect with a 24h timeout in the example browser-app
• contribute an example command to test the reconnect behavior (as defined in Introduce timeout for keeping connection contexts alive eclipse-theia/theia#13082)
  • We probably don't want to actually push this command, so I added it as a separate commit. We might want to revert it before actually merging anything.

Note: once the update to Theia 1.45 is in, the only required change is a Theia config option in the application, to actually enable the behavior. I don't think any change in required in GLSP itself. Typically, we want a relatively short timeout for Browser Application (especially if they are deployed to the cloud), to avoid large memory leaks, and an infinite timeout (-1) for Electron applications, as we only ever expect a single connection. I chose a 24-hours timeout, which is pretty long, because this is an example application that is typically deployed locally for testing; so we're closer to the Electron scenario (single user local deployment) than a regular web-app.

Part of eclipse-glsp/glsp#77

[tortmayr]

Should be merged after #196

[tortmayr]

The breaking change was introduced with 1.44.x so this should be "^1.44.0".
I will update this in #196 accordingly

[CamilleLetavernier]

Actually, I need 1.45, because this PR is integrating the changes from eclipse-theia/theia#13082. Unless I'm mistaken, these changes are only available in >= 1.45

Edit: it should even be 1.45.1, because 1.45 contains a memory leak in the reconnect feature

[ndoschek]

Thank you @CamilleLetavernier!
Code looks good to me, and I tested several user scenarios with the interrupt command and it works great! 🎉

[CamilleLetavernier]

Should be merged after #196

#196 has been merged; I've rebased this PR on top. I've also re-introduced the reconnect=true parameter in the glsp-client-contribution

[tortmayr]

@ndoschek Thanks for reviewing this.
@CamilleLetavernier I would like to test some corner cases on my own before we merge this but in general the change looks very good to me.

[tortmayr]

In general the change looks good to me and for the most cases disconnect/reconnect seems to work as expected.

However, there seems to be an issue if the server sends actions/updates while the fronted is disconnected. All actions send from the server in this period are lost.

To test this I modified the example server to send a status action with an incremented counter every second. If I disconnect the frontend for 10 seconds you can see that 10 status updates are lost:

This is especially critical if the client has a promise that his waiting for a server response (e.g. request actions or progress updates). E.g. if you disconnect during the initial model loading nothing happens after reconnect (i.e. the diagram stays empty).

AFAIK Theia has buffering in place to cache messages that are sent via the service socket while disconnected. So I think the issue here is somewhere in our GLSP frontend code.

Nevertheless we could still merge this and work on the mentioned issues in a follow-up PR.

[CamilleLetavernier]

This is especially critical if the client has a promise that his waiting for a server response (e.g. request actions or progress updates). E.g. if you disconnect during the initial model loading nothing happens after reconnect (i.e. the diagram stays empty).

AFAIK Theia has buffering in place to cache messages that are sent via the service socket while disconnected. So I think the issue here is somewhere in our GLSP frontend code.

Thanks for the review and feedback! Indeed, I was focusing on more "local" use cases, where e.g. the computer goes to sleep for a prolonged period of time (which is a very common issue in local development or electron scenarios). In such cases, we are unlikely to have any pending request. However, for real deployment, network failures are a totally realistic scenario we should handle.

Nevertheless we could still merge this and work on the mentioned issues in a follow-up PR.

That would be great! I think this PR already handles a number of use cases, and we can further improve on it in follow-ups.

[CamilleLetavernier]

I've removed the test command commit from the PR.

Edit: we decided to keep the test command in the Workflow Example, so I've restored it again :)