autozimu / LanguageClient-neovim

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

Check whether language server has crashed #1110

Closed jez closed 4 years ago

jez commented 4 years ago

Is your feature request related to a problem? Please describe.

Sometimes language servers crash. This means that LSP features stop working until the user intervenes and fixes the reason why the language server crashed.

I'd like to be notified when the server dies.

Describe the solution you'd like

I'm fine with any of these:

Again, there's nothing I can do to check this now, so any way to check whether the language server process crashed would be great.

martskins commented 4 years ago

This looks related to #320 and #774. Just leaving that here so we remember to close those if this eventually gets fixed.

This is a little tricky to implement if the server quits unexpectedly (without notifying), as the bit of code that handles that error is run in a different thread than the thread that we use to communicate with vim. I'm not saying it can't be done though, but it's no trivial. Also, detecting the error might also be tricky, I would expect the error to be std::io::Error with kind std::io::ErrorKind::UnexpectedEof, but I'm not sure if you can get that error from other situations which wouldn't require a restart.

I'll give it a try and see if I can get something working for this, but if anyone else wants to give it a go please do 😄

martskins commented 4 years ago

I'm thinking of implementing this as an option to automatically restart instead. I feel like most users would want to handle a server crash by restarting the server anyways, so maybe it's just better to provide a configuration option to enable automatically restarting. Doing it our side would also mean that we could include some exponential backoff logic to it so as not to constantly restart the server and take that logic away from the user side of things.

What do you think?

jez commented 4 years ago

I'm thinking of implementing this as an option to automatically restart instead

The primary case why I care about this is because new users misconfigure Sorbet.

You can retry however many times you want, backoff or not. That's not going to fix the problem.

I would very much like to show a message to the user what the problem was, but failing that, i would be fine with a boolean true/false so that I can write code that suggests they look in the contents of a specific log file.

jez commented 4 years ago

Automatic restarts would be another nice to have. Sorbet does crash from time to time (unhandled exception or whatever) and it would be nice to restart the server just in case it was a spurious bug.

So I think the two features are orthogonal, but specifically in this issue I am asking for a way to discover whether it crashed.

martskins commented 4 years ago

Out of curiosity, what would be your use case for that, if not restarting.

EDIT: Never mind, didn't see your previous reply 😄

jez commented 4 years ago

etc.

martskins commented 4 years ago

@jez See #1113

Note that I've added the event as we talked, but you can also detect whether the server is running or not by calling get(b:, 'LanguageClient_isServerRunning'). That is a buffer scoped variable, so you'll get the proper value for your language server there if you have a ruby file open when running it.

If you could give this a try it would be great. I'll wait a few days to see if @autozimu has something to say about that PR and if not, and everything works fine for you (and me), I'll merge.

martskins commented 4 years ago

Closed via #1113