Skip to content

node:async_hooks: remove the debug-build id and inspect hook of AsyncLocalStorage - #48532

Open
robobun wants to merge 2 commits into
mainfrom
robobun/eddc23b7/async-local-storage-debug-id
Open

robobun wants to merge 2 commits into
mainfrom
robobun/eddc23b7/async-local-storage-debug-id

Conversation

@robobun

@robobun robobun commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • Remove the id, the inspect hook, and the frame dump (debugFormatContextValue). The $debug calls keep their text.
  • A debug build now answers as release builds and Node.js do: no own keys, AsyncLocalStorage {} from util.inspect().
  • Verified on debug builds. test/js/node/async_hooks/async_hooks.node.test.ts: 4 pass / 3 fail, then 7 pass. visible-surface.test.ts: 1 pass / 2 fail, then 3 pass.
  • Self-reviewed: 14 concerns raised, 13 addressed. Not done: a tracking issue for what stays different (Notes).

Background

  • IS_BUN_DEVELOPMENT code and $debug(...) arguments run only in a debug build. They run also when the debug log is off.
  • A frame binds a storage to a value. A mock factory runs in a frame whose storage is the mock.
  • Considered a filter in the test: the runtime still differs. Considered an id in a WeakMap: debug-only state that no CI lane runs.

Downsides

  • Readers of the [async_hooks] debug log lose the storage ids and the frame dump. Before: set { zs2l7z@hooks: number; }. After: set.
  • Release builds: none found. The bundle of all built-in modules is byte-identical.
Notes

The new tests

All three are in test/js/node/async_hooks/async_hooks.node.test.ts. Each fails on a debug build of main and passes on a debug build of this branch. A release build passes them with and without the change, because only a debug build has the defect.

Test Removed code it fails on
has no own keys and no method for util.inspect() the __id__ write in the constructor, the inspect hook
disable() with a receiver that is no storage the this.__id__ read in the log call of disable() (a TypeError for undefined)
a job that the factory of a module mock scheduled the frame dump

test/js/node/visible-surface.test.ts is not changed. Its snapshot already has the release surface.

Measurements

Debug builds of main 89452d2 and of this branch, idle machine.

main this PR
async_hooks.node.test.ts 4 pass / 3 fail 7 pass / 0 fail
visible-surface.test.ts 1 pass / 2 fail 3 pass / 0 fail
per new AsyncLocalStorage(): Math.random calls, __id__ sets, __id__ gets 1, 1, 1 0, 0, 0
per run() + getStore(): __id__ gets 6 0
Reflect.ownKeys(storage) ["__id__"] []
util.inspect(storage) AsyncLocalStorage { [debug id]: "3f2j5w@hooks" } AsyncLocalStorage {}
debug bundle of node:async_hooks 16,680 B 15,616 B
embedded source archive (builtin_sources.bin), debug 1,227,202 B 1,227,206 B

Release code generators (bun scripts/build.ts --profile=release --target=codegen) on main 89452d2 and on this branch:

main this PR
bundle of node:async_hooks 12,421 B, md5 3d8299c3918c54105d85af39b5ad8e16 the same
bundle of all built-in modules (InternalModuleRegistryConstants.bin) 2,785,032 B, md5 16c943149c5feb72e05c4a5d86d88f27 the same
embedded source archive (builtin_sources.bin) 1,355,000 B 1,354,609 B

Release builds and Node.js v26.3.0 already give the "this PR" answers of rows 3 to 6.

--aot, measured on the earlier base 2596ac5: the program that --aot-out writes for a file that uses AsyncLocalStorage goes from 1,754,040 B to 1,751,632 B. The code map has the same 55 node:async_hooks functions with the same sizes (16,202 B).

The debug log

BUN_DEBUG_ASYNC_HOOKS=1 turns it on. Two nested run() calls print 20 lines before and after. 17 of them had a storage id. The part of the id after @ was meant to name the file that created the storage. It is the constant hooks today, because callerSourceOrigin() answers builtin://node/async/hooks.

main                                          this PR
[async_hooks] run zs2l7z@hooks                [async_hooks] run
[async_hooks] get undefined                   [async_hooks] get
[async_hooks] set { zs2l7z@hooks: number; }   [async_hooks] set
[async_hooks] run pmz2jp@hooks                [async_hooks] run

console.log(storage) on a debug build printed the one-line [debug id] form. It now prints the member list: 13 lines, where a release build prints 11 without the two private fields (measured on the earlier base 2596ac5).

Other suites on the debug build of this branch

With --timeout=270000, the per-test limit of the ASAN lanes in CI (scripts/runner.node.ts:1536). bun bd test has a 5 s default limit, and many of these tests start a debug bun of their own. Runs "with the bundle of main" use the debug binary of this branch with main's bundle of built-in JS. A debug binary reads that bundle from disk.

  • test/js/node/async_hooks/: all 5 files pass (63, 76, 5, 5, 7 tests).
  • Module mocks, whose frames have a mock as storage: mock-fn.test.js (848), mock/mock-module.test.ts (314), mock/mock-hoist.test.ts (134), vi-utils.test.ts (191), vitest-flavor.test.ts (191), require-bun-test.test.ts (9), mock/mock-module-non-string.test.ts (5), mock/mock-module-resolve-log.test.ts (1), mock/6874/A.test.ts (1), mock/6874/B.test.ts (1), mock/6879/6879.test.ts (2). All pass.
  • Bun.ModuleGraph, whose frames have a graph or the global object as storage: module-graph.test.ts (297), module-graph-matrix.test.ts (893), module-graph-workers.test.ts (479), module-graph-gc.test.ts (38), module-graph-io.test.ts (28), module-graph-callbacks.test.ts (1). All pass.
  • module-graph-isolation.test.ts: 413 pass in 4 of 6 runs. In the other 2 runs, 4 tests and 1 test of one concurrent group ran past their own poll deadline. With the bundle of main: 413 pass in 5 of 6 runs, and 5 tests of that group failed in the other run.
  • module-graph-compile.test.ts: 76 pass in 7 of 9 runs. In 2 runs the first build hook ran past its 60 s limit. The file writes four debug executables of 1.6 GB at once. With the bundle of main: the same failure in 1 of 8 runs.

Not changed by this PR

These make a debug build answer differently from a release build through other code. Each needs a change of its own.

  • $debug(...) arguments. src/codegen/replacements.ts:280 expands the call to (IS_BUN_DEVELOPMENT?$debug_log(...):void 0), so a debug build evaluates the arguments with the log off. A guard on the log switch there would stop that for every module.
  • Three debug assertions in src/js/node/async_hooks.ts. enterWith() and run() call this.getStore() inside $assert (lines 262 and 313), and assertValidFrame (line 74) requires the storage of a frame to be an object. With a receiver that is not a storage, enterWith() throws on a debug build and returns undefined on a release build.
  • test/js/node/members-of-prototype-objects-for-any-receiver.test.ts is 27 pass / 5 fail on a debug build before and after. Its node:async_hooks case goes from 32 differing lines to 20. The 20 lines are the answers of enterWith() and run() for receivers that are not storages. Its node:events, node:stream, node:http and node:zlib cases also fail. This PR does not look at them.
  • test/js/node/async_hooks/AsyncLocalStorage.test.ts at the 5 s default limit: 62 pass / 1 fail before and after. "re-entering a storage inside run() does not grow the context" takes 12.5 s before and 6.6 s after.
  • Two other debug ids, set only when the debug log is on: stream.debugId (src/js/internal/streams/native-readable.ts:68) and globalThis.__lastId / __getId (src/js/node/child_process.ts:73).

…LocalStorage

A debug build gave every AsyncLocalStorage an own `__id__` property and gave
the prototype a `Symbol(nodejs.util.inspect.custom)` method. A release build
has neither. test/js/node/visible-surface.test.ts records the surface of a
release build, so two of its tests failed on every debug build.

The debug log also read `__id__` from the storage of every frame on each
get() and set(). The storage of a frame can be a module mock. A mock of
vi.mock() or vi.doMock() throws on a read of an export that its factory did
not return, so on a debug build AsyncLocalStorage threw in a job that such a
factory scheduled.

Remove the id, the inspect hook, and the frame dump. The `[async_hooks]`
debug log keeps its lines, without the storage ids.
@robobun

robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:05 PM PT - Nov 10th, 2026

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


🧪   To try this PR locally:

bunx bun-pr 48532

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

bun-48532 --bun

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: reproduced, fix is in this PR (#48532).

How I reproduced it, on a debug build of main 89452d2:

bun bd test test/js/node/visible-surface.test.ts
  • 1 pass, 2 fail. The only lines that differ from the snapshot are the __id__ own key and the Symbol(nodejs.util.inspect.custom) prototype method of AsyncLocalStorage.
  • A debug build of this branch gives 3 pass, 0 fail.
const { AsyncLocalStorage } = require("node:async_hooks");
const storage = new AsyncLocalStorage();
console.log(Reflect.ownKeys(storage), require("node:util").inspect(storage));
  • A debug build of main prints [ "__id__" ] AsyncLocalStorage { [debug id]: "hepkay@hooks" }.
  • A debug build of this branch, bun 1.4.3-canary.1 (release) and Node v26.3.0 print [] AsyncLocalStorage {}.

CI: no failure comes from this diff. The diff changes only code that a debug build runs, and CI runs release builds. The release bundle of the built-in modules is byte-identical to the one of main.

Build 140025 on fe6edab (an empty commit on top of the diff), not finished yet:

  • 184 of 188 jobs passed. That is every build lane, and every test shard on Alpine, Debian (also ASAN and hardened), Ubuntu, Windows and macOS aarch64.
  • The other 4 jobs are the macOS x64 test shards. They wait for an agent. That lane is several hours behind. Their first attempts expired in the queue, so the buildkite/bun check shows as failing, with no failed job.
  • 6 test files failed once and passed on a retry: test/aot/compiled-code-mapping.test.ts, test/js/node/readline/history-of-interface-and-repl.test.ts, test/js/bun/gc/wasm-memory-dropped.test.ts, test/js/bun/spawn/spawn_waiter_thread.test.ts, test/cli/run/no-orphans.test.ts and test/js/bun/webview/webview-chrome.test.ts.

Build 139937 on fea61ad (the same diff), finished:

  • 187 of 188 jobs passed.
  • 1 macOS x64 test shard failed, on test/aot/parallel/node-http/objects-closed-proxies-without-traps.ts. In 1 of its 643 cases the compiled program prints the request close event one case late. The same test fails the same way on macOS x64 in builds 139913 and 139917 of two other branches. It passes on the macOS aarch64 lane of this commit.

@coderabbitai

coderabbitai Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

The changes remove context formatting, per-instance debug IDs, and the custom inspector representation from AsyncLocalStorage. New tests cover inspection output, disable() with an undefined receiver, and async context behavior in a module-mock factory.

Changes

AsyncLocalStorage debug output and tests

Layer / File(s) Summary
Remove debug context details
src/js/node/async_hooks.ts
Debug logging no longer formats context values or includes per-instance IDs. The development-only custom inspector formatter is removed. A frameMutations counter is declared.
Test inspection and async context behavior
test/js/node/async_hooks/async_hooks.node.test.ts
Tests check own keys and inspection output for a named instance with a default value, disable() with an undefined receiver, and job results [undefined, 2, undefined] in a module-mock factory.

Suggested reviewers: jarred-sumner, dylan-conway

Merge Risk: Minimal · up to fe6ed

Merge Risk: ⚪ Minimal · up to fe6ed

No actionable merge-blocking risk was identified.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly identifies the AsyncLocalStorage debug ID and inspect-hook removal, which is the main change.
Description check Passed The description explains the problem, the fix, and how the author verified the changes. It provides sufficient detail to meet the template’s purpose, though it uses different headings.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Recent review info

Run configuration

Commits

Reviewing files that changed from the base of the PR and between 89452d2 and fea61ad.

Files selected for processing: 2

  • src/js/node/async_hooks.ts
  • test/js/node/async_hooks/async_hooks.node.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant