perf(linter/plugins): move result-processing work off main JS thread

apps/oxlint/src/js_plugins/external_linter.rs

@@ -1,7 +1,4 @@
-use std::{
-    error::Error,
-    sync::{atomic::Ordering, mpsc::channel},
-};
+use std::sync::{atomic::Ordering, mpsc::channel};
 
 use napi::{
     Status,
@@ -45,17 +42,23 @@ fn wrap_load_plugin(cb: JsLoadPluginCb) -> ExternalLinterLoadPluginCb {
         let cb = &cb;
         let res = tokio::task::block_in_place(|| {
             tokio::runtime::Handle::current().block_on(async move {
-                let result = cb
-                    .call_async(FnArgs::from((plugin_url, package_name)))
-                    .await?
-                    .into_future()
-                    .await?;
-                let plugin_load_result: PluginLoadResult = serde_json::from_str(&result)?;
-                Ok(plugin_load_result)
+                cb.call_async(FnArgs::from((plugin_url, package_name))).await?.into_future().await
             })
         });
 
-        res.map_err(|err: Box<dyn Error>| format!("Error in `loadPlugin` callback: {err:?}"))
+        match res {
+            // `loadPlugin` returns JSON string if plugin loaded successfully, or an error occurred
+            Ok(json) => match serde_json::from_str::<PluginLoadResult>(&json) {
+                // Plugin loaded successfully, or error occurred on JS side
+                Ok(result) => Ok(result),
+                // Invalid JSON - should be impossible, because we control serialization on JS side
+                Err(err) => {
+                    Err(format!("Failed to deserialize JSON returned by `loadPlugin`: {err}"))
+                }
+            },
+            // `loadPlugin` threw an error - should be impossible because `loadPlugin` is wrapped in try-catch
+            Err(err) => Err(format!("`loadPlugin` threw an error: {err}")),
+        }
     })
 }
 
@@ -80,19 +83,19 @@ fn wrap_setup_configs(cb: JsSetupConfigsCb) -> ExternalLinterSetupConfigsCb {
             options_json,
             ThreadsafeFunctionCallMode::NonBlocking,
             move |result, _env| {
-                let _ = match &result {
-                    Ok(()) => tx.send(Ok(())),
-                    Err(e) => tx.send(Err(e.to_string())),
-                };
-                result
+                let _ = tx.send(result);
+                Ok(())
             },
         );
 
         if status == Status::Ok {
             match rx.recv() {
+                // Setup succeeded
                 Ok(Ok(())) => Ok(()),
-                Ok(Err(err)) => Err(format!("`setupConfigs` callback reported error: {err}")),
-                Err(err) => Err(format!("`setupConfigs` callback did not respond: {err}")),
+                // `setupConfigs` threw an error - should be impossible because it should be infallible
+                Ok(Err(err)) => Err(format!("`setupConfigs` threw an error: {err}")),
+                // Failure calling JS function
+                Err(err) => Err(format!("`setupConfigs` did not respond: {err}")),
             }
         } else {
             Err(format!("Failed to schedule `setupConfigs` callback: {status:?}"))
@@ -132,29 +135,32 @@ fn wrap_lint_file(cb: JsLintFileCb) -> ExternalLinterLintFileCb {
                 FnArgs::from((file_path, buffer_id, buffer, rule_ids, options_ids, settings_json)),
                 ThreadsafeFunctionCallMode::NonBlocking,
                 move |result, _env| {
-                    let _ = match &result {
-                        Ok(None) => tx.send(Ok(LintFileReturnValue::Success(Vec::new()))),
-                        Ok(Some(r)) => match serde_json::from_str::<LintFileReturnValue>(r) {
-                            Ok(v) => tx.send(Ok(v)),
-                            Err(_e) => {
-                                tx.send(Err("Failed to deserialize lint result".to_string()))
-                            }
-                        },
-                        Err(e) => tx.send(Err(e.to_string())),
-                    };
-
-                    result.map(|_| ())
+                    let _ = tx.send(result);
+                    Ok(())
                 },
             );
 
             if status == Status::Ok {
                 match rx.recv() {
-                    Ok(Ok(x)) => match x {
-                        LintFileReturnValue::Success(diagnostics) => Ok(diagnostics),
-                        LintFileReturnValue::Failure(err) => Err(err),
-                    },
-                    Ok(Err(err)) => Err(format!("`lintFile` callback reported error: {err}")),
-                    Err(err) => Err(format!("`lintFile` callback did not respond: {err}")),
+                    // `lintFile` returns `null` if no diagnostics reported, and no error occurred
+                    Ok(Ok(None)) => Ok(Vec::new()),
+                    // `lintFile` returns JSON string if diagnostics reported, or an error occurred
+                    Ok(Ok(Some(json))) => {
+                        match serde_json::from_str(&json) {
+                            // Diagnostics reported
+                            Ok(LintFileReturnValue::Success(diagnostics)) => Ok(diagnostics),
+                            // Error occurred on JS side
+                            Ok(LintFileReturnValue::Failure(err)) => Err(err),
+                            // Invalid JSON - should be impossible, because we control serialization on JS side
+                            Err(err) => Err(format!(
+                                "Failed to deserialize JSON returned by `lintFile`: {err}"
+                            )),
+                        }
+                    }
+                    // `lintFile` threw an error - should be impossible because `lintFile` is wrapped in try-catch
+                    Ok(Err(err)) => Err(format!("`lintFile` threw an error: {err}")),
+                    // Failure calling JS function
+                    Err(err) => Err(format!("`lintFile` did not respond: {err}")),
                 }
             } else {
                 Err(format!("Failed to schedule `lintFile` callback: {status:?}"))