fix(lsp): Ignore ServerCancelled error when pulling lsp diagnostics #30999
Conversation
9a426aa to
19b8289
Compare
|
I don't think this is the right fix. The LSP says that if the server sends this response we should look at the cancellation data and possibly retrigger the request: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnosticServerCancellationData. Kind of surprised that this isn't behind a client capability but this PR would be against what the protocol says we should do. But even if we ignore the retriggering for now, I don't think we should check the code in the request handler but instead handle it here where we ignore neovim/runtime/lua/vim/lsp/rpc.lua Lines 471 to 490 in b4599ac |
runtime/lua/vim/lsp/diagnostic.lua
Outdated
| function M.on_diagnostic(_, result, ctx, config) | ||
| function M.on_diagnostic(error, result, ctx, config) | ||
| -- Ignore ServerCancelled error | ||
| if error ~= nil and error.code == -32802 then |
There was a problem hiding this comment.
Instead of hardcoding the code here you should be adding it to protocol's error codes:
neovim/runtime/lua/vim/lsp/protocol.lua
Line 163 in b4599ac
|
I went with handling the error in the request handler, because server-side cancellation is only defined for pull diagnostics (and semantic token requests, but that's behind a capability). The (It's not behind a client capability, because the cancellation was introduced in the same version as pull diagnostics, which means there should be no clients that implement pull diagnostics but not server-side cancellation) |
19b8289 to
65a24a3
Compare
65a24a3 to
1de1ced
Compare
| if error ~= nil and error.code == protocol.ErrorCodes.ServerCancelled then | ||
| if error.data == nil or error.data.retriggerRequest == true then | ||
| local client = vim.lsp.get_client_by_id(ctx.client_id) | ||
| if client ~= nil then |
There was a problem hiding this comment.
Maybe an assertion that client isn’t nil is more appropriate?
There was a problem hiding this comment.
Is it possible that between the first request being sent and the server cancellation being received that the client has exited? If so, silently failing when client is nil seems like the better approach I think.
|
Hey @Jesse-Bakker, it seems like a lot of people are still struggling with #30985. Are you having issues with this PR? Let us know and we can help :) |
| if error.data == nil or error.data.retriggerRequest == true then | ||
| local client = vim.lsp.get_client_by_id(ctx.client_id) | ||
| if client ~= nil then | ||
| client.request(ctx.method, ctx.params) |
There was a problem hiding this comment.
Not 100% sure but I think this should be:
| client.request(ctx.method, ctx.params) | |
| client:request(ctx.method, ctx.params) |
Extra surprising since there is a |
|
Will continue this in #31345. Thank you @Jesse-Bakker for your contribution! |
Yeah this feels like a gap in the protocol. |
Fix #30985