bun test: report a test file or preload the loader cannot resolve instead of exiting; keep the JUnit report valid UTF-8

src/jsc/JSModuleLoader.rs

@@ -1,3 +1,5 @@
+use core::ptr::NonNull;
+
 use crate::{JSGlobalObject, JSInternalPromise, JsError, JsResult};
 use bun_core::String as BunString;
 
@@ -28,33 +30,60 @@ impl JSModuleLoader {
     /// Raw-pointer variant of `load_and_evaluate_module`. Returns the FFI
     /// `*mut JSInternalPromise` directly so callers that need to store or pass
     /// a mutable cell pointer don't launder provenance through `&T -> *mut T`.
+    ///
+    /// Every load failure comes back as a rejected promise (see
+    /// [`Self::reject_with_thrown_exception`]); `None` only while the VM is
+    /// being terminated, with the termination exception left pending.
     pub fn load_and_evaluate_module_ptr(
         global_object: *mut JSGlobalObject,
         module_name: Option<&BunString>,
-    ) -> Option<core::ptr::NonNull<JSInternalPromise>> {
+    ) -> Option<NonNull<JSInternalPromise>> {
         // `JSGlobalObject` is an opaque ZST handle; `opaque_ref` is the
         // centralised zero-byte deref proof (panics on null).
-        core::ptr::NonNull::new(JSC__JSModuleLoader__loadAndEvaluateModule(
-            JSGlobalObject::opaque_ref(global_object),
+        let global = JSGlobalObject::opaque_ref(global_object);
+        NonNull::new(JSC__JSModuleLoader__loadAndEvaluateModule(
+            global,
             module_name,
         ))
+        .or_else(|| Self::reject_with_thrown_exception(global))
     }
 
     /// Raw-pointer variant of `Self::import`. Returns the FFI
     /// `*mut JSInternalPromise` directly so callers that need to store or pass
     /// a mutable cell pointer (e.g. `VirtualMachine::pending_internal_promise`)
     /// don't launder provenance through `&T -> *mut T`. Mirrors
-    /// [`Self::load_and_evaluate_module_ptr`].
+    /// [`Self::load_and_evaluate_module_ptr`], including how failures are
+    /// reported: `Err` only while the VM is being terminated.
     pub fn import_ptr(
         global_object: *mut JSGlobalObject,
         module_name: &BunString,
-    ) -> JsResult<core::ptr::NonNull<JSInternalPromise>> {
+    ) -> JsResult<NonNull<JSInternalPromise>> {
         // `JSGlobalObject` is an opaque ZST handle; `opaque_ref` is the
         // centralised zero-byte deref proof (panics on null).
-        core::ptr::NonNull::new(JSModuleLoader__import(
-            JSGlobalObject::opaque_ref(global_object),
-            module_name,
-        ))
-        .ok_or(JsError::Thrown)
+        let global = JSGlobalObject::opaque_ref(global_object);
+        NonNull::new(JSModuleLoader__import(global, module_name))
+            .or_else(|| Self::reject_with_thrown_exception(global))
+            .ok_or(JsError::Thrown)
+    }
+
+    /// JSC resolves the specifier before it has a promise to reject
+    /// (Completion.cpp `loadAndEvaluateModule`, JSModuleLoader.cpp
+    /// `requestImportModule`), so an unresolvable one is thrown and the binding
+    /// returns null. One way to get there is a path whose bytes are not valid
+    /// UTF-8: once it is a JS string it no longer names the file. The callers
+    /// report load failures from the promise, so deliver the error that way,
+    /// marked handled like the loader's own promises so the unhandled
+    /// rejection tracker does not report it a second time.
+    fn reject_with_thrown_exception(global: &JSGlobalObject) -> Option<NonNull<JSInternalPromise>> {
+        let exception = global.try_take_exception()?;
+        // `try_take_exception` leaves a termination exception pending; so do we.
+        if exception.is_termination_exception() {
+            return None;
+        }
+        let promise = JSInternalPromise::create(global);
+        promise
+            .reject_as_handled(global, exception.to_error().unwrap_or(exception))
+            .ok()?;
+        Some(NonNull::from(promise))
     }
 }

src/jsc/ResolveMessage.rs

@@ -186,22 +186,18 @@ impl ResolveMessage {
                     let _ = write!(&mut out, "Module not found '{}'", BStr::new(specifier));
                     return out;
                 }
-                if bun_resolver::is_package_path(specifier)
+                let what = if bun_resolver::is_package_path(specifier)
                     && !strings::contains_char(specifier, b'/')
                 {
-                    let _ = write!(
-                        &mut out,
-                        "Cannot find package '{}' from '{}'",
-                        BStr::new(specifier),
-                        BStr::new(referrer),
-                    );
+                    "package"
                 } else {
-                    let _ = write!(
-                        &mut out,
-                        "Cannot find module '{}' from '{}'",
-                        BStr::new(specifier),
-                        BStr::new(referrer),
-                    );
+                    "module"
+                };
+                let _ = write!(&mut out, "Cannot find {what} '{}'", BStr::new(specifier));
+                // Entry points and preloads are loaded directly, not imported
+                // from anywhere; Node words those the same way.
+                if !referrer.is_empty() {
+                    let _ = write!(&mut out, " from '{}'", BStr::new(referrer));
                 }
                 return out;
             }

src/jsc/bindings/ZigGlobalObject.cpp

@@ -3609,7 +3609,7 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
     Zig::GlobalObject* globalObject = static_cast<Zig::GlobalObject*>(jsGlobalObject);
 
     ErrorableString res;
-    res.success = false;
+    memset(&res, 0, sizeof(res));
 
     BunString keyZ;
     if (key.isString()) {
@@ -3682,7 +3682,11 @@ JSC::Identifier GlobalObject::moduleLoaderResolve(JSGlobalObject* jsGlobalObject
         return result;
     } else {
         auto scope = DECLARE_THROW_SCOPE(globalObject->vm());
-        throwException(scope, res.result.err, globalObject);
+        // The resolver leaves `res` untouched when it threw (a plugin's onResolve
+        // throwing, for instance); that exception is the one to keep. Same shape
+        // as moduleLoaderImportModule below.
+        if (!scope.exception())
+            throwException(scope, res.result.err, globalObject);
         return globalObject->vm().propertyNames->emptyIdentifier;
     }
 }

src/runtime/cli/test_command.rs

@@ -141,7 +141,23 @@ mod bun_test {
     }
 }
 
+/// The report declares `encoding="UTF-8"`, but file paths, the hostname and the
+/// CI env vars are raw OS bytes: ill-formed sequences become U+FFFD, as the
+/// console already prints them (`bstr::BStr`).
 pub(crate) fn escape_xml(str_: &[u8], writer: &mut impl bun_io::Write) -> crate::Result<()> {
+    if strings::is_valid_utf8(str_) {
+        return escape_xml_utf8(str_, writer);
+    }
+    for chunk in str_.utf8_chunks() {
+        escape_xml_utf8(chunk.valid().as_bytes(), writer)?;
+        if !chunk.invalid().is_empty() {
+            writer.write_all("\u{FFFD}".as_bytes())?;
+        }
+    }
+    Ok(())
+}
+
+fn escape_xml_utf8(str_: &[u8], writer: &mut impl bun_io::Write) -> crate::Result<()> {
     let mut last: usize = 0;
     let mut i: usize = 0;
     let len = str_.len();

src/runtime/jsc_hooks.rs

@@ -712,8 +712,10 @@ fn generate_entry_point(_vm: &VirtualMachine, watch: bool, entry_path: &[u8]) ->
 /// preload promise if any, else null.
 ///
 /// Error mapping: resolver `Failure` returns the resolver error,
-/// `Pending`/`NotFound` returns `error.ModuleNotFound`,
-/// `JSModuleLoader.import` throwing returns `error.JSError`.
+/// `Pending`/`NotFound` returns `error.ModuleNotFound`. A preload the module
+/// loader cannot load comes back as a rejected promise like any other failing
+/// preload; `JSModuleLoader.import` only fails (`error.JSError`) when the VM is
+/// being terminated.
 ///
 /// # Safety
 /// `vm` is the live per-thread VM.

test/cli/run/preload-test.test.js

@@ -1,7 +1,7 @@
 import { spawnSync } from "bun";
 import { describe, expect, test } from "bun:test";
 import { mkdirSync, realpathSync } from "fs";
-import { bunEnv, bunExe } from "harness";
+import { bunEnv, bunExe, tempDir } from "harness";
 import { tmpdir } from "os";
 import { join } from "path";
 const preloadModule = `
@@ -210,4 +210,55 @@ plugin({
       expect(exitCode).toBe(1);
     }
   });
+
+  async function run(dir, ...args) {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), ...args],
+      cwd: String(dir),
+      stderr: "pipe",
+      stdout: "pipe",
+      env: bunEnv,
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { stdout, stderr, exitCode };
+  }
+
+  // A preload is handed to the module loader as the specifier itself; node:
+  // names skip the file resolver, so this is the loader's own refusal, which
+  // used to be reported as nothing more than "Error occurred loading entry point: JSError".
+  test("reports a preloaded node: module that does not exist", async () => {
+    using dir = tempDir("bun-preload-missing-builtin", { "main.js": `console.log("RAN main");` });
+    const result = await run(dir, "--preload", "node:does_not_exist", "main.js");
+
+    expect(result).toEqual({
+      stdout: "",
+      stderr: expect.stringContaining("error: No such built-in module: node:does_not_exist\n"),
+      exitCode: 1,
+    });
+  });
+
+  // Resolving the entry point runs the plugin; its exception used to be
+  // replaced by an uninitialized one, which crashed the process.
+  test("reports the error thrown by a preloaded plugin's onResolve for the entry point", async () => {
+    using dir = tempDir("bun-preload-plugin-throws", {
+      "plugin.js": `
+        Bun.plugin({
+          name: "refuse",
+          setup(build) {
+            build.onResolve({ filter: /main\\.js$/ }, () => {
+              throw new Error("refused by onResolve");
+            });
+          },
+        });
+      `,
+      "main.js": `console.log("RAN main");`,
+    });
+    const result = await run(dir, "--preload", "./plugin.js", "./main.js");
+
+    expect(result).toEqual({
+      stdout: "",
+      stderr: expect.stringContaining("error: refused by onResolve\n"),
+      exitCode: 1,
+    });
+  });
 });

test/cli/test/bun-test.test.ts

@@ -1,6 +1,6 @@
 import { spawnSync } from "bun";
 import { beforeAll, describe, expect, it, test } from "bun:test";
-import { bunEnv, bunExe, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
+import { bunEnv, bunExe, canCreateNonUtf8FileNames, isWindows, tempDir, tempDirWithFiles, tmpdirSync } from "harness";
 import { mkdirSync, rmSync, writeFileSync } from "node:fs";
 import { dirname, join, resolve } from "node:path";
 
@@ -1749,3 +1749,119 @@ describe.concurrent("test file discovery (scanner)", () => {
     expect(exitCode).toBe(0);
   });
 });
+
+// A scanned file, and each --preload, is handed to the module loader as the
+// specifier itself. When that specifier does not resolve, the failure used to
+// end the whole run: the file's header, then exit 1 with nothing else printed.
+// Each case below is one way to make such a specifier; what is asserted is that
+// the failure is reported under the file and the other files still run.
+describe.concurrent("test files and preloads the module loader cannot resolve", () => {
+  const passing = (name: string) =>
+    `import { test } from "bun:test"; test("t", () => { console.log("RAN ${name}"); });`;
+
+  async function runBunTest(dir: string, ...args: string[]) {
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), "test", ...args],
+      env: bunEnv,
+      cwd: dir,
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { stdout, stderr, exitCode };
+  }
+
+  test("a preload plugin whose onResolve throws for a file fails that file with the thrown error", async () => {
+    using dir = tempDir("unresolvable-plugin-throw", {
+      "plugin.ts": `
+        Bun.plugin({
+          name: "refuse",
+          setup(build) {
+            build.onResolve({ filter: /refused\\.test\\.ts$/ }, () => {
+              throw new Error("refused by onResolve");
+            });
+          },
+        });
+      `,
+      "a_first.test.ts": passing("a_first"),
+      "refused.test.ts": passing("refused"),
+      "z_last.test.ts": passing("z_last"),
+    });
+    const { stdout, stderr, exitCode } = await runBunTest(String(dir), "--preload", "./plugin.ts");
+
+    expect(stdout).toContain("RAN a_first");
+    expect(stdout).toContain("RAN z_last");
+    expect(stdout).not.toContain("RAN refused");
+    expect(stderr).toContain("\nrefused.test.ts:\n");
+    expect(stderr).toContain("error: refused by onResolve\n");
+    expect(stderr).toContain(" 2 pass");
+    expect(stderr).toContain(" 1 fail");
+    expect(stderr).toMatch(/\bacross 3 files\./);
+    expect(exitCode).toBe(1);
+  });
+
+  // '?' is not a legal file name character on Windows. Elsewhere the loader
+  // reads it as the start of a query string, so the file itself is not found.
+  test.skipIf(isWindows)("a file whose name contains '?' fails without stopping the run", async () => {
+    using dir = tempDir("unresolvable-question-mark", {
+      "a_first.test.ts": passing("a_first"),
+      "what?.test.ts": passing("what"),
+      "z_last.test.ts": passing("z_last"),
+    });
+    const { stdout, stderr, exitCode } = await runBunTest(String(dir));
+
+    expect(stdout).toContain("RAN a_first");
+    expect(stdout).toContain("RAN z_last");
+    expect(stdout).not.toContain("RAN what");
+    expect(stderr).toContain("\nwhat?.test.ts:\n");
+    // Named as the entry point it was loaded as: no "from", nothing imported it.
+    expect(stderr).toMatch(/error: Cannot find module '[^']*\/what\?\.test\.ts'\n/);
+    expect(stderr).toContain(" 2 pass");
+    expect(stderr).toContain(" 1 fail");
+    expect(stderr).toMatch(/\bacross 3 files\./);
+    expect(exitCode).toBe(1);
+  });
+
+  // Such a name only survives as raw bytes; as a specifier it holds U+FFFD and
+  // no longer names the file (Node cannot load it either). Buffer paths are the
+  // only way to create one, so these files cannot be part of the tempDir tree.
+  test.skipIf(!canCreateNonUtf8FileNames())("file and directory names that are not valid UTF-8", async () => {
+    using dir = tempDir("unresolvable-non-utf8", {
+      "a_first.test.ts": passing("a_first"),
+      "z_last.test.ts": passing("z_last"),
+    });
+    const invalidByte = Buffer.from([0xff]);
+    const root = Buffer.from(String(dir) + "/");
+    writeFileSync(Buffer.concat([root, Buffer.from("b"), invalidByte, Buffer.from(".test.ts")]), passing("unloadable"));
+    const subdir = Buffer.concat([root, Buffer.from("dir"), invalidByte]);
+    mkdirSync(subdir);
+    writeFileSync(Buffer.concat([subdir, Buffer.from("/inner.test.ts")]), passing("unloadable"));
+    const { stdout, stderr, exitCode } = await runBunTest(String(dir));
+
+    expect(stdout).toContain("RAN a_first");
+    expect(stdout).toContain("RAN z_last");
+    expect(stdout).not.toContain("RAN unloadable");
+    expect(stderr).toContain("\nb\uFFFD.test.ts:\n");
+    expect(stderr).toMatch(/error: Cannot find module '[^']*\/b\uFFFD\.test\.ts'\n/);
+    expect(stderr).toContain("\ndir\uFFFD/inner.test.ts:\n");
+    expect(stderr).toMatch(/error: Cannot find module '[^']*\/dir\uFFFD\/inner\.test\.ts'\n/);
+    expect(stderr).toContain(" 2 pass");
+    expect(stderr).toContain(" 2 fail");
+    expect(stderr).toMatch(/\bacross 4 files\./);
+    expect(exitCode).toBe(1);
+  });
+
+  // node: specifiers skip the file resolver, so the module loader is the first
+  // thing that rejects this one.
+  test("--preload of a node: module that does not exist reports it", async () => {
+    using dir = tempDir("unresolvable-node-preload", { "a.test.ts": passing("a") });
+    const { stdout, stderr, exitCode } = await runBunTest(String(dir), "--preload", "node:does_not_exist");
+
+    expect(stdout).not.toContain("RAN a");
+    expect(stderr).toContain("\na.test.ts:\n");
+    expect(stderr).toContain("error: No such built-in module: node:does_not_exist\n");
+    expect(stderr).toContain(" 0 pass");
+    expect(stderr).toContain(" 1 fail");
+    expect(exitCode).toBe(1);
+  });
+});