From 66b66de96a78af6162e581c2986e3ef4ec8733c4 Mon Sep 17 00:00:00 2001
From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com>
Date: Wed, 1 May 2024 20:31:11 -0700
Subject: fix(lsp): Catch cancellation exceptions thrown by TSC, stop waiting
 for TS result upon cancellation (#23645)

Fixes #23643.

We weren't catching the cancellation exception thrown by TSC on the JS
side, so the rust side was catching this exception and then attempting
to print out the exception via `toString`. That last bit resulted in a
cryptic `[object Object]` showing up in the logs like so:

```
Error during TS request "getCompletionEntryDetails":
  [object Object]
```

I'm not 100% sure how we weren't seeing this in the past. My guess is
that #23409 and the subsequent PR to improve the exception catching and
logging surfaced this, but I'm still not quite clear on it.

My initial fix here returned `null` to rust when a server request was
cancelled, but this resulted in a deserialization error when we
attempted to deserialize that into the expected response type. So now,
as soon as the request's cancellation token signals we'll stop waiting
for a response and return an error (which will get swallowed as the LSP
request is being cancelled).

I was a bit surprised to find that [this
branch](https://github.com/nathanwhit/deno/blob/0c671c9792ac706c1ecd60f88efdc5eb8e941917/cli/lsp/tsc.rs#L1093)
actually executes sometimes, I believe due to the fact that aborting a
future may not [immediately stop its
execution](https://docs.rs/futures/latest/futures/stream/struct.AbortHandle.html#method.abort).
---
 cli/tsc/99_main_compiler.js | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

(limited to 'cli/tsc')

diff --git a/cli/tsc/99_main_compiler.js b/cli/tsc/99_main_compiler.js
index e5b51cab0..ddbd77ae0 100644
--- a/cli/tsc/99_main_compiler.js
+++ b/cli/tsc/99_main_compiler.js
@@ -1053,6 +1053,15 @@ delete Object.prototype.__proto__;
     debug("<<< exec stop");
   }
 
+  /**
+   * @param {any} e
+   * @returns {e is (OperationCanceledError | ts.OperationCanceledException)}
+   */
+  function isCancellationError(e) {
+    return e instanceof OperationCanceledError ||
+      e instanceof ts.OperationCanceledException;
+  }
+
   function getAssets() {
     /** @type {{ specifier: string; text: string; }[]} */
     const assets = [];
@@ -1149,14 +1158,14 @@ delete Object.prototype.__proto__;
           return respond(id, diagnosticMap);
         } catch (e) {
           if (
-            !(e instanceof OperationCanceledError ||
-              e instanceof ts.OperationCanceledException)
+            !isCancellationError(e)
           ) {
             if ("stack" in e) {
               error(e.stack);
             } else {
               error(e);
             }
+            throw e;
           }
           return respond(id, {});
         }
@@ -1168,7 +1177,19 @@ delete Object.prototype.__proto__;
           if (method == "getCompletionEntryDetails") {
             args[4] ??= undefined;
           }
-          return respond(id, languageService[method](...args));
+          try {
+            return respond(id, languageService[method](...args));
+          } catch (e) {
+            if (!isCancellationError(e)) {
+              if ("stack" in e) {
+                error(e.stack);
+              } else {
+                error(e);
+              }
+              throw e;
+            }
+            return respond(id);
+          }
         }
         throw new TypeError(
           // @ts-ignore exhausted case statement sets type to never
-- 
cgit v1.2.3