executeCommandProvider fails with `command .. already exists` with multi-root servers

[Tehnix]

Context: vscode-hie-server

After upgrading vscode-languageclient from 3.5.0 to 4.1.3, we are having problems with multiple spawned language servers, which is needed for multi-root support. I want to note that this works in 3.5.0, and some breaking change was introduced (or errors were made non-silent) in 4.x.

The error comes from hie giving back its capabilities on initialization, where it registers executeCommandProviders. When a second server gets spawned, this command already exists, and VSCode then complains about this.

The Haskell language server sends this response,

{
  "result": {
    "capabilities": {
      "textDocumentSync": {
        "openClose": true,
        "change": 2,
        "willSave": false,
        "willSaveWaitUntil": false,
        "save": { "includeText": false }
      },
      "hoverProvider": true,
      "completionProvider": {
        "resolveProvider": true,
        "triggerCharacters": ["."]
      },
      "definitionProvider": true,
      "referencesProvider": true,
      "documentHighlightProvider": true,
      "documentSymbolProvider": true,
      "codeActionProvider": true,
      "documentFormattingProvider": true,
      "documentRangeFormattingProvider": true,
      "renameProvider": true,
      "executeCommandProvider": {
        "commands": ["applyrefact:applyOne", "hare:demote"]
      }
    }
  },
  "jsonrpc": "2.0",
  "id": 0
}

which causes an error to occur and the process to get shut down on the VSCode side of things,

> command 'applyrefact:applyOne' already exists
> [alanz.vscode-hie-server]command 'applyrefact:applyOne' already exists
> Error: command 'applyrefact:applyOne' already exists
	at e.registerCommand (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:594:33)
	at Object.registerCommand (/Applications/Visual Studio Code.app/Contents/Resources/app/out/vs/workbench/node/extensionHostProcess.js:656:797)
	at ExecuteCommandFeature.register (/Users/tehnix/GitHub/Clones/vscode-hie-server/node_modules/vscode-languageclient/lib/client.js:1433:53)
	at ExecuteCommandFeature.initialize (/Users/tehnix/GitHub/Clones/vscode-hie-server/node_modules/vscode-languageclient/lib/client.js:1423:14)
	at LanguageClient.initializeFeatures (/Users/tehnix/GitHub/Clones/vscode-hie-server/node_modules/vscode-languageclient/lib/client.js:2099:21)
	at TextDocumentFeature.initialize.connection.initialize.then (/Users/tehnix/GitHub/Clones/vscode-hie-server/node_modules/vscode-languageclient/lib/client.js:1819:18)
	at <anonymous>

Generally, I'd also like there to be a better story with editor commands and multiple servers. Currently we are doing something like this on activation,

  // Register editor commands for HIE, but only register the commands once.
  if (!hieCommandsRegistered) {
    registerHiePointCommand('hie.commands.genApplicative', 'hare:genapplicative', context);
    // ...
    hieCommandsRegistered = true;
  }

and then register the command like so,

    // Get the current file and workspace folder.
    const uri = editor.document.uri;
    const folder = workspace.getWorkspaceFolder(uri);
    // If there is a client registered for this workspace, use that client.
    if (folder !== undefined && clients.has(folder.uri.toString())) {
      const client = clients.get(folder.uri.toString());
      if (client !== undefined) {
        client.sendRequest('workspace/executeCommand', cmd).then(
          hints => {
            return true;
          },
          e => {
            console.error(e);
          }
        );
      }
    }

which looks up the client on activation.

[rcjsuen]

@Tehnix I think you didn't finish your sentence.

After upgrading to

[Tehnix]

@rcjsuen yeah sorry, did not know that Github comments on CMD + SHIFT + Enter ^_^ I've updated/finished the issue now.

[dbaeumer]

This is very likely caused by fixing bugs in the library and now correctly surfacing this. Registering the same command twice and make it magically work in a multi server setup never worked.

The underlying problem is that all servers register the commands using the same id. In general spawning n servers is nothing the LSP can magically handle in terms of de-duping the commands or other global things. So all I can recommand here is to create different command ids per launched server and making it clear which server is used when executing the command. If the commands need to be dispatch to all servers then you need to register one command in the extension and dispatch the command yourself to all servers using the provided sendRequest API on the LanguageClient

@Tehnix I would like to close the issue since IMO there is nothing the LSP can do to make this magically work.

[Tehnix]

How do you create different command ids though? Is this something that has to be done in the server, or can it be done in the client? I have been digging around for hooks to anything with executeCommandProvider in the client, but haven't found anything so far.

By ID are you referring to the first argument of new LanguageClient(langName, langName, serverOptions, clientOptions)? Because that is already unique when launching the server (is adds the workspace folder that it was launched in).

If you mean the server needs to be aware that it has multiple instances, then I don't think that's a realistic solution to this problem.

EDIT: My main point is, executeCommandProvider is entirely from the LSP server, and we do nothing in the client side to handle that currently.

[dbaeumer]

The solution you need to pick depends on how the command should work. In our example the hare:demote command. If the user executes this command from the command palette to which server should the command be send. Or are these all commands bound to code actions?

[Tehnix]

The hare:demote one I'm not sure, but the applyrefact:applyOne should be bound to code actions. It basically applies the refactor with the little light bulb icon.

[dbaeumer]

OK. then for applyrefact:applyOne you could then simply generate a UUID and use it. This makes it unique inside the client and you are able to bind n servers.

[Tehnix]

Hmm, I'm not exactly sure I follow where this is all supposed to be going on :/

Would this be in the server or in the client? Like, the UUID is generated in the executeCommandProvider returned by the server?

[Tehnix]

For now, we've reverted to version 4.0.1, where the LSP will not be stopped because of this, until we find a workaround.

[dbaeumer]

You should use uuids for the commands you register if the server is started more than once. Something like

let uuid = generateUUID();

....

      "executeCommandProvider": {
        "commands": [/*applyrefact:applyOne*/ uuid, "hare:demote"]
      }

[dbaeumer]

Let me know if this helps.