search

microsoft / vscode-languageserver-node

Language server protocol implementation for VSCode. This allows implementing language services in JS/TS running on node.js
MIT License
1.42k stars 318 forks source link

client: option to suppress language client start error message popup #1011

Open hyangah opened 2 years ago

hyangah commented 2 years ago

Recently we upgraded our language client to support LSP 3.17, and noticed certain error conditions in language client start up result in multiple popups even though we set RevealOutputChannelOn.Never.

Screen Shot 2022-06-23 at 7 41 24 PM

My guess is this is a change from https://github.com/microsoft/vscode-languageserver-node/issues/824

Our extension has custom initializationFailedHandler and errorHandler that retry or show a customized error notification already, so it would be nice if we can suppress them.

dbaeumer commented 2 years ago

Actually, it is on purpose that these errors are not suppressed since they indicate a deeper underlying problem (e.g. the server crashed or didn't start at all).

In that latest you can customize the message that is shown in these cases. See https://github.com/microsoft/vscode-languageserver-node/blob/17004ecb5b53f9e7bb658fe4f4794d2216e37e82/client/src/common/client.ts#L147

Can you explain why this is a normal behavior for your language server?

hyangah commented 2 years ago

Can you explain why this is a normal behavior for your language server?

Some of our advanced users launch our language server in what we call proxy mode. The proxy mode language server launched by vscode connects to a remote language server process (specified by either tcp or unix domain socket address given with a flag, or through the server's own discovery mechanism). This mode is useful when we need to debug/test our language server under development, or need to share one language server instance with other editors. In this mode, if the language server cannot connect to the remote server (e.g. the remote server is not up yet, or it crashed - this can occur often during development) or even the proxy crashes, we want the client to retry a few times. In case the remote server and the client race in start up, this retry reduces noise.

In that latest you can customize the message that is shown in these cases

Can we customize the followup action? We want more than the "Go to output" option - bug report generation.

When the user chooses the option, the extension collects necessary data (anonymized server crash trace, system info) and pre-fills a github issue.

Finally, I tested this proxy mode setup after removing all our custom initializationFailedHandler and errorHandler (so default behavior, but still with revealOutputChannelOn: Never), and observed multiple error popups are shown to user. Some error popups seemed deduped(?) but it resulted in an interesting flash effect while the client is retrying behind the scene. I wish we could suppress some of the popups.

Screen Shot 2022-06-24 at 9 11 42 AM
dbaeumer commented 2 years ago

I see your point. What we could do is the following: add an optional handled property to the Error and Cloe handler. If present and set to true we could not show any error dialog.

See code: https://github.com/microsoft/vscode-languageserver-node/blob/17004ecb5b53f9e7bb658fe4f4794d2216e37e82/client/src/common/client.ts#L147 and https://github.com/microsoft/vscode-languageserver-node/blob/17004ecb5b53f9e7bb658fe4f4794d2216e37e82/client/src/common/client.ts#L174

Would you like to give this a try and see whether it would improve the situation for you. We would also need to do something for the InitializationFailedHandler but that would be breaking right now and might need to wait until we have a new major version.

hyangah commented 2 years ago

Thanks @dbaeumer handled property can help.

Meanwhile I found explicitly specifying an empty message (return { message: '', action: CloseAction.Restart}) suppresses the popup window. I am not sure if that's intentional behavior nor what is suppressing this popup. (lsp client or vscode??) Can we make this a "feature" by documenting it?

However, I still couldn't avoid some of the forced popups in certain cases.

See this code that handles closed connection (e.g. connection closed by server): https://github.com/microsoft/vscode-languageserver-node/blob/60d4ecb5d6034d628bcc3a70b3c9359232360f07/client/src/common/client.ts#L1460-L1470

If the client is also in the Stopping state when the server-side connection close is detected first, clientOptions.errorHandler.closed will not be called. As a result, handlerResult.action remains as the default DoNotRestart and the message in line 1470 will be reported with 'force' option.

I think this problem will be present even with your handled property proposal. How about skipping the forced popup if the client-side shutdown is initiated too?