node:module: throw instead of crashing when a preload sets runMain to a non-callable, and report a throwing override

src/jsc/VirtualMachine.rs

@@ -68,9 +68,10 @@ pub type ExceptionList = Vec<crate::exception_list::JsException>;
 pub struct EntryPointResult {
     pub value: crate::strong::Optional, // jsc.Strong.Optional
     pub cjs_set_value: bool,
-    /// True when the entry module evaluated as CommonJS: Node reports a CJS
-    /// entry's top-level throw with origin `uncaughtException` but an ESM
-    /// entry rejection with `unhandledRejection`; the run command consults this.
+    /// True when the entry module evaluated as CommonJS (or a preload's
+    /// `Module.runMain` override threw synchronously): Node reports those with
+    /// origin `uncaughtException` but an ESM entry rejection with
+    /// `unhandledRejection`; the run command consults this.
     pub evaluated_as_cjs: bool,
 }
 
@@ -2312,6 +2313,8 @@ unsafe extern "C" {
         ctx: *mut c_void,
         callback: extern "C" fn(ctx: *mut c_void),
     );
+    /// `[[ZIG_EXPORT(zero_is_throw)]]` shape: returns zero iff it threw (the
+    /// override is not callable, or threw itself).
     safe fn NodeModuleModule__callOverriddenRunMain(
         global: &JSGlobalObject,
         argv1: JSValue,
@@ -2702,19 +2705,37 @@ impl VirtualMachine {
                     let global_ref = self.global();
                     let argv1 = jsc::bun_string_jsc::create_utf8_for_js(global_ref, MAIN_FILE_NAME)
                         .map_err(|_| crate::CrateError::JSError)?;
-                    let ret = jsc::from_js_host_call_generic(global_ref, || {
+                    let result = jsc::from_js_host_call(global_ref, || {
                         NodeModuleModule__callOverriddenRunMain(global_ref, argv1)
-                    })
-                    .map_err(|_| crate::CrateError::JSError)?;
-                    // If the override stored a promise itself, use that; otherwise
-                    // wrap its return value.
-                    if let Some(stored) = self.pending_internal_promise {
-                        return Ok(stored);
-                    }
-                    let resolved = JSC__JSInternalPromise__resolvedPromise(global_ref, ret);
-                    self.pending_internal_promise = Some(resolved);
+                    });
+                    let promise: *mut JSInternalPromise = match result {
+                        Ok(ret) => {
+                            // If the override stored a promise itself (by calling the
+                            // original runMain), use that; otherwise wrap its return value.
+                            if let Some(stored) = self.pending_internal_promise {
+                                return Ok(stored);
+                            }
+                            JSC__JSInternalPromise__resolvedPromise(global_ref, ret)
+                        }
+                        Err(err) => {
+                            // Not callable, or threw: hand the exception back as the entry
+                            // point's rejection so the caller reports it like an entry module
+                            // that throws. Marked handled up front, like the module loader's
+                            // own promises, so the caller is the only one reporting it. Node
+                            // throws this synchronously from its bootstrap, hence the
+                            // `uncaughtException` origin of a CJS entry.
+                            self.entry_point_result.evaluated_as_cjs = true;
+                            let promise = crate::JSPromise::create(global_ref);
+                            promise.set_handled();
+                            promise
+                                .reject(global_ref, Err(err))
+                                .map_err(|_| crate::CrateError::JSError)?;
+                            promise
+                        }
+                    };
+                    self.pending_internal_promise = Some(promise);
                     self.pending_internal_promise_is_protected = false;
-                    return Ok(resolved);
+                    return Ok(promise);
                 }
             }
 

src/jsc/bindings/ZigGlobalObject.h

@@ -517,7 +517,10 @@ class GlobalObject : public Bun::GlobalScope {
     /* TODO: these should use LazyProperty */                                                                \
                                                                                                              \
     V(public, LazyPropertyOfGlobalObject<JSCell>, m_moduleResolveFilenameFunction)                           \
-    V(public, LazyPropertyOfGlobalObject<JSCell>, m_moduleRunMainFunction)                                   \
+    V(public, LazyPropertyOfGlobalObject<JSFunction>, m_moduleRunMainFunction)                               \
+    /* Last value assigned to `require("module").runMain`; empty while it is the original function. */       \
+    /* Holds non-callables too (Node's is a plain data property): callability is checked at call time. */    \
+    V(public, WriteBarrier<JSC::Unknown>, m_moduleRunMainOverride)                                           \
     V(public, LazyPropertyOfGlobalObject<JSFunction>, m_modulePrototypeUnderscoreCompileFunction)            \
     V(public, LazyPropertyOfGlobalObject<JSFunction>, m_commonJSRequireESMFromHijackedExtensionFunction)     \
     V(public, LazyPropertyOfGlobalObject<JSObject>, m_nodeModuleConstructor)                                 \
@@ -820,8 +823,6 @@ class GlobalObject : public Bun::GlobalScope {
     bool hasOverriddenModuleResolveFilenameFunction = false;
     // De-optimization once `require("module").wrapper` or `require("module").wrap` is written to
     bool hasOverriddenModuleWrapper = false;
-    // De-optimization once `require("module").runMain` is written to
-    bool hasOverriddenModuleRunMain = false;
 
     // node:crypto deprecation warnings are emitted at most once per realm, like Node, whose
     // flags live in per-realm module state (lib/internal/crypto/keys.js). They must not be

src/jsc/modules/NodeModuleModule.cpp

@@ -808,25 +808,38 @@ JSC_DEFINE_HOST_FUNCTION(jsFunctionRunMain, (JSGlobalObject * globalObject, JSC:
     return JSC::JSValue::encode(JSC::jsUndefined());
 }
 
+static JSValue currentModuleRunMain(Zig::GlobalObject* globalObject)
+{
+    if (JSValue replacement = globalObject->m_moduleRunMainOverride.get())
+        return replacement;
+    return globalObject->m_moduleRunMainFunction.getInitializedOnMainThread(globalObject);
+}
+
 JSC_DEFINE_CUSTOM_GETTER(moduleRunMain,
     (JSGlobalObject * lexicalGlobalObject,
         EncodedJSValue thisValue,
         PropertyName propertyName))
 {
-    auto* globalObject = defaultGlobalObject(lexicalGlobalObject);
-
-    return JSValue::encode(
-        globalObject->m_moduleRunMainFunction.getInitializedOnMainThread(
-            globalObject));
+    return JSValue::encode(currentModuleRunMain(defaultGlobalObject(lexicalGlobalObject)));
 }
 
-extern "C" void Bun__VirtualMachine__setOverrideModuleRunMain(void* bunVM, bool isOriginal);
-extern "C" JSC::EncodedJSValue NodeModuleModule__callOverriddenRunMain(Zig::GlobalObject* global, JSValue argv1)
+extern "C" void Bun__VirtualMachine__setOverrideModuleRunMain(void* bunVM, bool isPatched);
+
+// Called by VirtualMachine::reload_entry_point when a preload replaced
+// `Module.runMain`. Throws (returns zero) if the replacement is not callable or
+// throws; Rust reports that as the entry point's failure.
+extern "C" JSC::EncodedJSValue NodeModuleModule__callOverriddenRunMain(Zig::GlobalObject* global, JSC::EncodedJSValue encodedArgv1)
 {
-    auto overrideHandler = uncheckedDowncast<JSObject>(global->m_moduleRunMainFunction.get(global));
+    auto& vm = JSC::getVM(global);
+    auto scope = DECLARE_THROW_SCOPE(vm);
+
     MarkedArgumentBuffer args;
-    args.append(argv1);
-    return JSC::JSValue::encode(JSC::profiledCall(global, JSC::ProfilingReason::API, overrideHandler, JSC::getCallData(overrideHandler), global, args));
+    args.append(JSValue::decode(encodedArgv1));
+    // Node calls it as `Module.runMain(mainPath)`, so `this` is the Module object.
+    JSValue thisValue = global->m_nodeModuleConstructor.getInitializedOnMainThread(global);
+    JSValue result = JSC::call(global, currentModuleRunMain(global), thisValue, args, "Module.runMain is not a function"_s);
+    RETURN_IF_EXCEPTION(scope, {});
+    return JSValue::encode(result);
 }
 
 JSC_DEFINE_CUSTOM_SETTER(setModuleRunMain,
@@ -836,20 +849,14 @@ JSC_DEFINE_CUSTOM_SETTER(setModuleRunMain,
 {
     auto* globalObject = defaultGlobalObject(lexicalGlobalObject);
     auto value = JSValue::decode(encodedValue);
-    if (value.isCell()) {
-        bool isOriginal = false;
-        if (value.isCallable()) {
-            JSC::CallData callData = JSC::getCallData(value);
-            if (callData.type == JSC::CallData::Type::Native) {
-                if (callData.native.function.untaggedPtr() == &jsFunctionRunMain) {
-                    isOriginal = true;
-                }
-            }
-        }
-        Bun__VirtualMachine__setOverrideModuleRunMain(globalObject->bunVM(), !isOriginal);
-        globalObject->m_moduleRunMainFunction.set(
-            lexicalGlobalObject->vm(), globalObject, value.asCell());
+    JSC::CallData callData = JSC::getCallData(value);
+    bool isOriginal = callData.type == JSC::CallData::Type::Native && callData.native.function.untaggedPtr() == &jsFunctionRunMain;
+    if (isOriginal) {
+        globalObject->m_moduleRunMainOverride.clear();
+    } else {
+        globalObject->m_moduleRunMainOverride.set(lexicalGlobalObject->vm(), globalObject, value);
     }
+    Bun__VirtualMachine__setOverrideModuleRunMain(globalObject->bunVM(), !isOriginal);
 
     return true;
 }
@@ -1143,7 +1150,7 @@ void addNodeModuleConstructorProperties(JSC::VM& vm,
         });
 
     globalObject->m_moduleRunMainFunction.initLater(
-        [](const Zig::GlobalObject::Initializer<JSCell>& init) {
+        [](const Zig::GlobalObject::Initializer<JSFunction>& init) {
             JSFunction* runMainFunction = JSFunction::create(
                 init.vm, init.owner, 2, "runMain"_s,
                 jsFunctionRunMain, JSC::ImplementationVisibility::Public,

test/js/node/module/node-module-module.test.js

@@ -1,6 +1,6 @@
 import { describe, expect, test } from "bun:test";
 import fs from "fs";
-import { bunEnv, bunExe, isWindows, ospath, tempDir } from "harness";
+import { bunEnv, bunExe, isWindows, normalizeBunSnapshot, ospath, tempDir } from "harness";
 import Module, { _nodeModulePaths, builtinModules, createRequire, isBuiltin, wrap } from "module";
 import path from "path";
 
@@ -602,6 +602,145 @@ console.log("survived", require("./late.js"));`,
     expect(stdout.trim()).toBe("pass");
     expect(await proc.exited).toBe(0);
   });
+
+  // Runs main.js with a preload that replaces Module.runMain, the way Node's
+  // bootstrap calls it: `Module.runMain(main)` after the preloads ran.
+  async function runWithRunMainPreload(preloadSource) {
+    using dir = tempDir("module-run-main", {
+      "preload.cjs": preloadSource,
+      "main.js": `console.log("main ran");`,
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "--require", "./preload.cjs", "./main.js"],
+      env: bunEnv,
+      cwd: String(dir),
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return {
+      stdout: normalizeBunSnapshot(stdout, String(dir)),
+      stderr: normalizeBunSnapshot(stderr, String(dir)),
+      exitCode,
+    };
+  }
+
+  test("Module.runMain override is called with Module as this", async () => {
+    const { stdout, stderr, exitCode } = await runWithRunMainPreload(`
+      const Module = require("module");
+      const original = Module.runMain;
+      Module.runMain = function (...args) {
+        console.log("this is Module:", this === Module);
+        return original.apply(this, args);
+      };
+    `);
+    expect(stdout).toMatchInlineSnapshot(`
+      "this is Module: true
+      main ran"
+    `);
+    expect(stderr).toBe("");
+    expect(exitCode).toBe(0);
+  });
+
+  // Node stores whatever is assigned and fails when its bootstrap calls it;
+  // the object and string cases used to segfault here, the primitives were
+  // silently ignored and main ran anyway.
+  test.each(["{}", '"not a function"', "undefined", "null", "5"])(
+    "Module.runMain = %s in a preload throws instead of running main",
+    async source => {
+      const { stdout, stderr, exitCode } = await runWithRunMainPreload(`require("module").runMain = ${source};`);
+      expect(stderr).toContain("TypeError: Module.runMain is not a function");
+      expect(stdout).toBe("");
+      expect(exitCode).toBe(1);
+    },
+  );
+
+  test.each([
+    ["a function that throws", `() => { throw new TypeError("runMain override threw"); }`, "runMain override threw"],
+    ["a class", `class NotCallable {}`, "class constructor"],
+  ])("Module.runMain override that throws (%s) reports the exception", async (_, source, expectedError) => {
+    const { stdout, stderr, exitCode } = await runWithRunMainPreload(`require("module").runMain = ${source};`);
+    expect(stderr).toContain(expectedError);
+    expect(stderr).not.toContain("Error occurred loading entry point");
+    expect(stdout).toBe("");
+    expect(exitCode).toBe(1);
+  });
+
+  test.each([
+    ["a function that throws", `() => { throw new TypeError("runMain override threw"); }`, "runMain override threw"],
+    ["a non-callable", `{}`, "Module.runMain is not a function"],
+  ])("Module.runMain override failure (%s) goes to process.on('uncaughtException')", async (_, source, message) => {
+    const { stdout, stderr, exitCode } = await runWithRunMainPreload(`
+      process.on("uncaughtException", (err, origin) => console.log("caught:", err.message, origin));
+      require("module").runMain = ${source};
+    `);
+    expect(stdout).toBe(`caught: ${message} uncaughtException`);
+    expect(stderr).toBe("");
+    expect(exitCode).toBe(0);
+  });
+
+  // A Worker's own preloads go through the same path; the failure has to end
+  // up as that worker's error (the non-callable case took down the whole process).
+  test.each([
+    ["a function that throws", `() => { throw new TypeError("runMain override threw"); }`, "runMain override threw"],
+    ["a non-callable", `{}`, "Module.runMain is not a function"],
+  ])(
+    "Module.runMain override failure (%s) in a Worker preload is reported as the worker's error",
+    async (_, source, message) => {
+      using dir = tempDir("module-run-main-worker", {
+        "preload.cjs": `require("module").runMain = ${source};`,
+        "worker.js": `console.log("worker ran");`,
+        "main.js": `
+          const worker = new Worker("./worker.js", { preload: ["./preload.cjs"] });
+          worker.onerror = event => console.log("worker error mentions it:", event.message.includes(${JSON.stringify(message)}));
+          worker.addEventListener("close", event => console.log("worker exit code:", event.code));
+        `,
+      });
+      await using proc = Bun.spawn({
+        cmd: [bunExe(), "./main.js"],
+        env: bunEnv,
+        cwd: String(dir),
+        stdout: "pipe",
+        stderr: "pipe",
+      });
+      const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+      expect(normalizeBunSnapshot(stdout)).toMatchInlineSnapshot(`
+        "worker error mentions it: true
+        worker exit code: 1"
+      `);
+      expect(stderr).toBe("");
+      expect(exitCode).toBe(0);
+    },
+  );
+
+  test("Module.runMain reads back whatever was assigned", async () => {
+    await using proc = Bun.spawn({
+      cmd: [
+        bunExe(),
+        "-e",
+        `
+          const Module = require("module");
+          const original = Module.runMain;
+          const roundTrips = [];
+          for (const value of [undefined, null, 5, "str", {}]) {
+            Module.runMain = value;
+            roundTrips.push(Module.runMain === value);
+          }
+          Module.runMain = original;
+          roundTrips.push(Module.runMain === original);
+          console.log(JSON.stringify(roundTrips));
+        `,
+      ],
+      env: bunEnv,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    expect(stdout).toBe("[true,true,true,true,true,true]\n");
+    expect(stderr).toBe("");
+    expect(exitCode).toBe(0);
+  });
+
   test.each(["no args", "--access-early"])("children, %s", async arg => {
     await using proc = Bun.spawn({
       cmd: [bunExe(), path.join(import.meta.dir, "children-fixture/a.cjs"), arg],