Skip to content

Preserve AsyncLocalStorage context during dynamic import() module evaluation - #32695

Closed
robobun wants to merge 5 commits into
mainfrom
farm/46f85f9b/als-dynamic-import-eval
Closed

Preserve AsyncLocalStorage context during dynamic import() module evaluation#32695
robobun wants to merge 5 commits into
mainfrom
farm/46f85f9b/als-dynamic-import-eval

address review: cover virtual modules, clean up captured context on i…

69a3175
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jun 25, 2026 in 23m 12s

Code review found 1 important issue

Found 5 candidates, confirmed 4. See review comments for details.

Details

Severity Count
🔴 Important 1
🟡 Nit 0
🟣 Pre-existing 0
Severity File:Line Issue
🔴 Important src/jsc/bindings/ZigGlobalObject.cpp:3492-3507 Missing exception checks after new JSMap operations (x64-asan CI failure)

Annotations

Check failure on line 3507 in src/jsc/bindings/ZigGlobalObject.cpp

See this annotation in the file changed.

@claude claude / Claude Code Review

Missing exception checks after new JSMap operations (x64-asan CI failure)

The new `JSMap` operations (`set`/`has`/`get`/`remove`) all open a `ThrowScope` in `JSOrderedHashTable`, but none of the call sites this PR adds follow with an exception check — this is the x64-asan CI failure ("unchecked exception at requestImportModule … thrown from JSOrderedHashTable.h:83") on `bun-server.test.ts` and `AsyncLocalStorage.test.ts`. Since these are JSString-keyed ops on an internal builtin map that provably cannot throw, declare a `ThrowScope` in the helpers and add `scope.asser