Skip to content
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

Open
datanoise opened this issue Jan 7, 2025 · 33 comments · Fixed by #3024 · May be fixed by #3026 or #3021
Open

Version 0.23 breaks NeoVim integration #3019

datanoise opened this issue Jan 7, 2025 · 33 comments · Fixed by #3024 · May be fixed by #3026 or #3021
Labels
bug Something isn't working help-wanted Extra attention is needed non-vscode

Comments

@datanoise
Copy link

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 anendto close theclassstatement", 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.

@datanoise datanoise added bug Something isn't working help-wanted Extra attention is needed non-vscode labels Jan 7, 2025
@vinistock
Copy link
Member

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.

@datanoise
Copy link
Author

Preceding this log entry there was:

[ERROR][2025-01-07 12:37:03] .../vim/lsp/rpc.lua:484 "No callback found for server response id 16"

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.

@nerdrew
Copy link

nerdrew commented Jan 7, 2025

I'm getting the same errors, but it is for lots of different codes.

.../vim/lsp/rpc.lua:515	"No callback found for server response id 67"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 67,
.../vim/lsp/rpc.lua:515	"No callback found for server response id 70"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 70,
.../vim/lsp/rpc.lua:515	"No callback found for server response id 73"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 73,
LSP logging initiated
.../vim/lsp/rpc.lua:515	"No callback found for server response id 32"
.../vim/lsp/rpc.lua:515	"No callback found for server response id 34"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 32,
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 34,
.../vim/lsp/rpc.lua:515	"No callback found for server response id 39"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 39,
LSP logging initiated
.../vim/lsp/rpc.lua:515	"No callback found for server response id 16"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 16,
.../vim/lsp/rpc.lua:515	"No callback found for server response id 42"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 42,
.../vim/lsp/rpc.lua:515	"No callback found for server response id 44"
...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 44,

This looks similar to this which points to a bug in another LSP server here...

@vinistock
Copy link
Member

Thank you for sharing that issue, it's very helpful. Could either of you try something?

If you edit the LSP code in place (BUNDLE_GEMFILE=.ruby-lsp/Gemfile bundle open) and remove this line, does the issue go away?

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.

@datanoise
Copy link
Author

No change for me. the same errors are being reported.

@datanoise
Copy link
Author

But commenting out the prior send_message seems to fix it for me.

@adam12
Copy link
Contributor

adam12 commented Jan 7, 2025

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.

@vinistock
Copy link
Member

But commenting out the prior send_message seems to fix it for me.

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.

@datanoise
Copy link
Author

I am testing it with this send_message removed and I don't see any issues so far. I am not familiar with NeoVim Lsp implementation, but it looks like it doesn't expect any response to canceled requests. It probably removes any references to them and when the server responds to them, it just blows up with the NO_RESULT_CALLBACK_FOUND error into the user face.

@vinistock vinistock linked a pull request Jan 7, 2025 that will close this issue
@vinistock
Copy link
Member

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.

@SamSaffron
Copy link

just tried it and no luck, I confirm that commenting out the sendmessage above is resolving it though, even on the branch

@vinistock
Copy link
Member

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 RequestCancelled for requests that were cancelled by the client, but the spec has no mention about what to do if the server receives the same request ID as a retry.

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.

vinistock added a commit that referenced this issue Jan 8, 2025
### 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 vinistock reopened this Jan 8, 2025
@natematykiewicz
Copy link
Contributor

natematykiewicz commented Jan 8, 2025

@vinistock I'm running 0.23.2 and PR #3024 didn't seem to resolve the issue:

[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 4"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 7"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 9"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 11"
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 4,    jsonrpc = "2.0",    result = {      data = { 0, 6, 29, 2, 1, 2, 4, 10, 13, 0 },      resultId = "1"    }  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 7,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 9,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 11,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 13"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 15"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 19"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 20"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 21"
[ERROR][2025-01-08 14:39:04] .../vim/lsp/rpc.lua:515	"No callback found for server response id 22"
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 13,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 15,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 19,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 20,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 21,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:04] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 22,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:10] .../vim/lsp/rpc.lua:515	"No callback found for server response id 274"
[ERROR][2025-01-08 14:39:10] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 274,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:11] .../vim/lsp/rpc.lua:515	"No callback found for server response id 283"
[ERROR][2025-01-08 14:39:11] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 283,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:19] .../vim/lsp/rpc.lua:515	"No callback found for server response id 350"
[ERROR][2025-01-08 14:39:19] .../vim/lsp/rpc.lua:515	"No callback found for server response id 351"
[ERROR][2025-01-08 14:39:19] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 350,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:19] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 351,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 14:39:33] .../vim/lsp/rpc.lua:515	"No callback found for server response id 485"
[ERROR][2025-01-08 14:39:33] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 485,    jsonrpc = "2.0",    result = {}  }}

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

@danieljimenez-mycase
Copy link

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.

@natematykiewicz
Copy link
Contributor

I was seeing the same. It keeps overriding my .ruby-lsp/Gemfile changes

@vinistock
Copy link
Member

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.

@natematykiewicz
Copy link
Contributor

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.

@natematykiewicz
Copy link
Contributor

I'm about to overwrite vendor/bundle/ruby/3.2.0/gems/ruby-lsp-0.23.2/ files. Hopefully that works.

@natematykiewicz
Copy link
Contributor

natematykiewicz commented Jan 8, 2025

Nope, it doesn't work. I get errors still:

[ERROR][2025-01-08 15:14:48] .../vim/lsp/rpc.lua:515	"No callback found for server response id 22"
[ERROR][2025-01-08 15:14:48] .../vim/lsp/rpc.lua:515	"No callback found for server response id 25"
[ERROR][2025-01-08 15:14:48] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 22,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 15:14:48] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 25,    jsonrpc = "2.0",    result = {}  }}
[ERROR][2025-01-08 15:14:50] .../vim/lsp/rpc.lua:515	"No callback found for server response id 32"
[ERROR][2025-01-08 15:14:50] ...m/lsp/client.lua:974	"LSP[ruby_lsp]"	"on_error"	{  code = "NO_RESULT_CALLBACK_FOUND",  err = {    id = 32,    jsonrpc = "2.0",    result = { {        kind = 3,        range = {          ["end"] = {            character = 14,            line = 2          },          start = {            character = 7,            line = 2          }        }      }, {        kind = 2,        range = {          ["end"] = {            character = 14,            line = 2          },          start = {            character = 7,            line = 2          }        }      } }  }}

And semantic highlighting is really wonky. It almost looks like it's off by a line. For example, sig is usually yellow, but now att right under is yellow. Similarly with rsi in "parsing" -- that seems to be yellow because of the let above.

Image

@adam12
Copy link
Contributor

adam12 commented Jan 8, 2025

Nope, it doesn't work

Confirmed. I see the same.

@adam12
Copy link
Contributor

adam12 commented Jan 8, 2025

If you're trying to debug ruby-lsp, I find it helpful to add it to your application Gemfile. The ruby-lsp should then honour that version and not clobber it.

Paired with path: option can be helpful to debug local copies or bisect (tho bisecting has it's own issues).

@natematykiewicz
Copy link
Contributor

I should also say, I did puts "here" inside that initialize function, and saw it in my LSP logs. (Then restarted Neovim without that, to make sure that non-LSP puts didn't break something.) So, I'm confident that I did run those changes.

@vinistock
Copy link
Member

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.

@natematykiewicz
Copy link
Contributor

natematykiewicz commented Jan 8, 2025

The semantic highlighting was broken by the #3026 changes. It looks fine running 0.23.2 code.

Image

I suspect the until LINE_BREAK == @source[@pos] is leaving it +1'ed once too many
https://github.com/Shopify/ruby-lsp/pull/3026/files

@gaffneyc
Copy link

gaffneyc commented Jan 8, 2025

Remember to make sure ruby-lsp quits before testing. I've had it stay running after closing vim on occasion.

@natematykiewicz
Copy link
Contributor

Oh wait:

          @pos += 1 until LINE_BREAK == @source[@pos]
          @pos += 1

I think it's the bonus @pos += 1 that's the problem

@vinistock
Copy link
Member

vinistock commented Jan 8, 2025

That was already there before #2938. When we find a line break, the loop ends before incrementing @pos, that's why there's an extra one. It accounts for the \n.

@natematykiewicz
Copy link
Contributor

Let's ignore the semantic token thing. I think I had accidentally deleted one too many @pos += 1 when I was making my edits. I can't reproduce that anymore.

@vinistock
Copy link
Member

Okay, let's test one more hypothesis: part of #2939 was fixing a mistake in the cancellation implementation. We were pushing $/cancelRequest notifications to our task queue, which meant that we would only see cancellations after the requests were already processed (defeating the whole point of cancelling it).

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?

@natematykiewicz
Copy link
Contributor

Yup! Reverting to 0.23.2 and removing this fixes it:

,
"$/cancelRequest"

@natematykiewicz
Copy link
Contributor

"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.

@dzirtusss
Copy link

@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 main@12345 will also work, but I haven't tested it with commit. If it will work, it may be possible to revert to older commits or freeze version.

However, latest master seems not crashing atm after revert, thnks @vinistock .

@dbaeumer
Copy link

dbaeumer commented Jan 9, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help-wanted Extra attention is needed non-vscode
Projects
None yet
10 participants