Inlay hints callback corruption?

[lithammer]

I've noticed a weird behaviour with the gopls language server. I can consistently reproduce it on bigger projects, where I assume the textDocument/inlayHint request takes a long time.

The first symptom was that the workspace never finished loading so no LSP functionality worked. But I was receiving this error:

Error: request textDocument/inlayHint failed (no result)

It seemed weird that the result would be empty, but no error from the server so I extended the error message like so:

-util.ErrMsg($'request {method} failed (no result)')
+util.ErrMsg($'request {method} failed (no result): {reply->string()}')

And it revealed something curious. This is the error I'm getting:

Error: request textDocument/inlayHint failed (no result): 
{'method': 'workspace/configuration', 'jsonrpc': '2.0', 'id': 2, 'params': {'items': [{'section': 'gopls'}]}}

So I'm receiving a workspace/configuration response in the textDocument/inlayHint callback??

---

var servers = [
  {
    name: 'gopls',
    path: 'gopls',
    args: ['serve'],
    filetype: ['go', 'gomod', 'gowork', 'gotmpl', 'gohtmltmpl', 'gotexttmpl'],
    syncInit: true,
    debug: true,
    traceLevel: 'verbose',
    rootSearch: ['go.work', 'go.mod'],
    workspaceConfig: {
      gopls: {
        analyses: {
          nilness: true,
          unusedparams: true,
          unusedwrite: true,
          useany: true,
        },
        hoverKind: 'FullDocumentation',
        linksInHover: false,
        gofumpt: true,
        completeUnimported: true,
        # semanticTokens: false,
        staticcheck: true,
        usePlaceholders: true,
        completionDocumentation: true,
        codelenses: {
          generate: true,
          test: true,
          run_vulncheck_exp: true,
        },
        hints: {
          assignVariableTypes: false,
          compositeLiteralFields: true,
          compositeLiteralTypes: false,
          constantValues: true,
          functionTypeParameters: true,
          parameterNames: true,
          rangeVariableTypes: true,
        },
      }
    }
  }
]

var opts = {
  autoComplete: true,
  autoHighlight: true,
  highlightDiagInline: true,
  showDiagOnStatusLine: true,
  showInlayHints: true,
  snippetSupport: true,
  vsnipSupport: true,
}

g:LspAddServer(servers)
g:LspOptionsSet(opts)

[Shane-XB-Qian]

how big what you mean? i tried 170M size prj at my laptop, occasionally looks i can repro this, and not sure your test on reply msg, but i felt it was gopls was not "really ready" for inlay-hints. // seems not exactly or not sure same case like rust case, but try https://github.com/yegappan/lsp/pull/357 maybe helpful.

[lithammer]

how big what you mean? i tried 170M size prj at my laptop, occasionally looks i can repro this,

Well I can fairly consistently reproduce it on this repo: https://github.com/googleapis/google-cloud-go

and not sure your test on reply msg, but i felt it was gopls was not "really ready" for inlay-hints. // seems not exactly or not sure same case like rust case, but try #357 maybe helpful.

Yeah kind of seems like we're sending textDocument/inlayHint too early 🤔 That fix doesn't seem to help, unfortunately.

[Shane-XB-Qian]

// seems not exactly or not sure same case like rust case, but try #357 maybe helpful.

Yeah kind of seems like we're sending textDocument/inlayHint too early 🤔 That fix doesn't seem to help, unfortunately.

i thought it helped, but you faced a bit early stage of that, and gopls seems not ready for that (inlay hint request) yet. i opened PR #396 perhaps help your case (or a refine anyway)

[lithammer]

396 doesn't really fix the problem though. I had to bump the timer to something like 15000 to get it to work.

I do wonder if this isn't a bug in Vim itself though. Specifically in ch_sendexpr() when you give it a callback. I don't really know how the LSP channel stuff works in Vim. But it seems like it's expecting a regular request → response cycle. But this doesn't work since a request can be initiated from both the client and the server at any time.

From my testing, gopls sends something like 3 workspace/configuration requests during startup. What ends up happening is something like this:

[1] Client -> Server: initialize
[1] Client <- Server: InitializeResult
[2] Client -> Server: initialized
[3] Client <- Server: workspace/configuration
[3] Client -> Server: ConfigurationItem[]
[4] Client -> Server: textDocument/inlayHint
[5] Client <- Server: workspace/configuration
[4] Client <- Server: InlayHint[] (this never happens)

Now the client is confused because it thinks [5] is a response to [4], but it's actually a server initiated request. Now the (gopls) server just sits there and waits for a response to its workspace/configuration request and refuses to handle any other requests.

I verified that because if I manually respond to the pending workspace/configuration request it starts living again (I hacked the callback function).

So just delaying the textDocument/inlayHint request only makes this problem less likely to happen since server requests are quite rare after the initial startup sequence.

[yegappan]

The Vim LSP channel code matches the response from the LSP server with an outstanding request by comparing the ID:

https://github.com/vim/vim/blob/master/src/channel.c#L3052

In the above example, it should match the ID in the message (4) with the outstanding request message.

If the IDs in both the request messages sent by the server and sent by the client match, then it will lead to a problem. This can be fixed by changing the may_invoke_callback() function to check whether the received message is a request or a response message. If the "method" field is present, then it is a request message. Otherwise it is a response message.

[yegappan]

Can you try the following patch to Vim?

diff --git a/src/channel.c b/src/channel.c
index cdb956e7c..115c26b61 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -2927,9 +2927,13 @@ may_invoke_callback(channel_T *channel, ch_part_T part)
            seq_nr = 0;
            if (d != NULL)
            {
-               di = dict_find(d, (char_u *)"id", -1);
-               if (di != NULL && di->di_tv.v_type == VAR_NUMBER)
-                   seq_nr = di->di_tv.vval.v_number;
+               // ignore "id" in the LSP request messages.
+               if (!dict_has_key(d, "method"))
+               {
+                   di = dict_find(d, (char_u *)"id", -1);
+                   if (di != NULL && di->di_tv.v_type == VAR_NUMBER)
+                       seq_nr = di->di_tv.vval.v_number;
+               }
            }

            argv[1] = *listtv;

[Shane-XB-Qian]

i verified, not sure outage, but seems it helped.

[lithammer]

I also verified that the above Vim patch seems to solve the problem 👍

[lithammer]

Closing this since it's an upstream issue and there are fixes on the way. Thanks!

• https://github.com/vim/vim/pull/13133
• https://github.com/vim/vim/pull/13141
• https://github.com/vim/vim/pull/13158

[yegappan]

Can you try the latest Vim (9.0.1924 or after) and see whether this issue with gopls is fixed?

[lithammer]

Unfortunately, this still happens on 9.0.1927:

:version
VIM - Vi IMproved 9.0 (2022 Jun 28, compiled Sep 21 2023 14:57:42)
Included patches: 1-1927

Error: request textDocument/inlayHint failed (no result):
{'method': 'workspace/configuration', 'jsonrpc': '2.0', 'id': 2, 'params': {'items': [{'section': 'gopls'}]}}

[yegappan]

I am not able to reproduce this problem using a test. Can you try the following patch on top of 9.0.1927?

diff --git a/src/channel.c b/src/channel.c
index 1de376884..38f610a9f 100644
--- a/src/channel.c
+++ b/src/channel.c
@@ -3052,13 +3052,27 @@ may_invoke_callback(channel_T *channel, ch_part_T part)
     {
        // JSON or JS or LSP mode: invoke the one-time callback with the
        // matching nr
-       for (cbitem = cbhead->cq_next; cbitem != NULL; cbitem = cbitem->cq_next)
-           if (cbitem->cq_seq_nr == seq_nr)
+       int lsp_req_msg = FALSE;
+
+       // Don't use a LSP server request message with the same sequence number
+       // as the client request message as the response message.
+       if (ch_mode == CH_MODE_LSP && argv[1].v_type == VAR_DICT
+               && dict_has_key(argv[1].vval.v_dict, "method"))
+           lsp_req_msg = TRUE;
+
+       if (!lsp_req_msg)
+       {
+           for (cbitem = cbhead->cq_next; cbitem != NULL;
+                   cbitem = cbitem->cq_next)
            {
-               invoke_one_time_callback(channel, cbhead, cbitem, argv);
-               called_otc = TRUE;
-               break;
+               if (cbitem->cq_seq_nr == seq_nr)
+               {
+                   invoke_one_time_callback(channel, cbhead, cbitem, argv);
+                   called_otc = TRUE;
+                   break;
+               }
            }
+       }
     }

     if (seq_nr > 0 && (ch_mode != CH_MODE_LSP || called_otc))

[yegappan]

I am not able to reproduce this problem using a test.

I am able to reproduce the problem using a test now. Basically the problem can happen when using either a channel callback function or when using a one-time callback function. The changes in 9.0.1927 fixed the problem when using the channel callback function. The above patch fixes the problem when using the one-time callback function.

Once you confirm that this patch fixes the problem in your tests, I will open a PR upstream.

[lithammer]

Once you confirm that this patch fixes the problem in your tests, I will open a PR upstream.

I played around with the patch and bit and so far I haven't been able to reproduce the bug. So it seems like that fixes the problem 👍 Nice!