diff options
author | Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> | 2024-05-01 20:31:11 -0700 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-05-01 20:31:11 -0700 |
commit | 66b66de96a78af6162e581c2986e3ef4ec8733c4 (patch) | |
tree | c2cab8f3badfb912df0b6961223804d2e73ebf1e /cli/tsc | |
parent | ee122c5af6b8251329c213b4042a6d71ff5d9b71 (diff) |
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).
Diffstat (limited to 'cli/tsc')
-rw-r--r-- | cli/tsc/99_main_compiler.js | 27 |
1 files changed, 24 insertions, 3 deletions
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 |