diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index ca5a75f82164..6d408495945c 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -68,9 +68,7 @@ pub type ExceptionList = Vec; 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. + /// CJS entry or throwing runMain override: reported with Node's `uncaughtException` origin, not `unhandledRejection`. pub evaluated_as_cjs: bool, } @@ -2312,6 +2310,7 @@ unsafe extern "C" { ctx: *mut c_void, callback: extern "C" fn(ctx: *mut c_void), ); + /// Returns zero iff it threw (override not callable, or threw). safe fn NodeModuleModule__callOverriddenRunMain( global: &JSGlobalObject, argv1: JSValue, @@ -2702,19 +2701,31 @@ 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) => { + // Calling the original runMain stores a promise; else wrap the return value. + if let Some(stored) = self.pending_internal_promise { + return Ok(stored); + } + JSC__JSInternalPromise__resolvedPromise(global_ref, ret) + } + Err(err) => { + // Marked handled so only the caller reports the rejection. + 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); } } diff --git a/src/jsc/bindings/ZigGlobalObject.h b/src/jsc/bindings/ZigGlobalObject.h index 35a8a4f69ec6..af8482963fa7 100644 --- a/src/jsc/bindings/ZigGlobalObject.h +++ b/src/jsc/bindings/ZigGlobalObject.h @@ -517,7 +517,9 @@ class GlobalObject : public Bun::GlobalScope { /* TODO: these should use LazyProperty */ \ \ V(public, LazyPropertyOfGlobalObject, m_moduleResolveFilenameFunction) \ - V(public, LazyPropertyOfGlobalObject, m_moduleRunMainFunction) \ + V(public, LazyPropertyOfGlobalObject, m_moduleRunMainFunction) \ + /* Last value assigned to `Module.runMain` (may be non-callable); empty while original. */ \ + V(public, WriteBarrier, m_moduleRunMainOverride) \ V(public, LazyPropertyOfGlobalObject, m_modulePrototypeUnderscoreCompileFunction) \ V(public, LazyPropertyOfGlobalObject, m_commonJSRequireESMFromHijackedExtensionFunction) \ V(public, LazyPropertyOfGlobalObject, m_nodeModuleConstructor) \ @@ -820,8 +822,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 diff --git a/src/jsc/modules/NodeModuleModule.cpp b/src/jsc/modules/NodeModuleModule.cpp index b8732d7d8fee..1c3544100f3b 100644 --- a/src/jsc/modules/NodeModuleModule.cpp +++ b/src/jsc/modules/NodeModuleModule.cpp @@ -808,25 +808,36 @@ 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); + +// Calls the runMain replacement set by a preload; returns zero if it is not callable or throws. +extern "C" JSC::EncodedJSValue NodeModuleModule__callOverriddenRunMain(Zig::GlobalObject* global, JSC::EncodedJSValue encodedArgv1) { - auto overrideHandler = uncheckedDowncast(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 +847,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 +1148,7 @@ void addNodeModuleConstructorProperties(JSC::VM& vm, }); globalObject->m_moduleRunMainFunction.initLater( - [](const Zig::GlobalObject::Initializer& init) { + [](const Zig::GlobalObject::Initializer& init) { JSFunction* runMainFunction = JSFunction::create( init.vm, init.owner, 2, "runMain"_s, jsFunctionRunMain, JSC::ImplementationVisibility::Public, diff --git a/test/js/node/module/node-module-module.test.js b/test/js/node/module/node-module-module.test.js index 9014990b19f1..fee72513cdec 100644 --- a/test/js/node/module/node-module-module.test.js +++ b/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],