Test: a failed dynamic import must not strand later importers of the same file - #33149
Test: a failed dynamic import must not strand later importers of the same file#33149robobun wants to merge 1 commit into
Conversation
|
Updated 1:50 AM PT - Jul 1st, 2026
❌ @robobun, your commit 4922727 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 33149That installs a local version of the PR into your bun-33149 --bun |
|
Reproduced on Bun 1.4.0: the second |
|
Warning Review limit reached
Next review available in: 28 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR updates the WebKit version pin, changes WebKit source syncing to fetch tags before checking out the target commit, and adds regression coverage for failed top-level dynamic imports with non-Error rejections. ChangesWebKit dependency and source sync
Module loader regression coverage
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/js/bun/resolve/concurrent-dynamic-import.test.ts`:
- Around line 63-69: Add the missing stderr assertion in the concurrent dynamic
import test: `stderr` is already captured in the `Promise.all` call but never
verified, so update the `concurrent-dynamic-import.test.ts` case to assert
`stderr` is empty using the same pattern as the sibling test. Place the
unconditional `expect(stderr).toBe("")` in the test block before the stdout/exit
code assertions, and keep the existing checks in the `proc`, `stdout`, and
`exitCode` flow unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6c5bfb7c-66a6-4f3e-862e-41176c4acf2d
📒 Files selected for processing (2)
scripts/build/deps/webkit.tstest/js/bun/resolve/concurrent-dynamic-import.test.ts
|
Found 3 issues this PR may fix:
🤖 Generated with Claude Code |
|
I checked the three candidate issues above against both released Bun and a debug build with this fix applied; none of them are fixed by this PR, so the
The first import in this PR's reproduction does reject today; what hangs forever is the next importer of the same file. |
|
Every failing lane in the last CI run is the same single cause, with nothing else failing: oven-sh/WebKit#262's preview build creates the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/sync-webkit-source.ts`:
- Around line 23-25: The current fetch in sync-webkit-source.ts is pulling every
tag from the WebKit remote, which is unnecessary for resolving WEBKIT_VERSION.
Update the fetch logic near the git fetch call to request only the specific
WEBKIT_VERSION tag instead of using the global tags fetch, and keep the existing
comment aligned with that behavior. Make sure the change is localized to the
sync flow that prepares the WebKit source and preserves compatibility with the
git versions used in CI/dev.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5f988c82-f537-4365-a04b-fc038f9a04c6
📒 Files selected for processing (2)
scripts/sync-webkit-source.tstest/js/bun/resolve/concurrent-dynamic-import.test.ts
|
The WebKit preview artifact this PR pins is published now: This PR needs one CI re-run now that the artifact exists. I cannot trigger it without another commit (my Buildkite token is read-scoped, and I already pushed one empty To merge: re-point Unrelated to this PR: |
Bumps `vendor/WebKit` to upstream `WebKit/WebKit@d81bcc3d833c` (2026-06-30). 887 upstream commits since the last Bun sync point (`b5ba38a21e17`, 2026-06-16); 117 touch `Source/JavaScriptCore`, 54 touch `Source/WTF`, 10 touch `Source/bmalloc`. The fork-side work is split across two merge PRs: - oven-sh/WebKit#261: `b5ba38a21e17` to `ce481c4cfedb`, merged as `0d9ee56ba4f6`. - oven-sh/WebKit#263: `ce481c4cfedb` to `d81bcc3d833c`, based on the fork's current `main`, so it also carries oven-sh/WebKit#257 and oven-sh/WebKit#262, which landed on the fork between the two merges and which fix real Bun bugs (see below). > [!NOTE] > oven-sh/WebKit#263 has landed on `main` as `c9ad5813fd23`, and `WEBKIT_VERSION` now points at that commit's release, `autobuild-c9ad5813fd23bd8b98b0738abc3d037ec716aa92`. Nothing else is pending on the WebKit side. It was landed as a squash rather than a merge commit, but `c9ad5813fd23`'s git tree is byte-identical to the preview head `f652829d78`'s tree (verified: both are `470eee60f5`), so the final artifacts are built from exactly the source all of the verification below ran against. The squash does drop the upstream parent link, so whoever does the next fork upgrade will need a merge-base fixup like the one that followed oven-sh/WebKit#251. Fixes #32793 ## Bun changes required by this upgrade ### From `b5ba38a2 -> ce481c4c`: three builtin rewrites Upstream removed three bytecode intrinsics that Bun's built-in JS modules use (https://bugs.webkit.org/show_bug.cgi?id=318076 and https://bugs.webkit.org/show_bug.cgi?id=318028). Bumping the version without these rewrites aborts on every `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` and on `new console.Console(stream).table(...)` (the global `console.table` is native and was unaffected). - `src/js/builtins/JSBufferConstructor.ts`: `$tryGetByIdWithWellKnownSymbol(value, "toPrimitive")` becomes an ordinary `value[Symbol.toPrimitive]` read, which is exactly what Node's `lib/buffer.js` does. The removed intrinsic never invoked accessors, so this is also a Node-compat improvement: a getter-defined `Symbol.toPrimitive` is now honored. `test/js/node/buffer-from-symbol-to-primitive.test.ts` covers it and fails on the released Bun. - `src/js/builtins/ConsoleObject.ts`: `$isMapIterator` / `$isSetIterator` become `require("node:util/types").isMapIterator / isSetIterator`, which is exactly what Node's `internal/console/constructor.js` does. `test/js/node/console/console-table-iterators.test.ts` is a snapshot test that passes on both the released Bun and this branch, proving the rewrite is behavior-preserving (there was no prior coverage for `Console#table` on a Map/Set iterator). - `src/js/builtins.d.ts`: the declarations for all ten removed intrinsics are deleted so nothing reaches for them again. ### From oven-sh/WebKit#257: `jest` fake-time code must use the new `NaN` sentinel oven-sh/WebKit#257 changed the "no override is active" sentinel on `JSGlobalObject::overridenDateNow` from `-1` to `NaN` (see `JSGlobalObject::jsDateNow()`). Bun's reset paths still wrote `-1`, which the new semantics read as a *live* override frozen at `-1` ms, so `jest.useRealTimers()` and `setSystemTime()` with no argument left `Date.now()` stuck at `1969-12-31T23:59:59.999Z`. `test/js/bun/test/test-timers.test.ts` ("we can go back in time") caught this against a Bun built from the #263 artifact. - `src/runtime/test_runner/timers/FakeTimers.rs`: `CurrentTime::clear` (the `jest.useRealTimers()` path) writes `f64::NAN` instead of `-1.0`. - `src/jsc/bindings/JSMockFunction.cpp`, `JSMock__jsSetSystemTime`: the reset value becomes `PNaN`. The old `>= 0` / `std::isnormal()` gating existed only to serve the `-1` sentinel and silently dropped every pre-epoch and zero override (a bug the #257 review called out); with the `NaN` sentinel every real timestamp is a valid override, so `setSystemTime(-1)` and `setSystemTime(new Date("1960-01-01"))` now work. A new test in `test/js/bun/test/test-timers.test.ts` covers the pre-epoch, zero, `-1`, and no-argument-reset cases and fails on Bun 1.4.0. - Deleted `JSMock__jsUseRealTimers` from `JSMockFunction.cpp`: nothing registers it (`jest.useRealTimers` is the Rust `use_real_timers` in `FakeTimers.rs`), and it was the third place that would have needed the sentinel change. `ce481c4c -> d81bcc3d` itself (the 84 upstream commits) requires nothing: no bytecode intrinsics changed, `JSType.h` did not change, and the only WebCore code-generator change is to a helper Bun does not use. ## Bun bugs fixed by the fork-side commits this carries - oven-sh/WebKit#257: `Date.now()` is tagged `DateNowIntrinsic`, so a hot call site is inlined by the DFG/FTL as a `DateNow` node, and `operationDateNow` returned the wall clock instead of reading `JSGlobalObject::overridenDateNow`. A `setSystemTime()` override therefore silently wore off as soon as a `Date.now()` call site tiered up (#32793). `test/regression/issue/32793.test.ts` spawns the loop from that issue and asserts the override never diverges; on the released Bun it diverges at iteration 99, and it also fails against a build of this branch's previous pin (`0d9ee56ba4f6`, which predates #257), so the test isolates exactly this commit. - oven-sh/WebKit#262: a top-level `import()` whose embedder fetch rejects with a value that is not an `ErrorInstance` (a single-message transpile failure rejects with a `BuildMessage`) poisoned its module registry entry, and every later importer of the same file hung forever. The Bun-side regression test lives in #33149, which also pins a WebKit preview; once this lands, #33149 reduces to its test plus its `scripts/sync-webkit-source.ts` improvement. ## Why the regression tests are spawned fixtures Without the three builtin rewrites, a Bun built against this WebKit aborts on the first `Buffer.from()` call: ``` ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(value, "toPrimitive"); ``` `Buffer.from` is fundamental enough that this takes down the `bun test` runner itself, before any test file finishes loading and before the JUnit reporter can write anything. So `buffer-from-symbol-to-primitive.test.ts` and `console-table-iterators.test.ts` live in two small files of their own and spawn a child Bun instead of calling the affected APIs in process: a child abort becomes an ordinary `exitCode`/`stdout` assertion failure, whereas an in-process call takes the parent test runner down with it and reports nothing. Their fail-before is "this branch minus the `src/js/` rewrites", which is a deterministic SIGABRT: ```sh git checkout $(git merge-base HEAD origin/main) -- src/js/ # keep scripts/ (the WEBKIT_VERSION bump) bun bd test test/js/node/buffer-from-symbol-to-primitive.test.ts # => ASSERTION FAILED: Private symbol not found: tryGetByIdWithWellKnownSymbol(...) (SIGABRT) git checkout HEAD -- src/js/ ``` `test/js/bun/test/test-timers.test.ts` is the equivalent fail-before for the sentinel change: with the `src/` diff reverted but the new WebKit kept, both "we can go back in time" and the new `setSystemTime` test fail. `test/regression/issue/32793.test.ts` is different from all of the above: its fix lives entirely in the vendored engine, selected by `scripts/build/deps/webkit.ts` rather than by anything under `src/`. So its before/after is a function of which WebKit artifact is linked, not of the `src/` diff: it fails on the released Bun (1.4.0), fails against this branch built with the previous pin `0d9ee56ba4f6`, and passes against this branch built with the #263 artifact. ## Verification All of the following were run against a debug Bun built from this branch and the `autobuild-preview-pr-263-f652829d` artifact. - The four test files this PR adds or modifies (`test/js/bun/test/test-timers.test.ts`, `test/regression/issue/32793.test.ts`, `test/js/node/buffer-from-symbol-to-primitive.test.ts`, `test/js/node/console/console-table-iterators.test.ts`): 5 tests, all pass. Every one fails in the right "before" state as described above. - Every test file in the repo that exercises `setSystemTime` / `useFakeTimers` / `useRealTimers` / `advanceTimers*` (the blast radius of the sentinel change): `test/js/bun/test/fake-timers/fake-timers.test.ts`, `test/js/bun/test/fake-timers/sinonjs/fake-timers.test.ts`, `test/js/bun/cron/in-process-cron.test.ts`, `test/regression/issue/25869.test.ts`, `test/regression/issue/26284.test.ts`, and the five `test/js/third_party/jsonwebtoken/*.test.js` suites. 249 pass, 1 todo, 0 fail. - One more file turned out to be in that blast radius: `test/js/bun/test/fake-timers/sinonjs/issue-207.test.ts`. The sentinel change makes pre-epoch and negative clock values work, so its three long-`.failing` tests ("should floor negative now values", "should floor negative start times", "should handle ticks on the negative side of the Epoch") started passing, which Bun's runner reports as a failure until the stale marker is removed. `d2309c92c2` removes them; all ten tests in the file pass against this branch. - `test/js/node/buffer.test.js`: 539 pass, 1 pre-existing skip, 0 fail. `test/js/node/console/console.test.ts`: 7 pass. Neither file is modified by this PR. - oven-sh/WebKit#263's preview build produced all 43 platform artifacts with no job failures. ## Upstream changelog <details> <summary><code>ce481c4c</code> -> <code>d81bcc3d</code> (the new hop in this revision)</summary> Memory safety and security: - `TypedArray.from()` out-of-bounds read via resizable `ArrayBuffer` resize/transfer inside the `mapFn` callback - Stale structure bit in `SlowPutArrayStorage` - Do not cache property absence on dictionary structures - Missing `codeBlock->m_lock` in `repatchGetBySlowPathCall` - Insert a write barrier for `MultiPutByOffset` when it can reallocate storage - Unconditionally keep `OMGOSREntryCallee` alive while updating its callsites - Disallow defining private names on WasmGC objects - Size limit on Yarr-generated code Correctness: - DFG spread did not account for `cellButterflyOnlyAtomStringsStructure` - YarrJIT `negativeOffsetIndexedAddress` discarded the adjusted base register - `DFGArgumentsEliminationPhase::removeViaKill` did not reset the node scan index between `InlineCallFrame`s Other: - An `ArrayStorage`-specific `GetByVal` DFG operation - ES2022 private method support in the Web Inspector - WTF: `TinyLRUCache::findIfCached` / `insert` </details> <details> <summary>Security / correctness fixes (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - Use-after-free after Wasm memory grow via a stale pointer folded by DFGConstantFoldingPhase - Keep `JSWebAssemblyMemory` alive from wasm-originated `JSArrayBuffer`s (UAF) - Use-after-free of `StreamingCompiler::m_ticket` - `BBQCallee` kept alive between callsite collection and repatch (upstreams and extends oven-sh/WebKit#210) - `Array.from(arguments)` fast path bails out past `MAX_STORAGE_VECTOR_LENGTH` - `JSArray::fastFlat` bounds-checked against the maximum legal length - `Array#flat` fast path was dropping prototype-backed holes in nested arrays - `Array.from(map.keys())` fast path was ignoring `Symbol.iterator` overrides - `RegExp#@@split` crash on a huge flags string - `LiteralParser` takes the slow path if the original structure changes - Missing exit-profile bails on some DFG nodes - `IteratorClose` in `forEachInIterable` uses the Map/Set's own realm's iterator structure - YARR: JIT backreference with a non-BMP character was skipping the next match start position - Wasm Memory64: fix parsing of data segment init expressions - Track `customSlotBase` for `CustomAccessorGetter` / `CustomAccessorSetter` - Set `MayStoreHole` appropriately; `"entries"` ArrayIterator emits `ExitOK` before `NewArray` - `poisonedDeadOSRExitValue` made crashier and on by default in debug builds </details> <details> <summary>Language features and spec alignment (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - BigInt Math proposal implemented - Temporal: spec-alignment passes across `PlainDate`, `PlainTime`, `Duration`, `Instant`, plus a unified `ParseISODateTime` - `Intl.PluralRules` reflects `compactDisplay` in its skeleton and `resolvedOptions()` </details> <details> <summary>Performance (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> JavaScript-visible fast paths: - `JSON.stringify` space/indentation argument handled by `FastStringifier` - Fast path for the `RegExp#flags` getter; cached named-capture `groups` Structure on `RegExp` - Map/Set iterator `next` and `%RegExpStringIteratorPrototype%.next` moved to C++ - int32 fast path in `parseInt` for short decimal strings - `NumericStrings` int cache lookup inlined into DFG/FTL `ToString(Int32)` - `Object#isPrototypeOf` uses the `InstanceOf` DFG node - `Object.freeze` / `seal` / `preventExtensions` no longer allocate `ArrayStorage` for `NonArray` objects - Map/Set `set` + `delete` churn no longer expands then immediately shrinks - Avoided an intermediate `StringImpl` allocation feeding `StringBuilder` into `makeString` - Wasm `intoCharCodeArray` bulk-copies 8-bit strings; unnecessary watchpoints avoided - `DFG::GetByVal` widens to `SpecBytecodeTop` instead of forcing an OSR exit on an empty value profile YARR (the RegExp engine): - `latin1Table` for efficient Char8 filtering, reused for Char16 and the interpreter - `StringList` optimization when captures are unused; atom fast path enabled for unicode patterns JIT compile time (a large batch aimed at DFG/FTL/B3/Air throughput): - Semi-NCA algorithm for `WTF::Dominators`; reverse post-order in `WTF::Liveness` and B3 `ReduceStrength`; `SparseBitVector` in `DFGLivenessAnalysis` - Unified `SSACalculator`; dominance frontiers computed once per graph - `AirFixObviousSpills`, `AirEliminateDeadCode`, `Air::RegLiveness`, and the greedy register allocator all got cheaper - B3 CSE reimplemented without `B3::Variable`; `B3::Value::effects` uses a constant table - `ReduceStrength` avoids fixed-point iteration; `InferSwitch` disabled in B3 for Wasm - Fixup-inserted `RegExp` primordial `TryGetById` chains replaced with a single `CheckStructure` - Parser: create save points only when actually needed - Baseline/LLInt profile `op_unsigned` overflow and report int32 overflow to the DFG </details> <details> <summary>WTF, bmalloc, and the build system (<code>b5ba38a2</code> -> <code>ce481c4c</code>)</summary> - `SaturatedArithmetic.h` and its APIs renamed to `Saturating` (Bun does not use these directly) - Checked arithmetic helpers now require two or more arguments - `OrderedHashTable` load-factor policy aligned with `HashTable` - `JSON::Value::dump` no longer corrupts large string values via `PrintStream` truncation - Structured clone: the remaining terminal JS value tags moved from WebCore into JSC (`CloneSerializerBase.h` / `StructuredCloneTags.h`). Bun keeps its own fork of `SerializedScriptValue.cpp`, so this is additive. - Workaround for a GCC 14+ false `-Wuninitialized` in `Variant`; `[[msvc::no_unique_address]]` used on MSVC - WTF / libpas exception codes changed to `0xbb08` - libpas: per-heap tagging policy rework; `pas_deallocate` split into inline-only and casual variants; PGO applied to WTF/bmalloc - CMake: prefix headers expanded, headermap generation reworked, stale staged headers pruned, and the Darwin platform block in `BPlatform.h` / `pas_platform.h` no longer excludes CMake builds (restored in the fork, see oven-sh/WebKit#261; without it the Linux-to-macOS cross lane fails to link `gigacageEnabledForProcess`) </details>
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
There was a problem hiding this comment.
Remove the code changes, rebase, keep the test. We already bumped WegbKit.
…same file Regression test for the hang fixed by oven-sh/WebKit#262, which is in the current WEBKIT_VERSION. // bad.ts contains "import {" (one parser error -> a BuildMessage) await import("./bad.ts").catch(() => {}); await import("./other.ts"); // other.ts: import "./bad.ts"; never settled A top-level dynamic import whose transpile fails with a single parser error rejects with a BuildMessage, which is not an ErrorInstance. JSC's moduleLoadTopSettled gated its fetch-error classification on dynamicDowncast<ErrorInstance>, so nothing was recorded and moduleLoadTopRejected fell back to setEvaluationError. That left the module registry entry with no fetch, module, or load promise; the next importer of the same file that goes through hostLoadImportedModule (a static import, or a dynamic import inside another module's graph load) then parked forever on a freshly created fetchPromise nothing settles.
robobun
changed the title
c35fde1 to
4922727
Compare
|
Done. The branch is now Verified after the rebase:
|
There was a problem hiding this comment.
LGTM — test-only regression coverage for a fix already on main; all prior review threads addressed and the scaffolding changes were dropped in the rebase.
Extended reasoning...
Overview
This PR now touches exactly one file, test/js/bun/resolve/concurrent-dynamic-import.test.ts. It converts the existing test to test.concurrent and adds a second test.concurrent case verifying that a top-level dynamic import() which rejects with a non-ErrorInstance value (a BuildMessage from a single parser error) does not leave later importers of the same file hung on a never-settled promise. The actual fix lives in oven-sh/WebKit#262, which is already in main's WEBKIT_VERSION — this PR just adds the regression test. Earlier revisions carried a temporary WEBKIT_VERSION preview-tag pin and sync-webkit-source.ts tweaks; those were removed in the final rebase, leaving a pure test addition.
Security risks
None. No production code, no dependency changes, no network access, no user-controlled input handling. The test spawns bunExe() in an isolated tempDir with bunEnv.
Level of scrutiny
Low. Test-only, ~40 lines added to an existing test file for the same feature area. The test follows house conventions precisely: using tempDir, await using proc, concurrent pipe drain via Promise.all, and a combined { stdout, stderr, exitCode, signalCode } .toEqual assertion so failures show everything at once. The author verified fail-before (USE_SYSTEM_BUN=1 → 5000ms timeout, the hang) and pass-after (bun bd test against main's WebKit → 2 pass), which is the required bar per CLAUDE.md.
Other factors
All four prior review threads (one CodeRabbit on the missing stderr assertion, two of mine on sync-webkit-source and one test.concurrent nit) are resolved — the stderr and concurrency feedback is reflected in the final diff, and the sync-webkit-source/webkit.ts files are no longer part of the change so those threads are moot. The bug-hunting pass found nothing. The failure mode on regression is a test timeout (child never exits), which is a clear, non-flaky signal given await using will kill the child on scope exit.
|
CI on build 67488 is red, but the new test is not in any of the failures; it ran and passed on every shard that includes it (278 shards passed). The diff is one test file and the binary-size annotation confirms it: +0.0 KB on every platform. The failures, none of which this change can affect:
I already pushed one empty retrigger commit earlier in this PR's life (before the rebase that removed it), so I am not pushing another. The diff is ready; a re-run should only re-roll those lanes. |
2 participants
Adds a regression test for a module-loader hang whose fix, oven-sh/WebKit#262, is already in the
WEBKIT_VERSIONonmain. No code changes.Reproduction (Bun <= 1.4.0)
The second
import()neither resolves nor rejects and the process never exits (timeout 10 bun run.mjsexits 124). Any module that imports such a file statically, or dynamically inside another module's graph load, is stranded, not just the direct importer. A test runner thatimport()s each file and catches errors hits this routinely.Cause
A top-level dynamic
import()whose transpile fails with a single parser error rejects with aBuildMessage, which is not anErrorInstance. JSC'smoduleLoadTopSettledgated its entire fetch-vs-evaluation error classification ondynamicDowncast<ErrorInstance>, so the rejection was never recorded as a fetch failure andmoduleLoadTopRejectedrecorded it as an evaluation error instead. That left the module registry entry atEvaluationFailedwith no fetch, module, or load promise. The nexthostLoadImportedModuleof the same key found a non-Newentry with nofetchError()and noloadPromise(), skipped the re-fetch because the status was notNew, and parked on a freshly created pendingfetchPromisethat nothing ever settles.oven-sh/WebKit#262 records a non-Error fetch rejection with
setFetchError(matching what the sibling nested-import path,moduleRegistryFetchSettled, already does), so later importers takehostLoadImportedModule'sfetchError()short circuit and reject instead of hanging.That misclassification is also what trips
ASSERTION FAILED: m_status == Status::FetchinginModuleRegistryEntry::fetchCompleteon an asserts build when a failing and a succeeding loader of one path race (for exampleimport(p, { with: { type: "toml" } })of a file with a TOML syntax error concurrently withimport(p, { with: { type: "text" } })):setEvaluationErrorleaves the entry in a statefetchCompletedoes not guard, whilesetFetchErrorlands it atFetchFailed, whichfetchCompletealready early-returns on. Independent of the import-attribute module map key in #32999 / oven-sh/WebKit#258; the hang needs no import attributes at all.Test
test/js/bun/resolve/concurrent-dynamic-import.test.ts, "a failed dynamic import does not strand later static importers of the same file". Plain TypeScript, no import attributes.Verification
USE_SYSTEM_BUN=1 bun test test/js/bun/resolve/concurrent-dynamic-import.test.ts: the new test fails with a 5000ms timeout on released Bun 1.4.0 (the hang); the existing test passes.bun bd test test/js/bun/resolve/concurrent-dynamic-import.test.tsagainstmain'sWEBKIT_VERSION(c9ad5813fd, which contains ModuleLoader: record a non-Error fetch rejection as a fetch failure WebKit#262): both tests pass.