Skip to content

Commit 659ba06

Browse files
committed
fix(linter/plugins): handle error from destroyWorkspace
1 parent a09ca33 commit 659ba06

3 files changed

Lines changed: 41 additions & 3 deletions

File tree

apps/oxlint/src/js_plugins/external_linter.rs

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,41 @@ fn wrap_create_workspace(cb: JsCreateWorkspaceCb) -> ExternalLinterCreateWorkspa
326326
}
327327

328328
/// Wrap `destroyWorkspace` JS callback as a normal Rust function.
329+
///
330+
/// The JS-side `destroyWorkspace` function is synchronous, but it's wrapped in a `ThreadsafeFunction`,
331+
/// so cannot be called synchronously. Use an `mpsc::channel` to wait for the result from JS side,
332+
/// and block current thread until `destroyWorkspace` completes execution.
329333
fn wrap_destroy_workspace(cb: JsDestroyWorkspaceCb) -> ExternalLinterDestroyWorkspaceCb {
330334
Arc::new(Box::new(move |workspace_uri| {
331-
let _ = cb.call(workspace_uri, ThreadsafeFunctionCallMode::NonBlocking);
335+
let (tx, rx) = channel();
336+
337+
// Send data to JS
338+
let status = cb.call_with_return_value(
339+
workspace_uri,
340+
ThreadsafeFunctionCallMode::NonBlocking,
341+
move |result, _env| {
342+
// This call cannot fail, because `rx.recv()` below blocks until it receives a message.
343+
// This closure is a `FnOnce`, so it can't be called more than once, so only 1 message can be sent.
344+
// Therefore, `rx` cannot be dropped before this call.
345+
let res = tx.send(result);
346+
debug_assert!(res.is_ok(), "Failed to send result of `destroyWorkspace`");
347+
Ok(())
348+
},
349+
);
350+
351+
if status == Status::Ok {
352+
match rx.recv() {
353+
// Destroying workspace succeeded
354+
Ok(Ok(())) => Ok(()),
355+
// `destroyWorkspace` threw an error
356+
Ok(Err(err)) => Err(format!("`destroyWorkspace` threw an error: {err}")),
357+
// Sender "hung up" - should be impossible because closure passed to `call_with_return_value`
358+
// takes ownership of the sender `tx`. Unless NAPI-RS drops the closure without calling it,
359+
// `tx.send()` always happens before `tx` is dropped.
360+
Err(err) => Err(format!("`destroyWorkspace` did not respond: {err}")),
361+
}
362+
} else {
363+
Err(format!("Failed to schedule `destroyWorkspace` callback: {status:?}"))
364+
}
332365
}))
333366
}

apps/oxlint/src/lsp/server_linter.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,11 @@ impl ToolBuilder for ServerLinterBuilder {
301301
fn shutdown(&self, root_uri: &Uri) {
302302
// Destroy JS workspace
303303
if let Some(external_linter) = &self.external_linter {
304-
(external_linter.destroy_workspace)(root_uri.as_str().to_string());
304+
let res = (external_linter.destroy_workspace)(root_uri.as_str().to_string());
305+
306+
if let Err(err) = res {
307+
error!("Failed to destroy JS workspace:\n{err}\n");
308+
}
305309
}
306310
}
307311
}

crates/oxc_linter/src/external_linter.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@ use crate::{
1212
pub type ExternalLinterCreateWorkspaceCb =
1313
Arc<Box<dyn Fn(String) -> Result<(), String> + Send + Sync>>;
1414

15-
pub type ExternalLinterDestroyWorkspaceCb = Arc<Box<dyn Fn(String) + Send + Sync>>;
15+
pub type ExternalLinterDestroyWorkspaceCb =
16+
Arc<Box<dyn Fn(String) -> Result<(), String> + Send + Sync>>;
1617

1718
pub type ExternalLinterLoadPluginCb = Arc<
1819
Box<

0 commit comments

Comments
 (0)