Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reconnect with engine with exponential backoff #8520

Closed
Tracked by #5202
farmaazon opened this issue Dec 12, 2023 · 13 comments · Fixed by #9691
Closed
Tracked by #5202

Reconnect with engine with exponential backoff #8520

farmaazon opened this issue Dec 12, 2023 · 13 comments · Fixed by #9691
Assignees
Labels
-gui d-easy Difficulty: little prior knowledge required p-medium Should be completed in the next few sprints x-new-feature Type: new feature request
Milestone

Comments

@farmaazon
Copy link
Contributor

farmaazon commented Dec 12, 2023

Add logic for reconnecting with language server (with the same host and port as before) when the connection is closed, without restoring any state to make client working (this will be implemented in #8522 and #8523).

@farmaazon farmaazon added d-easy Difficulty: little prior knowledge required p-medium Should be completed in the next few sprints x-new-feature Type: new feature request -gui labels Dec 12, 2023
@farmaazon
Copy link
Contributor Author

Refinement notes:

@enso-bot
Copy link

enso-bot bot commented Mar 13, 2024

Paweł Grabarz reports a new STANDUP for yesterday (2024-03-12):

Progress: Working on handling reconnection of whole project manager, cleaned up application entrypoint. It should be finished by 2024-03-12.

@farmaazon farmaazon assigned farmaazon and unassigned Frizi Mar 28, 2024
@enso-bot
Copy link

enso-bot bot commented Apr 5, 2024

Adam Obuchowicz reports a new 🔴 DELAY for today (2024-04-05):

Summary: There is 34 days delay in implementation of the Reconnect with engine with exponential backoff (#8520) task.
It will cause 0 days delay for the delivery of this weekly plan.

Delay Cause: The task was investigated by Paweł, and then left aside for a while.

@enso-bot
Copy link

enso-bot bot commented Apr 5, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-05):

Progress: 4 pages of notifications + some reviews. Then I took the "reconnecting with engine" issue and reproduced problem on my linux - indeed we loose connection and do not restore it (it doesn't refresh page automatically, as on Paweł's Windows). It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. implement reconnecting and test it.

@enso-bot
Copy link

enso-bot bot commented Apr 9, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-04-08):

Progress: Tried to find how to react for websocket closing; found one existing implementation of automatically-reconnecting WS, added it to our implementation; it works, but we need also to reinitialize protocol It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. Add automatic protocol re-initialization

@enso-bot
Copy link

enso-bot bot commented Apr 9, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-09):

Progress: Refactored LanguageServer class to handle initialization automatically. Got stuck at some heavy refactoring in ydocs code: started with merging "exponential backoff" implementation, then migrating all exceptions to Results in ydocs project. It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. Finish refactoring and finally see the reconnecting working.

@enso-bot
Copy link

enso-bot bot commented Apr 11, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-04-10):

Progress: Updated rest of code to handle Resuts instead of exceptions. Tested the solution: we reconnect and reinitialize after hibernation, but still more detailed testing is required It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. Test package and hopefully open a PR

@enso-bot
Copy link

enso-bot bot commented Apr 11, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-11):

Progress: Fixed issues arisen after refactoring; still one remains: a strange behavior on project close (but only on electron) It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. Fix the issue.

@enso-bot
Copy link

enso-bot bot commented Apr 12, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-12):

Progress: Fix remaining issues and tested with the newest bookclub. Created a PR. Only one problem remains: the data mock seems to not working properly and casing errors in e2e tests. It should be finished by 2024-04-15.

Next Day: Next day I will be working on the same task. Fix last isse and apply review comments.

@enso-bot
Copy link

enso-bot bot commented Apr 16, 2024

Adam Obuchowicz reports a new 🔴 DELAY for today (2024-04-16):

Summary: There is 4 days delay in implementation of the Reconnect with engine with exponential backoff (#8520) task.
It will cause 4 days delay for the delivery of this weekly plan.

I take a bit of margin, as the debugging races is hard and my estimates are unreliable.

Delay Cause: My PR triggered some races in E2E tests involving visualization chooser; they are really hard to debug.

@enso-bot
Copy link

enso-bot bot commented Apr 16, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-16):

Progress: Today investigated the failing E2E tests. Unfortunately, they are a hard nut to crack, as the fails are irregular and mostly happens on release builds. It should be finished by 2024-04-19.

Next Day: Next day I will be working on the same task. Finish tests debugging. Or take a break and finish the restoring of Execution Context

@enso-bot
Copy link

enso-bot bot commented Apr 18, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-04-17):

Progress: Got review and applied (or answered) all requested changes. Found the way how to manage flacky tests: In one case we wait visualization to actually load (so there's no longer "LoadingVisualization"), in the second we wait until vis is updated to newest expression. Also on this occasion fixed the vis preview not being "proper" JSON visualization, and handle errors when parsing LS responses. It should be finished by 2024-04-19.

Next Day: Next day I will be working on the same task. Check why AI checks are failing and the second round of review

@enso-bot
Copy link

enso-bot bot commented Apr 18, 2024

Adam Obuchowicz reports a new STANDUP for today (2024-04-18):

Progress: Found why AI tests are failing and fixed them. PR now waits for merge. Fixed some GUI issues with widgets which appeared on James's branch. Also updated Execution Context restoring (deduplicated some code). It should be finished by 2024-04-19.

Next Day: Next day I will be working on the #8522 task. Finish the execution context restoring. Test if ydocs server works afterbeing hibernated. If all will be ok, create a PR.

@mergify mergify bot closed this as completed in #9691 Apr 19, 2024
mergify bot pushed a commit that referenced this issue Apr 19, 2024
Fixes #8520

If the websocket is closed not by us, we automatically try to reconnect with it, and initialize the protocol again. **Restoring state (execution contexts, attached visualizations) is not part of this PR**.

It's a part of making IDE work after hibernation (or LS crash).

# Important Notes
It required somewhat heavy refactoring:
1. I decided to use an existing implementation of reconnecting websocket. Replaced (later discovered by me) our implementation.
2. The LanguageServer class now handles both reconnecting and re-initializing - that make usage of it simpler (no more `Promise<LanguageServer>` - each method will just wait for (re)connection and initialization.
3. The stuff in `net` src's module was partially moved to shared's counterpart (with tests). Merged `exponentialBackoff` implementations, which also brought me to
4. Rewriting LS client, so it returns Result instead of throwing, what is closer our desired state, and allows us using exponentialBackoff method without any wrappers.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui d-easy Difficulty: little prior knowledge required p-medium Should be completed in the next few sprints x-new-feature Type: new feature request
Projects
Status: 🗄️ Archived
Development

Successfully merging a pull request may close this issue.

5 participants