Skip to content

Reject the promise of a preload the module loader refuses #44838

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Reject the promise of a preload the module loader refuses #44838

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
70d5339
--hot: defer a reload that lands while the preloads load
robobun Oct 8, 2026
0cf72b5
--hot: exit with the load error when a reload cannot start
robobun Oct 8, 2026
d6f0ec1
Reject the promise of a preload the module loader refuses
robobun Oct 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/jsc/JSModuleLoader.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,4 +73,18 @@ impl JSModuleLoader {
))
.ok_or(JsError::Thrown)
}

/// The promise of a load that threw before it had one, for a load whose starter reports its
/// promise: an entry point, a preload, a test file. Rejected with what was thrown and handled,
/// like the loader's own; a termination leaves through `?`.
#[cold]
#[inline(never)]
pub fn rejected_load(
global: &JSGlobalObject,
err: JsError,
) -> JsResult<core::ptr::NonNull<JSInternalPromise>> {
let rejected = JSInternalPromise::rejected_promise_with_caught_exception(global, err)?;
rejected.set_handled();
Ok(core::ptr::NonNull::from(rejected))
}
}
50 changes: 31 additions & 19 deletions src/jsc/VirtualMachine.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2739,10 +2739,13 @@ pub struct RuntimeHooks {
/// (error already logged into `vm.log`).
pub generate_entry_point: fn(vm: &VirtualMachine, watch: bool, entry_path: &[u8]) -> bool,
/// `loadPreloads()` — runs `--preload` scripts. Returns the first rejected
/// preload promise if any, else null. Errors propagate
/// (resolver failures / `ModuleNotFound`).
/// preload promise if any (one the module loader refuses is rejected too),
/// else null. Errors propagate (resolver failures / `ModuleNotFound`).
pub load_preloads:
unsafe fn(vm: *mut VirtualMachine) -> crate::CrateResult<*mut JSInternalPromise>,
/// What `Run::start` does when the entry point's load gives an error and no
/// promise: print `vm.log` (or the error's name) and exit 1.
pub entry_point_load_failed: fn(vm: &mut VirtualMachine, err: crate::CrateError) -> !,
/// `ensureDebugger(block_until_connected)` — no-op when no debugger.
pub ensure_debugger: unsafe fn(vm: *mut VirtualMachine, block_until_connected: bool),
/// `eventLoop().autoTick()` — needs `Timer::All` for the timeout calc.
Expand Down Expand Up @@ -3517,9 +3520,11 @@ impl VirtualMachine {
let global_ref = self.global();
let promise = if !self.main_is_html_entrypoint {
let name = bun_core::String::borrow_utf8(MAIN_FILE_NAME);
jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, &name)
.map(NonNull::as_ptr)
.ok_or(crate::CrateError::JSError)?
match jsc::JSModuleLoader::load_and_evaluate_module_ptr(global, &name) {
Some(promise) => promise.as_ptr(),
None => jsc::JSModuleLoader::rejected_load(global_ref, jsc::JsError::Thrown)?
.as_ptr(),
}
} else {
let p: *mut JSInternalPromise = jsc::from_js_host_call_generic(global_ref, || {
Bun__loadHTMLEntryPoint(global_ref)
Expand All @@ -3536,10 +3541,14 @@ impl VirtualMachine {
self.entry_evaluation_started = false;
let global = self.global;
let main_str = bun_core::String::from_bytes(self.main());
let promise =
jsc::JSModuleLoader::resolve_and_load_and_evaluate_module_ptr(global, &main_str)
.map(NonNull::as_ptr)
.ok_or(crate::CrateError::JSError)?;
let promise = match jsc::JSModuleLoader::resolve_and_load_and_evaluate_module_ptr(
global, &main_str,
) {
Some(promise) => promise.as_ptr(),
// Not resolving is the entry's failure, like not loading.
None => jsc::JSModuleLoader::rejected_load(self.global(), jsc::JsError::Thrown)?
.as_ptr(),
};
self.set_pending_internal_promise(Some(promise));
Ok(promise)
}
Expand Down Expand Up @@ -4608,6 +4617,13 @@ impl VirtualMachine {
bun_core::reload_process(should_clear_terminal, false);
}

// `load_preloads` ticks the event loop between preloads and indexes the list it walks.
// A reload from inside that tick would run the preloads again and empty the list.
if self.is_in_preload {
self.hot_reload_deferred = true;
return;

@claude claude Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: Under bun test --hot, a save that lands while a test file's preloads load is now silently dropped until the next save; on the base a save after the preload settled in that tick reloaded at once. VirtualMachine.rs:4622-4624 sets hot_reload_deferred and returns, but the only consumer, report_exception_in_hot_reloaded_module_if_needed at :4512, is called solely from run_command.rs:1560-1568; test_command.rs:2300-2308 enables the same HotReloader with no equivalent call. Fix: run the deferred reload from the place load_preloads returns to (clear is_in_preload, then if hot_reload_deferred call reload) so every host of load_preloads, not only bun run, consumes the flag.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: bun test --hot (test_command.rs:2299-2308 installs HotReloader with VirtualMachine::reload as ctx) with any --preload; load_preloads (jsc_hooks.rs:701) runs for the first file and under --isolate for every file, ticking the loop at jsc_hooks.rs:850 between preloads. A file change posted by the watcher runs HotReloadTask::run (hot_reloader.rs:556-560) inside that tick; VirtualMachine.rs:4622 sees is_in_preload and sets hot_reload_deferred = true then returns. The dismissal cites run_command.rs:1560-1568 as the consumer, but grep shows report_exception_in_hot_reloaded_module_if_needed is only called from run_command.rs; test_command.rs never calls it and nothing in the test runner reads hot_reload_deferred. The next reload() at :4643 resets the flag to false without acting on the lost save. On the base, the same tick with the preload promise already fulfilled took the Fulfilled arm at :4640 and reloaded immediately. The remedy is to consume the flag where load_preloads' is_in_preload guard drops (jsc_hooks.rs:715-718) or in reload_entry_point after load_preloads returns.

Verification: pre-existing. VirtualMachine.rs:4622-4625 sets hot_reload_deferred and returns while is_in_preload; its only consumer (VirtualMachine.rs:4512-4514) is called solely from run_command.rs:1560-1568, never from test_command.rs:2299-2308, so under bun test --hot a save during load_preloads is parked until the next watcher event. The base either dropped the reload the same way or crashed.

}

if let Some(p) = self.pending_internal_promise() {
// SAFETY: `p` is a live JSC heap cell tracked by the VM.
match crate::JSPromise::status_ptr(p) {
Expand Down Expand Up @@ -4646,12 +4662,14 @@ impl VirtualMachine {
// the JSC module loader registry.
self.global().reload().expect("Failed to reload");
self.hot_reload_counter += 1;
// reload_entry_point() stores into pending_internal_promise on every return path.
// reload_entry_point() stores into pending_internal_promise whenever it returns a promise.
let main = self.main;
// Note: reshaped for borrowck — copy the `RawSlice` first to avoid
// overlapping `&self`/`&mut self` borrows.
if self.reload_entry_point(main.slice()).is_err() {
panic!("Failed to reload");
if let Err(err) = self.reload_entry_point(main.slice()) {
// No promise to report: a preload file that is gone, for one. As on the first start.
let hooks = runtime_hooks().expect("runtime hooks not installed");
(hooks.entry_point_load_failed)(self, err);
Comment on lines +4669 to +4672

@claude claude Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟣 pre-existing, not blocking: pre-existing: a --hot session whose last load had a failing preload now exits 1 when a reload runs while a preload file is momentarily absent, e.g. mid atomic save. VirtualMachine.rs:4669-4672 sends every Err of reload_entry_point to entry_point_load_failed, which exits; the NotFound arm at jsc_hooks.rs:798 returns Err for a file a write-temp-then-rename save has just unlinked. Fix: on a reload, report an unresolvable preload like a rejected one and keep waiting with its path watched, so the next save reloads; the first-start exit that hot.test.ts pins can stay. Base panicked at the same site; the PR text measures it at 19 of 30 rename saves.
A small fix can ride a push you are already making; otherwise a short reply is enough.

Why this was flagged

Trigger: bun --hot --preload ./first.js --preload ./second.js main.js where second.js threw or was refused on the last load, so load_preloads returned at jsc_hooks.rs:872 before the preload.clear() at jsc_hooks.rs:891 and the list is kept. The developer saves a preload with an editor that writes a temp file and renames it over the original. The watcher posts a HotReloadTask; Task::run (hot_reloader.rs:559) calls VirtualMachine::reload, which calls reload_entry_point while the file is unlinked. load_preloads hits ResolveResultUnion::NotFound at jsc_hooks.rs:785-798, logs preload not found and returns Err(ModuleNotFound). The new arm at VirtualMachine.rs:4669-4672 calls hooks.entry_point_load_failed (run_command.rs:1728): it prints the log and calls exit_with_unhandled_note (run_command.rs:1708), ending the process with exit code 1. The developer loses the hot session and must restart. The base branch reached panic!("Failed to reload") at the same line, so the termination is pre-existing; the PR turns it into an orderly exit.

Verification: pre-existing — acknowledged in diff: the PR description's "Downsides" section states "a save of another preload by rename can reload while that file is absent. With #44837 the process then exits 1 with preload not found (19 of 30 runs)", and the new test (test/cli/run/preload-test.test.js:481-491) pins exit 1. The base is strictly worse (panic), so merging does not regress it.

}
}

Expand Down Expand Up @@ -5650,13 +5668,7 @@ impl VirtualMachine {
Some(promise) => promise.as_ptr(),
// Not resolving is this file's failure, like not loading.
None => {
let rejected = crate::JSPromise::rejected_promise_with_caught_exception(
self.global(),
jsc::JsError::Thrown,
)?;
// Like the loader's: whoever loads the file reports it, not the rejection tracker.
rejected.set_handled();
std::ptr::from_mut(rejected)
jsc::JSModuleLoader::rejected_load(self.global(), jsc::JsError::Thrown)?.as_ptr()
}
};
self.set_pending_internal_promise(Some(promise));
Expand Down
6 changes: 4 additions & 2 deletions src/runtime/cli/run_command.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1716,14 +1716,16 @@ fn exit_with_unhandled_note(vm: &mut VirtualMachine) -> ! {
vm.global_exit();
}

/// Cold `Err(err)` arm of `vm.load_entry_point` in `Run::start`.
/// Cold `Err(err)` arm of `vm.load_entry_point` in `Run::start`, and of a `--hot` reload.
/// It exits under `--hot` and `--watch` too (#15177): nothing watches a file that was not
/// found, so no save would start a reload.
#[cold]
#[inline(never)]
#[cfg_attr(
any(target_os = "linux", target_os = "android"),
unsafe(link_section = ".text.unlikely")
)]
fn entry_point_load_failed(vm: &mut VirtualMachine, err: &crate::Error) -> ! {
pub(crate) fn entry_point_load_failed(vm: &mut VirtualMachine, err: &crate::Error) -> ! {
if log_has_msgs(vm) {
dump_build_error(vm);
log_clear_msgs(vm);
Expand Down
24 changes: 15 additions & 9 deletions src/runtime/jsc_hooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -689,11 +689,12 @@ fn generate_entry_point(_vm: &VirtualMachine, watch: bool, entry_path: &[u8]) ->
}

/// `loadPreloads()` — runs `--preload` scripts. Returns the first rejected
/// preload promise if any, else null.
/// preload promise if any, else null. A preload the module loader refuses
/// (`JSModuleLoader.import` throws) is a rejected one.
///
/// Error mapping: resolver `Failure` returns the resolver error,
/// `Pending`/`NotFound` returns `error.ModuleNotFound`,
/// `JSModuleLoader.import` throwing returns `error.JSError`.
/// `Pending`/`NotFound` returns `error.ModuleNotFound`, both with their text
/// in `vm.log`.
///
/// # Safety
/// `vm` is the live per-thread VM.
Expand Down Expand Up @@ -739,7 +740,8 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS
// `vm.transpiler.resolver`, not `vm.preload`).
let preload: *const [u8] = unsafe { &raw const *(&(*vm).preload)[i] };
// SAFETY: `preload` points at a live boxed slice for this iteration
// (heap-stable `Box<[u8]>` payload; nothing below mutates `vm.preload`).
// (heap-stable `Box<[u8]>` payload; nothing below mutates `vm.preload`:
// a reload that lands in the tick is deferred while `is_in_preload`).
let preload_slice: &[u8] = unsafe { &*preload };
// Strip "file://".
let normalized: &[u8] = preload_slice
Expand Down Expand Up @@ -811,11 +813,10 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS
let promise: *mut JSInternalPromise = match JSModuleLoader::import_ptr(global, &module_name)
{
Ok(p) => p.as_ptr(),
Err(_) => {
// The exception is
// already pending on `global`; bubble the tag so
// `reload_entry_point` forwards it.
return Err(bun_jsc::CrateError::JSError);
// The loader threw before this preload had a promise. That is the preload's
// failure, like one while it loads.
Err(err) => {
JSModuleLoader::rejected_load(JSGlobalObject::opaque_ref(global), err)?.as_ptr()
Comment on lines +818 to +819

@claude claude Bot Oct 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 (optional) CI users of bun test --parallel --reporter=junit get no JUnit file at all when a preload is refused; main writes one with a (worker crashed) failure per file. src/runtime/jsc_hooks.rs:819 turns the refusal into a rejected promise, so each worker takes the Rejected arm at src/runtime/cli/test_command.rs:2949, which counts a fail but records no test case. With nothing recorded, write_to_file at test_command.rs:867 returns before writing. Fix: record a failing test case for a file whose load promise rejects, on the serial and worker paths alike, so the report names the file and the error; that also closes #44435. The PR lists this as a downside; the exit code is still 1 but the report consumers rely on is gone.

Why this was flagged

Run bun test --parallel=2 --reporter=junit --reporter-outfile=out.xml --preload node:does_not_exist a.test.js b.test.js. On main load_preloads returned Err(JSError), the worker hit handle_top_level_test_error_before_javascript_start (src/runtime/cli/test_command.rs:3077) and Global::exit(1); the coordinator treated that as a crash (src/runtime/cli/test/parallel/Coordinator.rs:630-637) and replay_test_records wrote a (worker crashed) failing testcase per file into out.xml. After this change src/runtime/jsc_hooks.rs:818-820 returns a rejected promise instead, so the worker's TestCommand::run reaches the Status::Rejected arm at src/runtime/cli/test_command.rs:2949-2988: it bumps summary().fail, and returns Ok with no record_test_case. JunitReporter::write_to_file (test_command.rs:866-869) returns early when contents.is_empty(), so out.xml is never created. The process still exits 1, but a CI step that uploads or requires the JUnit file now reports a missing artifact instead of two failures.

Verification: src/runtime/jsc_hooks.rs:816-819 turns the import_ptr Err into JSModuleLoader::rejected_load(...), so TestCommand::run takes the Status::Rejected arm at test_command.rs:2949-2988 and returns Ok(()). contents stays empty and write_to_file at test_command.rs:866-869 returns without creating the outfile. On base replay_test_records recorded a (worker crashed) testcase per file, so the JUnit file was written.

}
};

Expand Down Expand Up @@ -893,6 +894,10 @@ unsafe fn load_preloads(vm: *mut VirtualMachine) -> bun_jsc::CrateResult<*mut JS
Ok(ptr::null_mut())
}

fn entry_point_load_failed(vm: &mut VirtualMachine, err: bun_jsc::CrateError) -> ! {
crate::cli::run_command::entry_point_load_failed(vm, &err.into())
}

/// `ensureDebugger(block_until_connected)` — no-op when no debugger.
///
/// # Safety
Expand Down Expand Up @@ -1488,6 +1493,7 @@ static __BUN_RUNTIME_HOOKS: RuntimeHooks = RuntimeHooks {
deinit_runtime_state,
generate_entry_point,
load_preloads,
entry_point_load_failed,
ensure_debugger,
auto_tick,
auto_tick_active,
Expand Down
Loading
Loading