fix(oxfmt): avoid embedded TSFN crash by returning errors as data (take2)

apps/oxfmt/src-js/bindings.d.ts

@@ -34,7 +34,7 @@ export declare const enum Severity {
  * # Panics
  * Panics if the current working directory cannot be determined.
  */
-export declare function format(filename: string, sourceText: string, options: any | undefined | null, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[]>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<FormatResult>
+export declare function format(filename: string, sourceText: string, options: any | undefined | null, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string | null>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[] | null>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[] | null>): Promise<FormatResult>
 
 export interface FormatResult {
   /** The formatted code. */
@@ -49,7 +49,7 @@ export interface FormatResult {
  * This API is specialized for JS/TS snippets embedded in non-JS files.
  * Unlike `format()`, it is called only for js-in-xxx `textToDoc()` flow.
  */
-export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmtPluginOptionsJson: string, parentContext: string, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[]>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<string | null>
+export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmtPluginOptionsJson: string, parentContext: string, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string | null>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[] | null>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[] | null>): Promise<string | null>
 
 /**
  * NAPI based JS CLI entry point.
@@ -66,4 +66,4 @@ export declare function jsTextToDoc(sourceExt: string, sourceText: string, oxfmt
  * - `mode`: If main logic will run in JS side, use this to indicate which mode
  * - `exitCode`: If main logic already ran in Rust side, return the exit code
  */
-export declare function runCli(args: Array<string>, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[]>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindcssClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[]>): Promise<[string, number | undefined | null]>
+export declare function runCli(args: Array<string>, initExternalFormatterCb: (numThreads: number) => Promise<string[]>, formatEmbeddedCb: (options: Record<string, any>, code: string) => Promise<string | null>, formatEmbeddedDocCb: (options: Record<string, any>, texts: string[]) => Promise<string[] | null>, formatFileCb: (options: Record<string, any>, code: string) => Promise<string>, sortTailwindcssClassesCb: (options: Record<string, any>, classes: string[]) => Promise<string[] | null>): Promise<[string, number | undefined | null]>

apps/oxfmt/src-js/cli/worker-proxy.ts

@@ -32,58 +32,67 @@ export async function disposeExternalFormatter(): Promise<void> {
   pool = null;
 }
 
+// ---
+
+// Used for non-JS files formatting
+export async function formatFile(
+  options: FormatFileParam["options"],
+  code: string,
+): Promise<string> {
+  return (
+    pool!
+      .run({ options, code } satisfies FormatFileParam, { name: "formatFile" })
+      // `tinypool` with `runtime: "child_process"` serializes Error as plain objects via IPC.
+      // (e.g. `{ name, message, stack, ... }`)
+      // And napi-rs converts unknown JS values to Rust Error by calling `String()` on them,
+      // which yields `"[object Object]"` for plain objects...
+      // So, this function reconstructs a proper `Error` instance so napi-rs can extract the message.
+      .catch((err) => {
+        if (err instanceof Error) throw err;
+        if (err !== null && typeof err === "object") {
+          const obj = err as { name: string; message: string };
+          const newErr = new Error(obj.message);
+          newErr.name = obj.name;
+          throw newErr;
+        }
+        throw new Error(String(err));
+      })
+  );
+}
+
+// ---
+
+// All functions below are used for JS files with embedded code
+//
+// NOTE: These functions return `null` on error instead of throwing.
+// When errors were propagated as rejected JS promises, which become `napi::Error` values in Rust TSFN await paths.
+// In heavily concurrent runs, dropping those error values could reach `napi_reference_unref` during teardown and trigger V8 fatal checks.
+
 export async function formatEmbeddedCode(
   options: FormatEmbeddedCodeParam["options"],
   code: string,
-): Promise<string> {
+): Promise<string | null> {
   return pool!
     .run({ options, code } satisfies FormatEmbeddedCodeParam, { name: "formatEmbeddedCode" })
-    .catch(rethrowAsError);
+    .catch(() => null);
 }
 
 export async function formatEmbeddedDoc(
   options: FormatEmbeddedDocParam["options"],
   texts: string[],
-): Promise<string[]> {
+): Promise<string[] | null> {
   return pool!
     .run({ options, texts } satisfies FormatEmbeddedDocParam, {
       name: "formatEmbeddedDoc",
     })
-    .catch(rethrowAsError);
-}
-
-export async function formatFile(
-  options: FormatFileParam["options"],
-  code: string,
-): Promise<string> {
-  return pool!
-    .run({ options, code } satisfies FormatFileParam, { name: "formatFile" })
-    .catch(rethrowAsError);
+    .catch(() => null);
 }
 
 export async function sortTailwindClasses(
   options: SortTailwindClassesArgs["options"],
   classes: string[],
-): Promise<string[]> {
+): Promise<string[] | null> {
   return pool!
     .run({ classes, options } satisfies SortTailwindClassesArgs, { name: "sortTailwindClasses" })
-    .catch(rethrowAsError);
-}
-
-// ---
-
-// `tinypool` with `runtime: "child_process"` serializes Error as plain objects via IPC.
-// (e.g. `{ name, message, stack, ... }`)
-// And napi-rs converts unknown JS values to Rust Error by calling `String()` on them,
-// which yields `"[object Object]"` for plain objects...
-// So, this function reconstructs a proper `Error` instance so napi-rs can extract the message.
-function rethrowAsError(err: unknown): never {
-  if (err instanceof Error) throw err;
-  if (err !== null && typeof err === "object") {
-    const obj = err as { name: string; message: string };
-    const newErr = new Error(obj.message);
-    newErr.name = obj.name;
-    throw newErr;
-  }
-  throw new Error(String(err));
+    .catch(() => null);
 }

apps/oxfmt/src/core/external_formatter.rs

@@ -31,13 +31,13 @@ pub type JsInitExternalFormatterCb = ThreadsafeFunction<
 >;
 
 /// Type alias for the callback function signature.
-/// Takes (options, code) as arguments and returns formatted code.
+/// Takes (options, code) as arguments and returns formatted code or null on error.
 /// The `options` object includes `parser` field set by Rust side.
 pub type JsFormatEmbeddedCb = ThreadsafeFunction<
     // Input arguments
     FnArgs<(Value, String)>, // (options, code)
     // Return type (what JS function returns)
-    Promise<String>,
+    Promise<Option<String>>,
     // Arguments (repeated)
     FnArgs<(Value, String)>,
     // Error status
@@ -47,13 +47,13 @@ pub type JsFormatEmbeddedCb = ThreadsafeFunction<
 >;
 
 /// Type alias for the Doc-path callback function signature (batch).
-/// Takes (options, texts[]) as arguments and returns Doc JSON string[] (one per text).
+/// Takes (options, texts[]) as arguments and returns Doc JSON string[] (one per text) or null on error.
 /// The `options` object includes `parser` field set by Rust side.
 pub type JsFormatEmbeddedDocCb = ThreadsafeFunction<
     // Input arguments
     FnArgs<(Value, Vec<String>)>, // (options, texts)
     // Return type (what JS function returns)
-    Promise<Vec<String>>,
+    Promise<Option<Vec<String>>>,
     // Arguments (repeated)
     FnArgs<(Value, Vec<String>)>,
     // Error status
@@ -79,11 +79,11 @@ pub type JsFormatFileCb = ThreadsafeFunction<
 >;
 
 /// Type alias for Tailwind class processing callback.
-/// Takes (options, classes) and returns sorted array.
+/// Takes (options, classes) and returns sorted array or null on error.
 /// The `filepath` is included in `options`.
 pub type JsSortTailwindClassesCb = ThreadsafeFunction<
     FnArgs<(Value, Vec<String>)>, // Input: (options, classes)
-    Promise<Vec<String>>,         // Return: promise of sorted array
+    Promise<Option<Vec<String>>>, // Return: promise of sorted array or null
     FnArgs<(Value, Vec<String>)>,
     Status,
     false,
@@ -427,7 +427,10 @@ fn wrap_format_embedded(
             let status = cb.call_async(FnArgs::from((options, code.to_string()))).await;
             match status {
                 Ok(promise) => match promise.await {
-                    Ok(formatted_code) => Ok(formatted_code),
+                    Ok(Some(formatted_code)) => Ok(formatted_code),
+                    Ok(None) => Err("Embedded formatting failed".to_string()),
+                    // JS side never rejects; it returns `null` on error instead.
+                    // `Err` here would only come from a napi-rs internal failure.
                     Err(err) => Err(err.reason.clone()),
                 },
                 Err(err) => Err(err.reason.clone()),
@@ -453,7 +456,10 @@ fn wrap_format_embedded_doc(
             let status = cb.call_async(FnArgs::from((options, texts_owned))).await;
             match status {
                 Ok(promise) => match promise.await {
-                    Ok(doc_jsons) => Ok(doc_jsons),
+                    Ok(Some(doc_jsons)) => Ok(doc_jsons),
+                    Ok(None) => Err("Embedded doc formatting failed".to_string()),
+                    // JS side never rejects; it returns `null` on error instead.
+                    // `Err` here would only come from a napi-rs internal failure.
                     Err(err) => Err(err.reason.clone()),
                 },
                 Err(err) => Err(err.reason.clone()),
@@ -503,11 +509,11 @@ fn wrap_sort_tailwind_classes(
             let args = FnArgs::from((options.clone(), classes.clone()));
             match cb.call_async(args).await {
                 Ok(promise) => match promise.await {
-                    Ok(sorted) => sorted,
-                    // Return original classes on error
-                    Err(_) => classes,
+                    Ok(Some(sorted)) => sorted,
+                    // JS side never rejects; it returns `null` on error instead.
+                    // `Err` here would only come from a napi-rs internal failure.
+                    Ok(None) | Err(_) => classes,
                 },
-                // Return original classes on error
                 Err(_) => classes,
             }
         });

apps/oxfmt/src/main_napi.rs

@@ -36,13 +36,17 @@ pub async fn run_cli(
     args: Vec<String>,
     #[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
     init_external_formatter_cb: JsInitExternalFormatterCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
+    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string | null>")]
     format_embedded_cb: JsFormatEmbeddedCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[] | null>"
+    )]
     format_embedded_doc_cb: JsFormatEmbeddedDocCb,
     #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
     format_file_cb: JsFormatFileCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[] | null>"
+    )]
     sort_tailwindcss_classes_cb: JsSortTailwindClassesCb,
 ) -> (String, Option<u8>) {
     // Convert `String` args to `OsString` for compatibility with `bpaf`
@@ -143,13 +147,17 @@ pub async fn format(
     options: Option<Value>,
     #[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
     init_external_formatter_cb: JsInitExternalFormatterCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
+    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string | null>")]
     format_embedded_cb: JsFormatEmbeddedCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[] | null>"
+    )]
     format_embedded_doc_cb: JsFormatEmbeddedDocCb,
     #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
     format_file_cb: JsFormatFileCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[] | null>"
+    )]
     sort_tailwind_classes_cb: JsSortTailwindClassesCb,
 ) -> FormatResult {
     let format_api::ApiFormatResult { code, errors } = format_api::run(
@@ -182,13 +190,17 @@ pub async fn js_text_to_doc(
     parent_context: String,
     #[napi(ts_arg_type = "(numThreads: number) => Promise<string[]>")]
     init_external_formatter_cb: JsInitExternalFormatterCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
+    #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string | null>")]
     format_embedded_cb: JsFormatEmbeddedCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, texts: string[]) => Promise<string[] | null>"
+    )]
     format_embedded_doc_cb: JsFormatEmbeddedDocCb,
     #[napi(ts_arg_type = "(options: Record<string, any>, code: string) => Promise<string>")]
     format_file_cb: JsFormatFileCb,
-    #[napi(ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[]>")]
+    #[napi(
+        ts_arg_type = "(options: Record<string, any>, classes: string[]) => Promise<string[] | null>"
+    )]
     sort_tailwind_classes_cb: JsSortTailwindClassesCb,
 ) -> Option<String> {
     utils::init_tracing();