Skip to content

Conversation

@martskins
Copy link
Collaborator

@martskins martskins commented Oct 23, 2020

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.

@martskins martskins force-pushed the restart-on-crash branch 2 times, most recently from 17cdba6 to 86b298e Compare October 23, 2020 20:36
@jez
Copy link
Contributor

jez commented Oct 24, 2020

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
Copy link
Contributor

jez commented Oct 24, 2020

I believe this also implements the request in #993

@martskins
Copy link
Collaborator Author

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
Copy link
Contributor

jez commented Oct 24, 2020

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:

  • when you switch to a new buffer that doesn't have an active language server started yet
  • when lsp_types::notification::Exit::METHOD is called

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

@jez
Copy link
Contributor

jez commented Oct 24, 2020

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
Copy link
Contributor

jez commented Oct 24, 2020

restart

This is great, thanks!

@martskins
Copy link
Collaborator Author

@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 😁

martskins added a commit to martskins/LanguageClient-neovim that referenced this pull request Dec 6, 2020
martskins added a commit to martskins/LanguageClient-neovim that referenced this pull request Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants