Rescue NonExistingDocumentError in base server (#2331)

Shopify · Jul 18, 2024 · 91346f3 · 91346f3
1 parent c24a09c
commit 91346f3
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 29 deletions.
diff --git 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
@@ -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
@@ -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);
 });