fix(child_process): throw an unhandled spawn/fork error event; close out #11490 #11350 #10730 #11359

changelog.d/11623-child-unhandled-error.md

@@ -0,0 +1,11 @@
+**child_process: an `error` event with no listener now throws, as in Node (#10730).** When `spawn()` or `fork()` fails (for example `ENOENT` for a missing binary), Perry emits a deferred `error` event on the ChildProcess. If no `error` listener was registered, Perry used to drop the error, fire `close`, and let the program continue. Node's EventEmitter throws an unhandled `error` event instead: the process dies with `Unhandled 'error' event` before `close` fires.
+
+That gap is why `test_gap_9592_child_timeout_threads` passed for Perry on macOS while Node crashed. The fixture spawned `/bin/true`, which macOS does not have, and never listened for `error`. #10730 guessed Perry was resolving the missing absolute path through `PATH`. It was not: Perry reported `ENOENT` correctly, and nothing was listening for it.
+
+The emit now reports whether any listener took the error (`failed_spawn::emit_spawn_error`). If none did, the error object is thrown as-is from the `setImmediate` callback that delivers it, after that frame's handle scopes have dropped, so `uncaughtException` and the uncaught-exit path see the same object Node rethrows. `close` is not scheduled in that case. A handled error behaves as before. No JS-callable native signature changed.
+
+Regression coverage:
+- `test-files/test_gap_10730_child_unhandled_error.ts` checks three cases: a handled error, an unhandled one, and a listener that was removed. It is byte-identical to Node with the fix. Without the fix Perry prints `close without listener` and no `uncaught` lines.
+- `child_process::failed_spawn::tests::spawn_error_{without,with}_listener_is_{unhandled,handled}` are the unit-level half.
+
+`perry-runtime --lib` single-threaded: main 4704 passed, 0 failed. This change: 4706 passed, 0 failed (the two new tests).

crates/perry-runtime/src/child_process/failed_spawn.rs

@@ -9,16 +9,56 @@ pub(super) extern "C" fn emit_error_then_close(
     closure: *const ClosureHeader,
     this: crate::closure::JsThis,
 ) -> f64 {
-    let scope = crate::gc::RuntimeHandleScope::new();
-    let cp = scope.root_nanbox_f64(cp_this(this, closure));
-    reactor::cp_emit_spawn_error(closure, this);
-    let close =
-        crate::closure::js_closure_alloc(crate::fn_info!(reactor::cp_emit_spawn_close, 0), 1);
-    crate::closure::js_closure_set_capture_ptr(close, 0, cp.get_nanbox_f64().to_bits() as i64);
-    crate::timer::js_set_timeout_callback(close as i64, 1.0);
+    let unhandled = {
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let cp = scope.root_nanbox_f64(cp_this(this, closure));
+        let unhandled = emit_spawn_error(cp.get_nanbox_f64());
+        // An unhandled error ends the process in Node before `close` fires.
+        if unhandled.is_none() {
+            let close = crate::closure::js_closure_alloc(
+                crate::fn_info!(reactor::cp_emit_spawn_close, 0),
+                1,
+            );
+            crate::closure::js_closure_set_capture_ptr(
+                close,
+                0,
+                cp.get_nanbox_f64().to_bits() as i64,
+            );
+            crate::timer::js_set_timeout_callback(close as i64, 1.0);
+        }
+        unhandled
+    };
+    throw_if_unhandled(unhandled);
     cp_undefined()
 }
 
+/// Emit the deferred spawn/fork failure `error` on the ChildProcess `cp`.
+///
+/// Returns the error when no `error` listener took it (#10730). Node's
+/// EventEmitter throws an `error` event nobody listens for, so a failed
+/// `spawn("/no/such/file")` without a handler dies with `Unhandled 'error'
+/// event`. Perry used to drop it and carry on, which made a missing binary
+/// look like a child that ran and exited.
+pub(super) fn emit_spawn_error(cp: f64) -> Option<f64> {
+    let scope = crate::gc::RuntimeHandleScope::new();
+    let cp = scope.root_nanbox_f64(cp);
+    let err = scope.root_nanbox_f64(cp_get_field(cp.get_nanbox_f64(), b"__cpError"));
+    if JSValue::from_bits(err.get_nanbox_f64().to_bits()).is_undefined() {
+        return None;
+    }
+    let handled = cp_emit(cp.get_nanbox_f64(), "error", &[err.get_nanbox_f64()]);
+    cp_set_field(cp.get_nanbox_f64(), b"signalCode", TAG_NULL_F64);
+    (!handled).then(|| err.get_nanbox_f64())
+}
+
+/// Throw `unhandled` as an uncaught exception. Called only once every handle
+/// scope in the emitting frame has dropped, so the throw skips no cleanup.
+pub(super) fn throw_if_unhandled(unhandled: Option<f64>) {
+    if let Some(err) = unhandled {
+        crate::exception::js_throw(err);
+    }
+}
+
 pub(super) fn finish_outputs(cp: f64) {
     let scope = crate::gc::RuntimeHandleScope::new();
     let cp = scope.root_nanbox_f64(cp);
@@ -75,6 +115,54 @@ mod tests {
         }
     }
 
+    extern "C" fn swallow_error(
+        _closure: *const ClosureHeader,
+        _this: crate::closure::JsThis,
+        _err: f64,
+    ) -> f64 {
+        cp_undefined()
+    }
+
+    /// A failed-spawn ChildProcess stand-in: just the fields the emit reads.
+    fn failed_child<'s>(scope: &'s crate::gc::RuntimeHandleScope) -> crate::gc::RuntimeHandle<'s> {
+        let cp = scope.root_nanbox_f64(cp_box_ptr(crate::object::js_object_alloc(0, 4).cast()));
+        let err = cp_make_error(
+            "spawn /missing ENOENT",
+            &[("code", cp_box_string("ENOENT"))],
+        );
+        cp_set_field(cp.get_nanbox_f64(), b"__cpError", err);
+        cp
+    }
+
+    /// #10730: with no `error` listener the error comes back to be thrown.
+    /// Before the fix the emit reported nothing, so the error was dropped.
+    #[test]
+    fn spawn_error_without_listener_is_unhandled() {
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let cp = failed_child(&scope);
+        let err = cp_get_field(cp.get_nanbox_f64(), b"__cpError");
+        let unhandled = emit_spawn_error(cp.get_nanbox_f64())
+            .expect("an error event nobody listens for must be thrown");
+        assert_eq!(
+            unhandled.to_bits(),
+            err.to_bits(),
+            "Node rethrows the same object"
+        );
+    }
+
+    #[test]
+    fn spawn_error_with_listener_is_handled() {
+        let scope = crate::gc::RuntimeHandleScope::new();
+        let cp = failed_child(&scope);
+        let listener = crate::closure::js_closure_alloc(crate::fn_info!(swallow_error, 1), 0);
+        super::super::emitter::cp_register(
+            cp.get_nanbox_f64(),
+            cp_box_string("error"),
+            cp_box_ptr(listener.cast()),
+        );
+        assert!(emit_spawn_error(cp.get_nanbox_f64()).is_none());
+    }
+
     #[test]
     fn absent_failed_output_is_ignored() {
         finish_output(TAG_NULL_F64);

crates/perry-runtime/src/child_process/reactor.rs

@@ -1054,13 +1054,8 @@ pub(super) extern "C" fn cp_emit_spawn_error(
     closure: *const ClosureHeader,
     this: crate::closure::JsThis,
 ) -> f64 {
-    let scope = crate::gc::RuntimeHandleScope::new();
-    let cp = scope.root_nanbox_f64(cp_this(this, closure));
-    let err = scope.root_nanbox_f64(cp_get_field(cp.get_nanbox_f64(), b"__cpError"));
-    if !JSValue::from_bits(err.get_nanbox_f64().to_bits()).is_undefined() {
-        cp_emit(cp.get_nanbox_f64(), "error", &[err.get_nanbox_f64()]);
-        cp_set_field(cp.get_nanbox_f64(), b"signalCode", TAG_NULL_F64);
-    }
+    let unhandled = super::failed_spawn::emit_spawn_error(cp_this(this, closure));
+    super::failed_spawn::throw_if_unhandled(unhandled);
     cp_undefined()
 }
 

test-files/test_gap_10730_child_unhandled_error.ts

@@ -0,0 +1,45 @@
+// #10730 — a ChildProcess `error` event with no listener must throw, as every
+// Node EventEmitter does. Perry dropped it: a failed spawn of a missing binary
+// looked like a child that ran, `close` fired, and the program carried on. That
+// is how test_gap_9592's missing `/bin/true` on macOS read as a Perry pass
+// while Node died with `Unhandled 'error' event`.
+//
+// The throw is observed through `uncaughtException` so the result is printed
+// rather than inferred from an exit status.
+import { spawn } from "node:child_process";
+
+// Absolute, and absent on every platform the suite runs on.
+const MISSING = "/nonexistent-perry-10730/true";
+
+const events: string[] = [];
+process.on("uncaughtException", (err: any) => {
+  events.push(`uncaught ${err.code} ${err.syscall} ${err instanceof Error}`);
+});
+
+// 1. With a listener: the error is delivered, nothing is thrown, `close` fires.
+await new Promise<void>((resolve) => {
+  const child = spawn(MISSING, [], { stdio: "ignore" });
+  child.on("error", (err: any) => events.push(`handled ${err.code} ${err.path}`));
+  child.on("close", (code: any) => {
+    events.push(`close ${code}`);
+    resolve();
+  });
+});
+
+// 2. Without one: the error is thrown, and `close` never fires.
+{
+  const child = spawn(MISSING, ["a"], { stdio: "ignore" });
+  child.on("close", () => events.push("close without listener"));
+}
+
+// 3. A listener that was removed again is no listener.
+{
+  const child = spawn(MISSING, ["b"], { stdio: "ignore" });
+  const onError = () => events.push("removed listener ran");
+  child.on("error", onError);
+  child.removeListener("error", onError);
+}
+
+setTimeout(() => {
+  for (const line of events) console.log(line);
+}, 200);