summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorNathan Whitaker <17734409+nathanwhit@users.noreply.github.com>2024-05-01 20:31:11 -0700
committerGitHub <noreply@github.com>2024-05-01 20:31:11 -0700
commit66b66de96a78af6162e581c2986e3ef4ec8733c4 (patch)
treec2cab8f3badfb912df0b6961223804d2e73ebf1e
parentee122c5af6b8251329c213b4042a6d71ff5d9b71 (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).
-rw-r--r--cli/lsp/tsc.rs26
-rw-r--r--cli/tsc/99_main_compiler.js27
2 files changed, 45 insertions, 8 deletions
diff --git a/cli/lsp/tsc.rs b/cli/lsp/tsc.rs
index 66e834bc2..606e47d72 100644
--- a/cli/lsp/tsc.rs
+++ b/cli/lsp/tsc.rs
@@ -1074,18 +1074,25 @@ impl TsServer {
}
let token = token.child_token();
let droppable_token = DroppableToken(token.clone());
- let (tx, rx) = oneshot::channel::<Result<String, AnyError>>();
+ let (tx, mut rx) = oneshot::channel::<Result<String, AnyError>>();
let change = self.pending_change.lock().take();
if self
.sender
- .send((req, snapshot, tx, token, change))
+ .send((req, snapshot, tx, token.clone(), change))
.is_err()
{
return Err(anyhow!("failed to send request to tsc thread"));
}
- let value = rx.await??;
- drop(droppable_token);
- Ok(serde_json::from_str(&value)?)
+ tokio::select! {
+ value = &mut rx => {
+ let value = value??;
+ drop(droppable_token);
+ Ok(serde_json::from_str(&value)?)
+ }
+ _ = token.cancelled() => {
+ Err(anyhow!("request cancelled"))
+ }
+ }
}
}
@@ -4276,6 +4283,15 @@ impl TscRuntime {
"Error during TS request \"{method}\":\n {}",
stack_trace.to_rust_string_lossy(tc_scope),
);
+ } else if let Some(message) = tc_scope.message() {
+ lsp_warn!(
+ "Error during TS request \"{method}\":\n {}\n {}",
+ message.get(tc_scope).to_rust_string_lossy(tc_scope),
+ tc_scope
+ .exception()
+ .map(|exc| exc.to_rust_string_lossy(tc_scope))
+ .unwrap_or_default()
+ );
} else {
lsp_warn!(
"Error during TS request \"{method}\":\n {}",
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