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

GUI2 times out connecting to Language Server on large projects #9347

Closed
somebody1234 opened this issue Mar 11, 2024 · 4 comments · Fixed by #10542
Closed

GUI2 times out connecting to Language Server on large projects #9347

somebody1234 opened this issue Mar 11, 2024 · 4 comments · Fixed by #10542
Assignees
Labels
--bug Type: bug -gui p-high Should be completed in the next sprint

Comments

@somebody1234
Copy link
Contributor

Repro steps

  1. Create a new project from "Colorado COVID" template on the Cloud backend
  2. Wait for the project to open

Expected result

No errors

Actual result

Connecting to Language Server times out

Details

Console logs:
localhost-1709807376658.log

WebSocket messages:
Note that the first reply from the backend arrives 17 (!) seconds after the request. This is higher than the 15 second limit, which is demonstrated by the second request being sent after roughly 16 seconds.
image

Console logs (screenshots):
image

image

@somebody1234 somebody1234 added p-medium Should be completed in the next few sprints --bug Type: bug -gui labels Mar 11, 2024
@AdRiley AdRiley added this to the Beta Release milestone Mar 11, 2024
@farmaazon
Copy link
Contributor

I think the timeout itself is not much a problem; but the retry should properly interpret the 6002 error, and assume that the protocol is initialized.

There is however a problem here, namely that we need to know the response of initProtocol, because this is the only source of content roots. We could make sure we handle every response of initProtocol, even if it's for timed out request, but I'm not sure if libraries we use allows that.

I think the proper solution is to add an endpoint to language server file/roots allowing us to ask for them if needed. @4e6 are you able to add one?

@farmaazon farmaazon added the s-info-needed Status: more information needed from submitter label Mar 12, 2024
@farmaazon
Copy link
Contributor

farmaazon commented Mar 14, 2024

Refinement notes:

  • For now, let's increase the timeout. We can check if it's possible to retrieve proper response on 6002 error.
  • Estimate includes possible unit tests for handling the error code, what may require slight refactoring.

@farmaazon farmaazon moved this from ❓New to 📤 Backlog in Issues Board Mar 14, 2024
@4e6
Copy link
Contributor

4e6 commented Mar 22, 2024

@farmaazon errors can have a payload field with arbitrary JSON. We can even send the context roots with the 6002 Session already initialised error. Adding the file/roots endpoint is also possible

@AdRiley AdRiley modified the milestones: Beta Release, 2024-07 Release Jun 15, 2024
@farmaazon farmaazon added p-high Should be completed in the next sprint and removed s-info-needed Status: more information needed from submitter p-medium Should be completed in the next few sprints labels Jul 10, 2024
@farmaazon farmaazon self-assigned this Jul 11, 2024
@farmaazon farmaazon moved this from 📤 Backlog to ⚙️ Design in Issues Board Jul 11, 2024
@farmaazon farmaazon moved this from ⚙️ Design to 🔧 Implementation in Issues Board Jul 11, 2024
@enso-bot
Copy link

enso-bot bot commented Jul 19, 2024

Adam Obuchowicz reports a new STANDUP for yesterday (2024-07-18):

Progress: Returned for a while to my modification of LS protocol, allowing us to recover from "already initialized" error. Improved solution a bit, fixed tests and documentation. the PR is ready to review; also started the next task of improving look of multiselect drop-down. It should be finished by 2024-07-22.

Next Day: Next day I will be working on the #9617 task. Finish new task

@farmaazon farmaazon moved this from 🔧 Implementation to 👁️ Code review in Issues Board Jul 19, 2024
@github-project-automation github-project-automation bot moved this from 👁️ Code review to 🟢 Accepted in Issues Board Jul 23, 2024
@farmaazon farmaazon moved this from 🟢 Accepted to 🗄️ Archived in Issues Board Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
--bug Type: bug -gui p-high Should be completed in the next sprint
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants