mirror of
https://github.com/denoland/deno.git
synced 2024-11-22 04:51:22 +00:00
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](0c671c9792/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).
This commit is contained in:
parent
ee122c5af6
commit
66b66de96a
@ -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 {}",
|
||||
|
@ -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
|
||||
|
Loading…
Reference in New Issue
Block a user