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

Missing work done progress token from client init #27938

Closed
vilelamarcospaulo opened this issue Mar 19, 2024 · 4 comments · Fixed by #28182
Closed

Missing work done progress token from client init #27938

vilelamarcospaulo opened this issue Mar 19, 2024 · 4 comments · Fixed by #28182
Labels
enhancement feature request lsp
Milestone

Comments

@vilelamarcospaulo
Copy link

Problem

Right now, clojure-lsp relies on the microsoft lsp specification that delegates the Work Done progress token creation to the sender.

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#initiatingWorkDoneProgress

Work Done progress can be initiated in two different ways:

  1. by the sender of a request (mostly clients) using the predefined workDoneToken property in the requests parameter literal. The document will refer to this kind of progress as client initiated progress.

The problem is their implementation ignores the $/progress publication when this is not present, this way none lsp-status tool works correctly to give to user feedback, what is bad since we don't really know when the processing is finished and can take a while.
Others LSP (luals, golsp, etc...) just sets a random value (string | int) when it's not present and everything is working.

But in this case (maybe other LSP also relies in this definitions) it's just is ignored.

To resolve this, in my local neovim build updated the function Client:initialize() implementation to also sent the property workDoneToken = "foobar" inside the initialize_params. Since this is the same approach used by vscode and emacs.
After this change the $/progress starts to receive every progress update, thus lsp-status and fidget was working just fine.

Steps to reproduce using "nvim -u minimal_init.lua"

vim.lsp.set_log_level("debug")
vim.lsp.start_client({
  name = 'my-server-name',
  cmd = { 'clojure-lsp' },
  root_dir = vim.fn.getcwd()
})

Expected behavior

should contains the $/progress in the lsp debug logs

-- [DEBUG][2024-03-19 18:26:22] .../vim/lsp/rpc.lua:403	"rpc.receive"	{  jsonrpc = "2.0",  method = "$/progress",  params  = {    token = "foobar",    value = {      kind = "begin",      percentage = 0,      title = "clojure-lsp"    }  }}
-- [DEBUG][2024-03-19 18:26:22] .../vim/lsp/rpc.lua:403	"rpc.receive"	{  jsonrpc = "2.0",  method = "$/progress",  params = {    token = "foobar",    value = {      kind = "report",      message = "Finding kondo config",      percentage = 5    }  }}

Neovim version (nvim -v)

Nvim 0.9.5

Language server name/version

clojure-lsp/2024.02.01-11.01.59

Operating system/version

macos/14.2.1 (sonoma)

Log file

No response

@vilelamarcospaulo vilelamarcospaulo added bug issues reporting wrong behavior lsp labels Mar 19, 2024
@vilelamarcospaulo vilelamarcospaulo changed the title work done progress token Missing work done progress token from client init Mar 19, 2024
@justinmk justinmk added this to the backlog milestone Mar 20, 2024
@mfussenegger mfussenegger added enhancement feature request and removed bug issues reporting wrong behavior labels Mar 20, 2024
@rudiejd
Copy link
Contributor

rudiejd commented Mar 27, 2024

should this be a config parameter for each language server, or should Neovim always create the progress token itself? Are there downsides to creating / not creating the token as the client?

@vilelamarcospaulo
Copy link
Author

Hello @rudiejd
Looking at other implementations, there basic two scenarios for the language servers. Or they expect for the server to send this like the clojure-lsp case, or they ignore the one sent and create a new every time like goplp and lua_ls.

From what i saw, when the server creates it by itself this creates no effect on the expected behavior. But for the ones where this is expected this change will enables to fully work.

Since this is expected based on the specification for LSP in general, i don't believe it should be for each language server, but a implementation for the protocol itself. VSCode for example always send a generated UUID in the init call. Emacs in the other hand just send a fixed string "1".

Bottom line is, receive this parameter is already expected from the LSPs since it's part of the specification. Some servers will ignore it and others will use the received one. So theres no real downside in this.

@rudiejd
Copy link
Contributor

rudiejd commented Mar 28, 2024

Thank you for the detailed response! I noticed VSCode makes this configurable, so I chose to make it configurable just in case. I'm not sure if there are any benefits of sending a UUID, so I just sent "1" for now similar to emacs. Let me know if my branch works for you

@vilelamarcospaulo
Copy link
Author

@rudiejd Thanks for doing this implementation, after taking a fast look it looks pretty much ok to me. Right now i'm also working with a fixed string in my local build, and it's just fine, so i believe sending fixed "1" as emacs will be the best choice.

mfussenegger added a commit to mfussenegger/neovim that referenced this issue Apr 5, 2024
Problem:

Some servers don't report progress during initialize unless the client
sets the `workDoneToken`

See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#initiatingWorkDoneProgress

In particular:

> There is no specific client capability signaling whether a client will
> send a progress token per request. The reason for this is that this is
> in many clients not a static aspect and might even change for every
> request instance for the same request type. So the capability is signal
> on every request instance by the presence of a workDoneToken property.

And:

> Servers can also initiate progress reporting using the
> window/workDoneProgress/create request. This is useful if the server
> needs to report progress outside of a request (for example the server
> needs to re-index a database). The token can then be used to report
> progress using the same notifications used as for client initiated
> progress.

So far progress report functionality was relying entirely on the latter.

Solution:

Set a `workDoneToken`

Closes neovim#27938
mfussenegger added a commit that referenced this issue Apr 5, 2024
Problem:

Some servers don't report progress during initialize unless the client
sets the `workDoneToken`

See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#initiatingWorkDoneProgress

In particular:

> There is no specific client capability signaling whether a client will
> send a progress token per request. The reason for this is that this is
> in many clients not a static aspect and might even change for every
> request instance for the same request type. So the capability is signal
> on every request instance by the presence of a workDoneToken property.

And:

> Servers can also initiate progress reporting using the
> window/workDoneProgress/create request. This is useful if the server
> needs to report progress outside of a request (for example the server
> needs to re-index a database). The token can then be used to report
> progress using the same notifications used as for client initiated
> progress.

So far progress report functionality was relying entirely on the latter.

Solution:

Set a `workDoneToken`

Closes #27938
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement feature request lsp
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants