autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 273 forks source link

Add support for automatic server restart on crash #1113

Closed martskins closed 3 years ago

martskins commented 3 years ago

This PR adds support for automatic server restart retries on crash.

The way this is implemented it allows the user to specify whether or not to try to restart the server on crash via the option LanguageClient_restartOnCrash (which defaults to 1). Additionally, the user can specify a max number of retries via LanguageClient_maxRestartRetries (which applies for each languageId separately, and defaults to 5).

The restarts follow a very naive exponential backoff, I could have added a new dependency on a library that implements a better backoff logic, but I think this simple approach should be enough.

Also, when a server crashes, the client will emit a new event LanguageServerCrashed.

Note that the attempted restart gets the current file name for the server initialisation, so if for example, your Go server crashes, but you are currently editing a JS file, things might go wrong.

Closes #1110, #320 and #774.

jez commented 3 years ago

I haven't yet tried this out, but I will later (maybe tomorrow).

The question I will be trying to answer:

Let's say that the server crashes. I wrote an autocmd to handle LanguageServerCrashed, and I set a variable g:it_crashed that gets shown in a statusline to indicate as such.

Then after 3 retries, the server process comes back (maybe the user tapped a yubikey, maybe the network came back online, maybe a VPN reconnected, etc.) What information is available for me to know that I can overwrite the g:it_crashed variable to start showing a good icon in the statusline?

For example, I believe that LanguageServerStarted is emitted when the bin/languageclient process starts, but not when the launguage server spawn happens. I am not sure, but that is what I want to check.

jez commented 3 years ago

I believe this also implements the request in #993

martskins commented 3 years ago

Note that you can just use b:LanguageClient_isServerRunning instead of setting that g:it_crashed variable. It won't distinguish between the server having crashed and just not running, but maybe that's enough for you.

And yeah, as you said, the event is emitted when the client starts (it's also LanguageClientStarted, not LanguageServerStarted). If you want to detected when the server has started you can check the variable I mentioned above, that will let you verify if the server is running for that particular filetype too instead of globally.

Good catch on that other issue!

jez commented 3 years ago

Note that you can just use b:LanguageClient_isServerRunning

I am pretty sure you cannot currently do this. Maybe I'm reading the code wrong, but VIM_IS_SERVER_RUNNING is only ever set to zero in two places:

it doesn't get set to zero when server spawns correctly at first and crashes later.

jez commented 3 years ago

only ever set to zero in two places

oh never mind i see that it's now set in a third by this PR. let me try that out locally and see if it's enough

jez commented 3 years ago

restart

This is great, thanks!

martskins commented 3 years ago

@jez Spotted an issue with this, not critical though. When you manually want to stop the server by calling LanguageClientStop the client will automatically try to restart the server because it thinks the server crashed. Fished this in #1120. I tried it and it all seemed to be working normally now, but if you want to give it a try considering that you have airline thingy going on as well, that'd be great 😁