Skip to content

Error handler unable to intercept and suppress error notification #1608

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

Open
rcjsuen opened this issue Feb 3, 2025 · 5 comments
Open

Error handler unable to intercept and suppress error notification #1608

rcjsuen opened this issue Feb 3, 2025 · 5 comments
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Milestone

Comments

@rcjsuen
Copy link
Contributor

rcjsuen commented Feb 3, 2025

let handlerResult: CloseHandlerResult = { action: CloseAction.DoNotRestart };
if (this.$state !== ClientState.Stopping) {
try {
handlerResult = await this._clientOptions.errorHandler!.closed();
} catch (error) {
// Ignore errors coming from the error handler.
}
}
this._connection = undefined;
if (handlerResult.action === CloseAction.DoNotRestart) {
this.error(handlerResult.message ?? 'Connection to server got closed. Server will not be restarted.', undefined, handlerResult.handled === true ? false : 'force');

I am developing a language server with an external dependency so I want to start/stop it myself. Unfortunately, if the dependency goes missing then the "Connection to server got closed. Server will not be restarted." message will appear if this.$state === ClientState.Stopping because I can't use the this._clientOptions.errorHandler to try to suppress it. Now when the dependency comes back the language server will restart even though the notification claimed that it "will not be restarted".

Essentially, I do not want the vscode-languageclient to popup any errors about the server being dead because I want to control everything myself but I can't avoid this particular notification (there may be others I have not encountered yet...?) because of this if statement.

@dbaeumer
Copy link
Member

dbaeumer commented Feb 5, 2025

Changing this might be a surprise for users that hook their own error handler. One idea would be to hide this behind a flag.

@dbaeumer dbaeumer added this to the On Deck milestone Feb 5, 2025
@dbaeumer dbaeumer added help wanted Issues identified as good community contribution opportunities feature-request Request for new features or functionality labels Feb 5, 2025
@dbaeumer
Copy link
Member

dbaeumer commented Feb 5, 2025

PR as always welcome.

@rcjsuen
Copy link
Contributor Author

rcjsuen commented Feb 6, 2025

Changing this might be a surprise for users that hook their own error handler. One idea would be to hide this behind a flag.

Hi @dbaeumer, for my clarity, could you rephrase what you mean by "hide this behind a flag"? Did you just mean adding adding a new boolean property to LanguageClientOptions in client/src/common/client.ts to control the new behaviour?

@dbaeumer
Copy link
Member

Did you just mean adding adding a new boolean property to LanguageClientOptions in client/src/common/client.ts to control the new behaviour?

Yes, that is exactly what I had in mind.

@noeldevelops
Copy link

We are also encountering this issue in implementing a language client for our extension.

Our language server connects over websocket and we expect occasional closures, which we handle & then reconnect. But it seems we cannot prevent the error message notification that says "Restarting server failed", even though our server restarts ok in the background and is continuing to provide intellisense in the editor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities
Projects
None yet
Development

No branches or pull requests

3 participants