Module loader: a removed module whose in-flight load fails no longer poisons later imports of it (WebKit bump for oven-sh/WebKit#474)

[robobun]

Stacked on #39674: its branch is the base, the same way oven-sh/WebKit#474 is stacked on oven-sh/WebKit#472. The diff is the pin moving from the #472 preview to the #474 preview, plus the tests. GitHub retargets this to main when #39674 lands.

Problem

• delete require.cache[path], mock.module() and plugin module() remove a module's registry entry, possibly while a load of it is in flight. If that load then failed, the loader stored the error under the path again: the next import() or require() rejected with the removed load's error and never loaded the file again. If a replacement load had registered the path first, the error landed in the replacement and a module that had loaded fine became unimportable, or the replacement import() never settled. Module loader: removing a registry entry out from under the import cache no longer crashes the next import() (WebKit bump for oven-sh/WebKit#472) #39674 covers the crash flavor of the same window.
• Cause, in JavaScriptCore: moduleLoadTopSettled, moduleLoadTopRejected and moduleLoadStoreError (JSMicrotask.cpp) stored the error through ensureRegistered(key), which creates a fresh entry once removeEntry() (src/jsc/bindings/ZigGlobalObject.cpp:792, BunPlugin.cpp:706) has dropped the one the load started from. JSModuleLoader::loadModule() answers every later load of the key from that entry's error.

Fix

• Two engine changes. Upstream 319474@main (on main since the 8c4fd56347 upgrade, Upgrade WebKit to 8c4fd56347 #40276) no longer registers a rejected fetch and only stores an error into an entry that exists, so the key is no longer registered again. JSModuleLoader: a top-level load stores its failure into the entry it loaded WebKit#474 adds the fork residual: a top-level load records the entry it loaded and stores its failure into that entry, so a removed load cannot store into a replacement's entry. No Bun source change.
• Correct because the error is the outcome of loading that one entry. While the entry is registered nothing changes. Once removed, the error stays with the detached entry, the path starts from nothing, and a replacement keeps its own state.
• The upstream change is also visible without a removal: a file whose load failed (a syntax error, a plugin onLoad that rejected) is loaded again by the next import() or require(), so a file fixed in the meantime loads. Before, every later load replayed the first error for the life of the process. main ships that already but has no test for it; the tests here pin it.
• Verified: eight new tests. Five removal shapes in test/cli/run/require-cache.test.ts, three reload shapes in test/js/bun/resolve/build-error.test.ts and test/js/bun/plugin/plugins.test.ts. On this PR's base (the Unable to set up devcontainer #472 preview, which has the upstream change) seven pass and a replacement load finished first fails; all eight pass on the Bug: URL path join is not correct #474 preview. All eight failed on the pin before the upgrade. Module loader: removing a registry entry out from under the import cache no longer crashes the next import() (WebKit bump for oven-sh/WebKit#472) #39674's tests pass on the new pin too. Notes below.

Background

• The registry maps a module key to a ModuleRegistryEntry: the load's promises, its record, and one error. removeEntry() is a Bun addition to the engine. Upstream never removes entries, so for upstream "the entry for this key" and "the entry this load started from" are the same thing.
• A top-level load runs as engine microtasks that carry a ModuleLoadingContext. The per-dependency loads always held their entry in it. The top-level ones held only the key and looked the entry up again when the load failed.

Notes

Fail-before: on the pin before the upgrade (the first #472 preview, and Bun 1.4.0 with the repro scripts) the four dynamic removal shapes print the removed load's error for the second import with onLoadCalls: 1, the require() shape hangs on the replacement import() until the test times out, the two build-error tests get BuildMessage for the fixed file, and the plugin test gets the first error back without a second onLoad call. On the current base (bun bd --webkit-version=autobuild-preview-pr-472-cfccea9d), which carries upstream 319474@main, only a replacement load finished first fails: getRegisteredMayBeNull() finds the replacement's entry and the removed load's error lands there. That is the shape oven-sh/WebKit#474 fixes. The engine-side split was first measured by recompiling the three affected unified-source bundles into the first #472 preview library, with and without the upstream change; the pass-after has been repeated on every published #474 preview since.

Suites run on the new pin: test/js/bun/resolve/, test/js/bun/plugin/, test/js/bun/test/mock/, test/js/node/module/, test/cli/run/, test/cli/test/, and the test of #33149. The failures left on this debug-asan container also fail on the unmodified engine: the seven require.cache leak tests (#39473 has the numbers), load the same empty JS file 2000 times (its sibling takes 13.6 s on both engines), --parallel lazily scales workers based on file duration, should resolve self-imports by name (a 5 s timeout around six debug-build spawns), the FUSE tests and the NO_ORPHANS uid/gid test. plugins.test.ts does not survive --rerun-each on the base either (its recursion plugin overflows on re-registration), so the new test there was checked by a normal run; it uses a path of its own per run regardless.

Open PRs this touches. #33419 works around the cached plugin failure on the Bun side; the upstream policy now on main does what its import() cases ask for (the new plugins.test.ts test is that shape). #39204 (oven-sh/WebKit#451) is about which error object a later load of a failed file gets: for a later top-level import() or require() the answer is now a fresh load, while the per-dependency replay it also covers is unchanged. #33149 adds a test for oven-sh/WebKit#262; it passes here, but its comment describes a mechanism that is gone (nothing is registered for the rejected fetch any more, so the static importer fetches again). #38072 and #38645 are about require() of a module that is evicted during, or threw during, its own evaluation; they are different bugs and are not affected.

If oven-sh/WebKit#472 and #474 merge together, the reverse packaging works too: this PR moves to the merged sha and #39674 becomes tests only.

Note

WEBKIT_VERSION points at the preview tag autobuild-preview-pr-474-56b8426b while oven-sh/WebKit#474 is open. It has to move to the merged main sha before this lands, after #39674's pin does the same.

---

no test proof · iteration 9 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/run/require-cache.test.ts

[coderabbitai]

Warning

Review limit reached

Your included review limit has been reached.

You’re in a promotional period — use the checkbox below to run this review for free:

• Run review for free

_{On-demand reviews are free for the next 31 days. After that, they cost $0.25 per reviewed file.}

How can I continue?

Run this review now using the option above, or comment @coderabbitai review --use-credits.

You can also wait for the limit to reset (next review available in 15 minutes), then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: ccb5d32b-cff0-4582-82fa-9fb4e1b6a7e2

📥 Commits

Reviewing files that changed from the base of the PR and between 6e906e4 and b3bddca.

📒 Files selected for processing (5)

• scripts/build/deps/webkit.ts
• test/cli/run/require-cache.test.ts
• test/js/bun/plugin/plugins.test.ts
• test/js/bun/resolve/build-error.test.ts
• test/js/bun/test/mock/mock-module.test.ts

---

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

[robobun]

Status: engine fix in oven-sh/WebKit#474 (preview autobuild-preview-pr-474-56b8426b, pinned here), stacked on oven-sh/WebKit#472. This PR is stacked on #39674 the same way, so its diff is the pin move plus the tests for #474.

Reproduced with the five removal fixtures in test/cli/run/require-cache.test.ts and the three reload tests in test/js/bun/resolve/build-error.test.ts and test/js/bun/plugin/plugins.test.ts. On the pin before the 8c4fd56347 upgrade (and on Bun 1.4.0) all eight failed: the removal fixtures printed the removed load's error for the second import, the require() one hung on the replacement import(), and the reload tests got the first error back. Since the upgrade brought upstream 319474@main, this PR's base passes seven of them; a replacement load finished first still fails there and passes on the #474 preview, as do the other seven, #39674's tests and test/js/bun/resolve/, test/js/bun/plugin/, test/js/bun/test/mock/, test/js/node/module/, test/cli/run/ and test/cli/test/ (the failures left on this debug-asan container fail the same way on the unmodified engine, listed in the description).

CI on 54f5bd2b (Buildkite 107738): the new tests pass on every lane. The one red test is test/js/web/url/url.test.ts on Darwin x64, a failure main has as well (already with triage) and unrelated to this change. require-cache.test.ts itself only flaked on its pre-existing leak fixtures on x64-asan (passed on retry; #39473 covers those fixtures). The rest is retry-passing flake.

Next step is not in this PR: once oven-sh/WebKit#472 and #474 merge and #39674 moves its pin, WEBKIT_VERSION here moves to the main sha and CI runs again.

[robobun]

The review note on scripts/build/deps/webkit.ts:6 is the expected state for this PR: the preview pin is a placeholder until oven-sh/WebKit#472 and oven-sh/WebKit#474 merge. I will move WEBKIT_VERSION to the merged main sha then, after #39674 does the same for #472. Nothing else is pending from the bot reviews.

[robobun]

^{Updated 8:39 AM PT - Aug 28th, 2026}

❌ @robobun, your commit 54f5bd2 has 1 failures in Build #107738 (All Failures):

• test/js/web/url/url.test.ts - code 1: TypeError: Invalid URL on 🍎 any x64

---

🧪   To try this PR locally:

bunx bun-pr 39711

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

bun-39711 --bun

[robobun]

Restacked: this PR is now based on #39674 (the Bun side of oven-sh/WebKit#472), mirroring oven-sh/WebKit#474 on #472, so the diff is the pin move plus the tests. The failure-shape tests now extend #39674's fixture instead of duplicating it, and three tests were added for the part of the engine change that is visible without delete require.cache: a file or plugin module whose load failed is loaded again by the next import() or require() (build-error.test.ts, plugins.test.ts). All eight fail on the base pin and pass on the new one. Description updated.

[robobun]

The conflict came from the base moving: #39674 was rebased onto current main (its pin is now the oven-sh/WebKit#472 preview rebased onto b7f217b4), and oven-sh/WebKit#472 itself was rebased, so oven-sh/WebKit#474 is rebased on top of it (same two commits, files unchanged) and this branch is rebased onto the new #39674 head. The only conflict was the WEBKIT_VERSION line, resolved to the preview tag of the rebased #474. The push follows as soon as that preview build is published, so that CI runs against it.

[robobun]

Rebased and pushed. oven-sh/WebKit#474 now sits on the rebased oven-sh/WebKit#472 (its two commits apply unchanged, all five engine files are byte-identical to the reviewed version), this branch sits on the rebased #39674, and the pin is the new #474 preview autobuild-preview-pr-474-4875ff69. The only conflict was the WEBKIT_VERSION line. Re-checked on this base: the eight new tests fail against the base pin (autobuild-preview-pr-472-86b6c4df) and pass against the new one, as do #39674's tests. The PR is mergeable again.

[robobun]

Re-checked after a second conflict notice: nothing to resolve. The base (#39674's branch) is still at dd6015e7c6, the commit this branch was rebased onto, GitHub reports the PR as mergeable against it, and a test merge of this branch into current main (git merge-tree) is clean as well, since no main commit since the merge base touches the five files involved. The notice predates the rebase pushed as 0ff6fbc816. CI for that head is running (Buildkite build 102338).

[robobun]

Rebased again: #39674 (the base) was rebased onto current main, so this branch was rebased onto its new head (c45fe342c9). The three commits applied without conflicts, the diff against the base is unchanged (pin plus the same tests), and the pin still points at the current oven-sh/WebKit#474 preview since neither engine PR moved. The touched test files pass on the rebased tree; the PR is mergeable again.

[robobun]

Rebased onto the new head of #39674 (f2b3d945e2, itself rebased onto current main). The three commits applied without conflicts; the diff against the base is unchanged (pin plus the same eight tests) and the pin still points at the current oven-sh/WebKit#474 preview, since neither engine PR has moved. The plugin test file picked up main's new object-loader test (#39998) next to the test added here; both files pass on the rebased tree. Mergeable again.

[robobun]

Same cause as before: the base moved. oven-sh/WebKit#472 was rebased onto the current fork main (aea1f010), #39674 followed with its pin, so oven-sh/WebKit#474 is re-applied on the new #472 head (same two commits, same diff) and this branch is rebased onto the new #39674 head (c763233063). Only the WEBKIT_VERSION line conflicted; it is resolved to the preview tag of the rebased #474. The push follows once that preview build is published, so that CI runs against it.

[robobun]

Pushed. oven-sh/WebKit#474 is re-applied on the rebased #472 (its diff is unchanged), its preview autobuild-preview-pr-474-0bddaf4d is published and pinned here, and this branch sits on the current #39674 head. The only conflict was the WEBKIT_VERSION line; main's new plugin test (#40069) merged next to the one added here without conflict. The touched test files pass on the rebased tree. Mergeable again.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Bot review of a5463c37 found nothing, no review threads are open on either PR. On its CI run (Buildkite 106464, still finishing) the new tests pass on every lane so far; the red file is bun-build-api.test.ts, a main-side failure unrelated to this change and handed to triage.

[robobun]

The base moved again (oven-sh/WebKit#472 rebased onto fork main 7259739917 plus the Windows arm64 CI fix, #39674 followed). oven-sh/WebKit#474 is re-applied on the new #472 head with an identical diff, and this branch is rebased onto the new #39674 head (6e4ce9bfc9); only the WEBKIT_VERSION line conflicted and it is resolved to the preview tag of the rebased #474. The push follows once that preview build is published.

[robobun]

Pushed. oven-sh/WebKit#474 is re-applied on the rebased #472 (identical diff), its preview autobuild-preview-pr-474-6a94e344 is published and pinned here, and this branch sits on the current #39674 head. Only the WEBKIT_VERSION line conflicted; main's new build-error.test.ts tests (#40568) merged next to the ones added here without conflict. The touched test files pass on the rebased tree. Mergeable again.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

The base moved again (oven-sh/WebKit#472 rebased onto fork main 0bb01ed5, #39674 followed). oven-sh/WebKit#474 is re-applied on the new #472 head with an identical diff, and this branch is rebased onto the new #39674 head (86e448efe6); only the WEBKIT_VERSION line conflicted and it is resolved to the preview tag of the rebased #474. The push follows once that preview build is published.

[robobun]

Pushed. oven-sh/WebKit#474 is re-applied on the rebased #472 (identical diff), its preview autobuild-preview-pr-474-2cd2d81b is published and pinned here, and this branch sits on the current #39674 head. Only the WEBKIT_VERSION line conflicted; main's new build-error.test.ts test (#40594) merged next to the ones added here without conflict. The touched test files pass on the rebased tree. Mergeable again.

[robobun]

Nothing to rebase on this side yet: this PR is mergeable against its base (#39674 at 86e448efe6, unchanged since the last push). The conflict is the stack's against main, whose WEBKIT_VERSION moved again (f5deafe090), and it is the base PR's WEBKIT_VERSION line that conflicts with it. Once #39674 rebases (it has done so after every pin move), oven-sh/WebKit#474 and this branch follow as before. The fork-main commits behind that pin touch moduleLoadTopSettled (a provided-module path around provideFetch()), so the next re-application of oven-sh/WebKit#474 will be checked against them rather than assumed identical.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Bot review of 63fb6696 found nothing, no review threads are open on either PR. Its CI run (Buildkite 107181, still finishing) has the new tests green on every lane so far; the red files are test/cli/install/ tests whose git dependencies got HTTP 504 from api.github.com during the run, unrelated to this change and handed to triage.

[robobun]

The base moved again (#39674 now on the oven-sh/WebKit#472 preview at f7cb3cb4; #472 itself has since moved once more onto the fork main with the 6b879687ee upgrade). oven-sh/WebKit#474 is re-applied on the current #472 head (6a17c6a216). This time the residual needed one adaptation: fork main now skips provideFetch() in moduleLoadTopSettled for an entry registered through provideModule(), so the line that records the entry moved after that conditional block and covers both cases. The rest of the diff is unchanged. This branch is rebased onto the new #39674 head (b067a2117f); only the WEBKIT_VERSION line conflicted. The push follows once the preview build is published.

[robobun]

Holding the push. oven-sh/WebKit#474 now sits on the #472 head that includes the 6b879687ee upgrade, and its preview (autobuild-preview-pr-474-0e817363) is published, but this PR's base (#39674 at b067a2117f) predates main's adaptation to that engine (#40681: moduleLoaderFetch gained a referrer argument), so a bun tree built from this base against the new preview does not compile (BakeGlobalObject.cpp:223). Pinning an older preview is not an option either (the previous #474 preview build was superseded before it published). The branch is rebased and pinned locally; it gets pushed as soon as #39674 has rebased onto current main, which its own conflict with main calls for.

[robobun]

The base now includes main's adaptation to the upgraded engine (#40681), so the hold is over. oven-sh/WebKit#474 is re-applied on the current #472 head (eaacac2867, on fork main ceb9f90f) with an identical diff, and this branch is rebased onto the new #39674 head (e6fa1776e8); only the WEBKIT_VERSION line conflicted and it is resolved to the preview tag of the rebased #474. The push follows once that preview build is published.

[robobun]

Pushed. oven-sh/WebKit#474 sits on the current #472 head (identical diff), its preview autobuild-preview-pr-474-56b8426b is published and pinned here, and this branch sits on the current #39674 head, which carries main's adaptation to the upgraded engine. Only the WEBKIT_VERSION line conflicted. The tree builds against the new preview and the touched test files pass. Mergeable again.

[robobun]

#39674 moved once more while the previous push was being verified (it now pins the #472 preview eaacac28, the same #472 head oven-sh/WebKit#474 already sits on), so this branch is rebased onto its new head (768ee79a93) with the same pin. Only the WEBKIT_VERSION line conflicted. Builds and the touched test files pass. Mergeable again.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[robobun]

Bot review of 54f5bd2b found nothing, no review threads are open on either PR. On its CI run (Buildkite 107738, 177 of 181 jobs done) the new tests pass on every lane; the one red test is url.test.ts on Darwin x64, the pre-existing main failure already with triage.