-
Notifications
You must be signed in to change notification settings - Fork 172
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
Version 0.23 breaks NeoVim integration #3019
Comments
Thanks for the report. Is there any other output? Looking at the snippet you shared, it looks like there's part of an error and part of a syntax error diagnostic. We didn't change anything related to syntax error diagnostics and that has been implemented for a long time, so I assume there's some other error happening there. |
Preceding this log entry there was:
Not sure if it helps. All I know is that reverting back to prior version fixes the issue. No configuration changes in my NeoVim config. |
I'm getting the same errors, but it is for lots of different codes.
This looks similar to this which points to a bug in another LSP server here... |
Thank you for sharing that issue, it's very helpful. Could either of you try something? If you edit the LSP code in place ( Reading through those threads, what I understood is that a request cancelled by the client successfully has to continue returning the special cancelled error code even if the client decides to retry the cancelled request, but I want to be sure. |
No change for me. the same errors are being reported. |
But commenting out the prior |
Assuming Neovim 0.10.3? There were a few LSP changes made.
I noticed the same errors yesterday but only once, and didn't have time to look. |
That might cause things to hang, because then the server is receiving a request, but not responding back with anything. I'm a bit confused with what is expected here. The spec says that the server has to return something, even for cancelled requests - and it mentions that the server should return the request cancelled error code. The spec does not mention anything about cancelled requests being retried or what to do then. Currently, if we receive the same request a second time after being cancelled, we "forget" that it was cancelled and actually go through with computing it, returning a response. I expected that not deleting it from the list of cancelled requests would fix it, because then instead of returning a response, we would simply continue returning the special error code. |
I am testing it with this |
If someone could give the changes in #3021 a try, that would be really helpful. I tried to dig into what Rust Analyzer does and my understanding is that cancelled requests are simply ignored. |
just tried it and no luck, I confirm that commenting out the sendmessage above is resolving it though, even on the branch |
Okay, I created #3024 to revert the behaviour back to what it used to be (which didn't cause problems for NeoVim) until we figure out what's going on. @dbaeumer can I ask you for some guidance? My understanding is that the server should return Is retrying cancelled requests expected? And what should servers respond with? The error response in the first request and then ignore the rest? I'm happy to put up a PR adding more detail to the cancellation support text in the spec once we confirm what's the expected behaviour. |
### Motivation This is a temporary revert to fix #3019 while we understand the correct response flow for retried cancelled requests. This PR reverts the behaviour introduced in #2939, which didn't cause problems for NeoVim. ### Implementation I didn't perform a full revert of every single change, since we do want to ensure that we're following the spec. I just stopped returning the error and went back to returning a `nil` response until we figure out what's going on. ### Automated Tests Changed the test so that it asserts the current behaviour. We'll switch it back later.
@vinistock I'm running 0.23.2 and PR #3024 didn't seem to resolve the issue:
I get all sorts of LSP errors while typing. I'll gladly help debug this, I'm just not sure where to start. I'm running Neovim 0.10.1, btw |
Same. Is there a way to revert to a lower version? Was not able to find anything in the docs. I uninstall the gem and install a lower version but ruby-lsp auto updates when it runs. |
I was seeing the same. It keeps overriding my |
Can someone please test #3026? It reverts the only other PR in v0.23 that started returning error responses. If that fixes the issue, I can cut a release with it. |
I would, but I'm actually not sure how. Every time I try to install a different version of ruby-lsp, booting ruby-lsp reverts my Gemfile changes and puts it back to the latest release. |
I'm about to overwrite |
Confirmed. I see the same. |
If you're trying to debug ruby-lsp, I find it helpful to add it to your application Paired with |
I should also say, I did |
I don't know how to do it in NeoVim, but it would be really helpful to turn on server tracing and see the flow of a few requests. I'm not sure what other change from v0.23 could be causing this, I was suspecting the request cancellation or the location not found errors. We also didn't change anything related to semantic highlighting in this release, so not sure how that could be happening. The fact that everything works normally in VS Code doesn't help either. |
The semantic highlighting was broken by the #3026 changes. It looks fine running 0.23.2 code. I suspect the |
Remember to make sure ruby-lsp quits before testing. I've had it stay running after closing vim on occasion. |
Oh wait: @pos += 1 until LINE_BREAK == @source[@pos]
@pos += 1 I think it's the bonus |
That was already there before #2938. When we find a line break, the loop ends before incrementing |
Let's ignore the semantic token thing. I think I had accidentally deleted one too many |
Okay, let's test one more hypothesis: part of #2939 was fixing a mistake in the cancellation implementation. We were pushing In that PR, we started handling cancellations in the main thread without pushing them to the queue, so that if the request got cancelled we could stop it from running. If you re-introduce the incorrect behaviour, by removing this line and forcing the cancellation notifications to be pushed to the queue again, does the issue go away? |
Yup! Reverting to 0.23.2 and removing this fixes it:
|
"fixes it" (in air quotes) I'm not sure what the ramifications of removing that are. I just know that I'm not getting a ton of errors as I type anymore. |
@natematykiewicz you could do: lspconfig.ruby_lsp.setup({ capabilities = capabilities, cmd = { 'ruby-lsp', '--branch', 'main' } }) then it automatically picks main branch version, not the latest gem version. I guess However, latest master seems not crashing atm after revert, thnks @vinistock . |
@vinistock have not read the whole thread, so I hope my answer helps: for the VS Code LSP client libs I never retry a canceled request in the lib itself. I let VS Code itself decide when the best time is to do the retry. In this case VS Code will issue a new request which in the library will result in an LSP request with a DIFFERENT request id. Reusing request ids is something I don't do since it IMO only causes issues with cleaning something up in an async environment. I am open to add something to the spec to say that if a client retries a request it should send the same request with a new ID. |
Description
While typing Neovim sends incomplete code to ruby-lsp which results in errors like this:
[ERROR][2025-01-07 12:37:03] ...m/lsp/client.lua:1041 "LSP[ruby_lsp]" "on_error" { code = "NO_RESULT_CALLBACK_FOUND", err = { id = 16, jsonrpc = "2.0", result = { items = { { message = "unexpected end-of-input, assuming it is closing the parent top level context", range = { ["end"] = { character = 0, line = 160 }, start = { character = 3, line = 159 } }, severity = 1, source = "Prism" }, { message = "expected an
endto close the
classstatement", range = { ["end"] = { character = 0, line = 160 }, start = { character = 0, line = 160 } }, severity = 1, source = "Prism" }, { message = "mismatched indentations at 'end' with 'if' at 71", range = { ["end"] = { character = 5, line = 80 }, start = { character = 2, line = 80 } }, severity = 2, source = "Prism" }, { message = "mismatched indentations at 'end' with 'def' at 70", range = { ["end"] = { character = 3, line = 159 }, start = { character = 0, line = 159 } }, severity = 2, source = "Prism" } }, kind = "full" } } }
It is impossible to type now, because it seems these errors pop up on each key stroke. Reverting to version
0.22.1
fixes the issue.The text was updated successfully, but these errors were encountered: