Server sends invalid request workspace/diagnostics/refresh

[jwortmann]

Using version 1.18.1 from the release assets (Windows).

1. This server makes requests with the method workspace/diagnostics/refresh. Such a request does not exist in the LSP specs.

   You probably have a typo and want workspace/diagnostic/refresh. See Diagnostics Refresh.

2. Besides that, it seems that the server never responds to the workspace/diagnostic client request. Here is an example for the params (WorkspaceDiagnosticParams) of the request:

   {
       "previousResultIds": [],
       "partialResultToken": "$ublime-partial-result-progress-19",
       "identifier": "luau"
   }

   The server uses the token to send a $/progress notification with the diagnostics for the files, which is pretty cool. Unfortunately, the diagnostics for the files are all empty - I assume it might be a placeholder for now (they also miss the mandatory version field). But then the server never actually responds to the request. To send a response is mandatory in JSON-RPC and the language server protocol. Presuming there were actual diagnostics in the partial results, I would expect the WorkspaceDiagnosticReport response with an empty array for the items (because the relevant diagnostics seemingly have already been sent via the partial results):

   {
       "items": []
   }

   Or alternatively, it should not use the partial result token, i.e. don't send the $/progress notification, and instead put the diagnostics only into the response.

---

By the way, and if you are wondering, I'm just using this server to test a client implementation for the pull diagnostics, because it's the only server I found yet, that announces support for workspace diagnostics 👍

[JohnnyMorganz]

Thanks for pointing out the typo! Surprised that has been missed for a while, I did notice it never did anything but never investigated further. (I would've assumed VSCode LSP client would warn on unknown request methods)

Wrt the missing version field, sounds like a serialisation problem of null vs undefined again, which is annoying. Will take a look.

We do not respond to the workspace diagnostics request on purpose, and always stream the results. I believe this is a case mentioned in the spec. Since we do not respond to the request, it should be long living and means we can continuously stream and update results for that single request.

Why we do this, is because once we respond to the request, the LSP client then initiates another workspace diagnostics request (atleast, the VSCode client did after a fixed time interval - I think 5 seconds?). Originally we did no streaming, but that meant VSCode continuously spammed us with workspace diagnostic requests, which can be expensive at times to compute. Now, we stream the results on changes only, where on the initial request we send all the current results as a $/progress, then any change in files we send an update afterwards for the affected files only. (We do actually respond to the request in the end, but typically with a ServerCancelled error and DiagnosticServerCancellationData to ask the client to end that request and trigger a fresh one).

Hope that info helps!

[JohnnyMorganz]

From the spec:

In contrast to the document diagnostic request the workspace request can be long running and is not bound to a specific workspace or document state. If the client supports streaming for the workspace diagnostic pull it is legal to provide a document diagnostic report multiple times for the same document URI. The last one reported will win over previous reports.

and

It is recommended for clients to implement partial result progress for the workspace pull to allow servers to keep the request open for a long time. If a server closes a workspace diagnostic pull request the client should re-trigger the request.

[jwortmann]

Oh, this is great info! Yes, I think you are right and leaving the request open to continuously stream updates is a valid use case. It seems I had overlooked this part in the specs, thanks for pointing that out.

My initial client implementation would cancel the request after some delay, when there are new changes in a file made by the user, and the server still hasn't responded, before making a new request. That strategy would probably not work well together with the server intention to continuously stream the updates. I probably have to rethink that implementation, so that it works both with servers which want to use the partial results for streaming, and also with servers which send the results just via the regular response. It seems that this part might be a bit tricky on the client side, but I'll give it a try and see how it works.

[JohnnyMorganz]

would cancel the request after some delay, when there are new changes in a file made by the user, and the server still hasn't responded, before making a new request

This sounds more like document diagnostics. My understanding is workspace diagnostics (from the client's perspective) is just a background task that runs periodically without specific file change events