Skip to content
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

Error handler unable to intercept and suppress error notification #1608

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

Error handler unable to intercept and suppress error notification #1608

rcjsuen opened this issue Feb 3, 2025 · 4 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.

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

2 participants