bun test: report a test file whose entry resolve throws instead of exiting the run

[robobun]

Problem

• One test file whose name (or parent directory name) is not valid UTF-8 ends the whole bun test run: banner only, exit 1, no error, no summary, no --reporter-outfile junit file, and the remaining files never run. Under --parallel the file shows up as (worker crashed: exit code 1).
• bun test loads each file by handing its path to the module loader as the entry specifier (reload_entry_point_for_test_runner, src/jsc/VirtualMachine.rs:4828). The path becomes a JS string on the way, so the undecodable byte turns into U+FFFD and the entry no longer names the file the scanner found.
• Resolving the entry specifier happens synchronously inside JSC::loadAndEvaluateModule, so that failure is a thrown exception and load_and_evaluate_module_ptr returns None, which the runner mapped to Err(JSError).
• Both per-file call sites in TestCommand::run_all_tests (and the --parallel worker loop) send any Err to handle_top_level_test_error_before_javascript_start, whose release body is Global::exit(1). The pending exception is never printed.
• bun run does not hit this: it loads a synthetic main module that imports the real file, so the same resolve failure arrives as a rejected promise and is reported.

Fix

• When load_and_evaluate_module_ptr returns None, take the pending exception and return it as an already-rejected load promise, marked handled first so the promise rejection tracker does not report it a second time.
• TestCommand::run already handles a rejected load promise (a test file with a bad import takes that path today): it reports the error under the file's header, counts the file as failed, honors --bail, and moves on. The unloadable file now prints error: Cannot find module '/path/z\uFFFD.test.ts' from '', the other files run, the summary and junit report are written, and the exit code is 1 because a file failed. The --parallel worker survives the file for the same reason.
• Verified with test/cli/test/bun-test.test.ts ("a test file whose name is not valid UTF-8", Linux only since that is the only CI platform whose file systems accept such names): both tests fail on the unfixed binary (first one never runs the other file or writes the report, second one sees worker crashed) and pass with this change.
• Also ran test/cli/test/bun-test.test.ts in full, test/cli/test/isolation.test.ts, test/cli/test/parallel.test.ts (one pre-existing timing-dependent failure in this container, --parallel lazily scales workers based on file duration, unrelated to this path), test/js/junit-reporter/junit.test.js, and test/regression/issue/26851.test.ts; and the repro under BUN_JSC_validateExceptionChecks=1.

Background

• Entry specifier: the module name handed to JSC's loadAndEvaluateModule. JSC resolves it first (calling back into Bun's resolver) and only then creates the promise that tracks fetching and evaluating the module graph. A resolve failure therefore surfaces as a synchronous throw, while anything that goes wrong later (a missing import inside the file, a top-level throw) rejects the returned promise.
• load_and_evaluate_module_ptr returning None means the C++ side threw and the exception is still pending on the VM. JSPromise::reject(global, Err(JsError::Thrown)) is the existing helper that takes that pending exception and rejects a promise with it.
• Promise rejection tracker: JSC notifies the embedder whenever a promise is rejected with no handler attached; Bun queues those and later reports them as unhandled rejections. The module loader marks its own load promises as handled because the runner reads their state directly; the promise created here is marked the same way for the same reason.

Repro

mkdir t && cd t
mkdir good; echo 'import {test,expect} from "bun:test"; test("good", ()=>{expect(1).toBe(1)});' > good/a.test.js
python3 -c 'open(b"z\xe9.test.js","w").write("import {test} from \"bun:test\"; test(\"b\", ()=>{});")'
bun test --reporter=junit --reporter-outfile=out.xml; echo "rc=$?"; ls out.xml

Before:

bun test v1.4.0-canary.1 (da3851e57)

z�.test.js:
rc=1
ls: cannot access 'out.xml': No such file or directory

After:

z�.test.js:

# Unhandled error between tests
-------------------------------
error: Cannot find module '/tmp/t/z�.test.js' from ''
-------------------------------

good/a.test.js:
(pass) good

 1 pass
 1 fail
 1 error
Ran 2 tests across 2 files.
rc=1
out.xml

Same result for a Latin-1 directory name (caf\xe9/b.test.js), with --isolate, and with --parallel (the worker reports the file instead of exiting).

---

no test proof · iteration 0 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/cli/test/bun-test.test.ts

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 21 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: dad7e071-c64e-43af-9e25-20773a491157

📥 Commits

Reviewing files that changed from the base of the PR and between ada9163 and b4b5c0c.

📒 Files selected for processing (2)

• src/jsc/VirtualMachine.rs
• test/cli/test/bun-test.test.ts

---

_{Comment @coderabbitai help to get the list of available commands.}

[robobun]

^{Updated 6:05 PM PT - Aug 13th, 2026}

❌ @robobun, your commit b4b5c0c has some failures in Build #95050 (All Failures)

---

🧪   To try this PR locally:

bunx bun-pr 38258

That installs a local version of the PR into your bun-38258 executable, so you can run:

bun-38258 --bun

[robobun]

Status: reproduced on 1.4.0-canary (da3851e) with the repro in the description (a z\xe9.test.js next to a passing file: banner only, exit 1, no summary, no junit file; same for a caf\xe9/ directory and under --isolate; worker crashed: exit code 1 under --parallel). With this branch the file is reported as a failed file and the rest of the run completes. Test: test/cli/test/bun-test.test.ts, "a test file whose name is not valid UTF-8" (fails on the unfixed binary, passes here).

The automated review raised nothing to change. Waiting on CI and a maintainer review.

[claude]

I reviewed this PR and didn't find any bugs. The change is small and follows the established set_handled() → reject(Err(JsError::Thrown)) pattern, but since it touches JSC exception handling in VirtualMachine.rs, a human look would still be worthwhile.

What was reviewed:

• The None arm: JSC__JSPromise__create and setHandled are nothrow and don't declare a scope, so allocating/marking the promise while the resolve exception is still pending is safe; reject(.., Err(JsError::Thrown)) then takes that exception before calling into C++.
• set_handled() before reject() matches reject_as_handled (JSPromise.rs:427-429), so the rejection tracker won't double-report.
• ? on reject only propagates JsTerminated, which correctly still routes to handle_top_level_test_error_before_javascript_start.
• Tests: Linux-only gate is justified, pipes drained concurrently, using cleanup, both serial and --parallel covered.

Extended reasoning...

Overview

The PR changes one match site in VirtualMachine::reload_entry_point_for_test_runner (src/jsc/VirtualMachine.rs:4828-4846): when load_and_evaluate_module_ptr returns None (synchronous throw during entry-specifier resolve), instead of returning Err(JSError) — which the test runner's callers turn into Global::exit(1) via handle_top_level_test_error_before_javascript_start — it wraps the pending exception in a pre-handled rejected JSInternalPromise and returns that. The runner already handles a rejected load promise (bad-import path today), so the file is reported as failed and the run continues. Two Linux-only tests in test/cli/test/bun-test.test.ts cover serial and --parallel behavior with a Latin-1-byte filename and directory name.

Security risks

None. This is error-path routing inside the test runner; no untrusted input parsing, auth, or resource limits are touched.

Level of scrutiny

Medium-high. VirtualMachine.rs is core runtime code and the change sits at the JSC exception boundary. I traced the specifics that could go wrong here:

• JSInternalPromise is a Rust alias for JSPromise (src/jsc/lib.rs:206), so create calls JSC__JSPromise__create, which is a bare JSPromise::create(vm, structure) allocation with no ThrowScope — safe to call with an exception already pending.
• set_handled() is [[ZIG_EXPORT(nothrow)]] and just calls markAsHandled(); also safe with a pending exception.
• reject(.., Err(JsError::Thrown)) (JSPromise.rs:402-414) takes the pending exception via try_take_exception() before calling JSC__JSPromise__reject, so the C++ reject path sees a clean scope. If the taken exception is a termination it returns Err(JsTerminated), which ? propagates and the caller still hits Global::exit(1) — same as before for that case.
• Marking handled before rejecting mirrors reject_as_handled (JSPromise.rs:427-429) and prevents the rejection tracker from queueing a duplicate report; the runner reads the promise state directly.
• GC: the fresh promise is on the stack until stored in pending_internal_promise and ensure_still_alive'd immediately after; reject on a handler-less promise runs no reactions.

The author reports running the repro under BUN_JSC_validateExceptionChecks=1, which is the right check for the "allocate while exception pending" concern.

Other factors

The tests are well-constructed per REVIEW.md conventions: using tempDir, concurrent pipe drains, exit-code asserted last, describe.concurrent, and a comment justifying the skipIf(!isLinux) gate. They assert the positive contract (other file runs, summary printed, junit written, exit 1) and the negative one (RAN unloadable never appears, --parallel worker doesn't print worker crashed). The two other load_and_evaluate_module_ptr call sites at lines 2730 and 2753 are for bun run's single-entry path, where Err(JSError) → exit is still the intended behavior (and the synthetic-main path there makes this failure mode a rejected promise anyway, per the PR description).

Given that this is a targeted change in a critical file at the JSC exception boundary, I'm deferring rather than approving so a maintainer can confirm the exception-handling shape.

[robobun]

Heads-up: #38273 fixes the same bug (a test file whose entry specifier fails to resolve ending the run) one level down, in JSModuleLoader::load_and_evaluate_module_ptr / import_ptr, so that --preload and the other direct-load callers get the same treatment, and it also has to fix moduleLoaderResolve throwing an uninitialised error when a plugin's onResolve throws, which either version of this fix otherwise turns into a crash (repro in that PR's description: a preload plugin whose onResolve throws for a test file). Since the wrapper-level change covers the reload_entry_point_for_test_runner site this PR patches, the two should not both land; if #38273 is preferred this one can be closed, otherwise the moduleLoaderResolve part is needed here too.