From 91346f3683ac6d5a9bac97d18a6444f0cf237b84 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Thu, 18 Jul 2024 16:27:14 -0400 Subject: [PATCH] Rescue `NonExistingDocumentError` in base server (#2331) --- lib/ruby_lsp/base_server.rb | 2 +- vscode/src/client.ts | 58 ++++++++++++++-------------- vscode/src/test/suite/client.test.ts | 16 ++++++++ 3 files changed, 47 insertions(+), 29 deletions(-) diff --git a/lib/ruby_lsp/base_server.rb b/lib/ruby_lsp/base_server.rb index 0419cb2eb..eb73b1373 100644 --- a/lib/ruby_lsp/base_server.rb +++ b/lib/ruby_lsp/base_server.rb @@ -53,7 +53,7 @@ def start # We don't want to try to parse documents on text synchronization notifications @store.get(parsed_uri).parse unless method.start_with?("textDocument/did") - rescue Errno::ENOENT + rescue Store::NonExistingDocumentError # If we receive a request for a file that no longer exists, we don't want to fail end end diff --git a/vscode/src/client.ts b/vscode/src/client.ts index 9f73a3058..6117a1709 100644 --- a/vscode/src/client.ts +++ b/vscode/src/client.ts @@ -338,35 +338,37 @@ export default class Client extends LanguageClient implements ClientInterface { this.logResponseTime(bench.duration, request); return result; } catch (error: any) { - if ( - this.baseFolder === "ruby-lsp" || - this.baseFolder === "ruby-lsp-rails" - ) { - await vscode.window.showErrorMessage( - `Ruby LSP error ${error.data.errorClass}: ${error.data.errorMessage}\n\n${error.data.backtrace}`, - ); - } else if (error.data) { - const { errorMessage, errorClass, backtrace } = error.data; - - if (errorMessage && errorClass && backtrace) { - // Sanitize the backtrace coming from the server to remove the user's home directory from it, then mark it as - // a trusted value. Otherwise the VS Code telemetry logger redacts the entire backtrace and we are unable to - // see where in the server the error occurred - const stack = new vscode.TelemetryTrustedValue( - backtrace - .split("\n") - .map((line: string) => line.replace(os.homedir(), "~")) - .join("\n"), - ) as any; - - this.telemetry.logError( - { - message: errorMessage, - name: errorClass, - stack, - }, - { ...error.data, serverVersion: this.serverVersion }, + if (error.data) { + if ( + this.baseFolder === "ruby-lsp" || + this.baseFolder === "ruby-lsp-rails" + ) { + await vscode.window.showErrorMessage( + `Ruby LSP error ${error.data.errorClass}: ${error.data.errorMessage}\n\n${error.data.backtrace}`, ); + } else { + const { errorMessage, errorClass, backtrace } = error.data; + + if (errorMessage && errorClass && backtrace) { + // Sanitize the backtrace coming from the server to remove the user's home directory from it, then mark it + // as a trusted value. Otherwise the VS Code telemetry logger redacts the entire backtrace and we are unable + // to see where in the server the error occurred + const stack = new vscode.TelemetryTrustedValue( + backtrace + .split("\n") + .map((line: string) => line.replace(os.homedir(), "~")) + .join("\n"), + ) as any; + + this.telemetry.logError( + { + message: errorMessage, + name: errorClass, + stack, + }, + { ...error.data, serverVersion: this.serverVersion }, + ); + } } } diff --git a/vscode/src/test/suite/client.test.ts b/vscode/src/test/suite/client.test.ts index 795b6806b..b290d51b0 100644 --- a/vscode/src/test/suite/client.test.ts +++ b/vscode/src/test/suite/client.test.ts @@ -692,4 +692,20 @@ suite("Client", () => { /lib\/ruby\/\d\.\d\.\d\/\*\*\/\*/, ); }); + + test("requests for non existing documents do not crash the server", async () => { + await assert.rejects( + async () => + client.sendRequest("textDocument/documentSymbol", { + textDocument: { + uri: documentUri.toString(), + }, + }), + (error: any) => { + assert.strictEqual(error.data, null); + assert.strictEqual(error.code, -32602); + return true; + }, + ); + }).timeout(20000); });