Retry symbol search whilst language server is still initialising

[feltech]

• Langauge servers can take a while to initialise, and this has a
  specific error code associated with it when querying.
• So detect that specific error response and retry symbol search every
  second until it works, or until the search menu is closed.

[feltech]

Specifically, I found this when trying to use the cquery (C++) language server. Opening a .cpp file then <C-S-t> too quickly would lead to a search box with a loading spinner that never stops loading. This was caused by an exception like

Error: /path/to/file.cpp is being indexed.
    at new ResponseError (/path/to/oni/node_modules/vscode-jsonrpc/lib/messages.js:46)
    at handleResponse (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:430)
    at processMessageQueue (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:258)
    at Immediate._onImmediate (/path/to/oni/node_modules/vscode-jsonrpc/lib/main.js:242)
    at runCallback (timers.js:789)
    at tryOnImmediate (timers.js:751)
    at processImmediate [as _immediateCallback] (timers.js:722)

which I traced to a specific error code coming from the language server.

[feltech]

Hmm, those "WARNING: head commit changed" commits are annoying, but harmless (they're empty). I think I accidentally tried to amend a commit I'd already pushed. Not sure how those messages got put there, I've not seen that before. Something to do with eclipse (which I use as a history/conflict viewer), probably.

[akinsho]

@feltech since our CI runs on multiple platforms you might need to use path.join here to make the mock paths platform agnostic

[akinsho]

@feltech thanks for the PR its nice to see the language server get some much needed love and tests as well 😍, re. the retry strategy I'm wondering if its possible to also delay queries till the server is initialised. There is an intialize request sent by the client in the spec and possibly we can await its result before taking other requests?

[codecov]

Codecov Report

Merging #2634 into master will increase coverage by 0.3%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #2634     +/-   ##
=========================================
+ Coverage   45.38%   45.68%   +0.3%     
=========================================
  Files         361      361             
  Lines       14576    14586     +10     
  Branches     1915     1916      +1     
=========================================
+ Hits         6615     6664     +49     
+ Misses       7737     7698     -39     
  Partials      224      224

Impacted Files	Coverage Δ
browser/src/Editor/NeovimEditor/Symbols.ts	64.38% <100%> (+58.03%)	⬆️
...lugins/Api/LanguageClient/LanguageClientHelpers.ts	65.85% <0%> (+2.43%)	⬆️
browser/src/Utility.ts	50.74% <0%> (+3.73%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdc740c...46d619a. Read the comment docs.

[feltech]

Indeed, that initialize sounds like the real answer, I'll look into it.

[feltech]

Unfortunately, at least for cquery, the initialize request seems inadequate. Oni logs "[LanguageClientManager]: Initialized" after the initialize request completes, but we still see the exception after that (see pic of debug console)

[akinsho]

@feltech based on the protocol it seems you have to wait for the InitializeResult to be sent back from the lsp, does the initialize call return with the InitializeResult or is that sent back separately? I think based on this example there might be separate notification event from the lsp when the initialization is complete

[akinsho]

Btw don't let the initialisation issue be a blocker for now, I'm planning to have a look at bits of our lsp client at some point to facilitate things like #1717

[feltech]

Good stuff! I have a couple more PRs incoming, so I'll perhaps take a deeper looks at language server initialize later.

[CrossR]

Can this be merged now then? (I assume it was waiting on some checks when you approved it @Akin909)

[akinsho]

@CrossR was good to merge just forgot to hit the button