Skip to content

util.inspect and util.types.isAsyncFunction: read the kind of a function from the engine - #45735

Open
robobun wants to merge 3 commits into
mainfrom
robobun/c7eb8b12/util-inspect-function-kind
Open

robobun wants to merge 3 commits into
mainfrom
robobun/c7eb8b12/util-inspect-function-kind

Conversation

@robobun

@robobun robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • util.inspect reads the kind of a function from its source text. A generator method prints [Function: gen] GeneratorFunction, Node.js prints [GeneratorFunction: gen]. A method named asyncStorage prints [AsyncFunction: asyncStorage] Function.
  • The cause is two text tests at src/js/internal/util/inspect.js:147-150, a 2023 workaround. The native util.types.isAsyncFunction (src/jsc/modules/NodeUtilTypesModule.cpp:954) still reads the current prototype of the function.
  • util.inspect throws Error: Invalid object for a function whose chain has no constructor.

Fix

  • util.types.isAsyncFunction reads the parse mode of the function, as isGeneratorFunction does.
  • inspect.js binds both with $newCppFunction. The text tests are gone. internalGetConstructorName accepts a function: no throw.
  • Verified: test/js/node/util/node-inspect-tests/internal-inspect.test.js (50 fail on main) and test/js/node/util/test-util-types.test.js (11 fail on main), expected values from Node.js v26.3.0. Also test/js/node/util/, test/js/bun/util/inspect.test.js.
  • Self-reviewed: 9 concerns raised, 9 addressed.

Background

  • The parse mode is what JavaScriptCore records when it parses a function (async, generator, method, arrow). It never changes.
  • Node.js captures both checks at startup. Bun loads inspect.js after user code, so it binds private copies (as util.inspect: don't take the iterator guards from the writable node:util/types #45291).
  • Considered a private async check for inspect.js only: it adds a host function and leaves util.types.isAsyncFunction wrong.

Downsides

  • util.types.isAsyncFunction changes for 13 of 76 probed forms, always to Node's answer (util.promisify(asyncFn) gives false). 12 of 4,638 builtin labels change.
  • On a chain with no constructor, 467 of 817 probed function shapes print another word than Node.js. All threw before.
  • Release binary text grows by 508 B. util.inspect(fn) runs 5,705 instructions per call, before 7,296.
Notes

Origin. A fuzzer that compares the label of util.inspect(fn) with util.types found the wrong labels. There is no issue and no user report. The change to util.types.isAsyncFunction and the fix of the throw are not part of that report. They are here because Node.js takes the label from these two checks, and because the second place where inspect.js names the kind of a function needs them.

Repro (Node.js v26.3.0 prints the second column)

import util from "node:util";
const o = { *gen() {}, async *agen() {}, asyncStorage() {}, async() {} };
class C { static async *sagen() {} asyncHooks() {} }
util.inspect(o.gen);                  // [Function: gen] GeneratorFunction            [GeneratorFunction: gen]
util.inspect(o.agen);                 // [AsyncFunction: agen] AsyncGeneratorFunction  [AsyncGeneratorFunction: agen]
util.inspect(o.asyncStorage);         // [AsyncFunction: asyncStorage] Function        [Function: asyncStorage]
util.inspect(o.async);                // [AsyncFunction: async] Function               [Function: async]
util.inspect(C.sagen);                // [AsyncFunction: sagen] AsyncGeneratorFunction [AsyncGeneratorFunction: sagen]
util.inspect(C.prototype.asyncHooks); // [AsyncFunction: asyncHooks] Function          [Function: asyncHooks]
util.inspect(Object.setPrototypeOf(function f() {}, { __proto__: null }));
// Error: Invalid object                          [Function: f] Function <[Object: null prototype] {}>
util.types.isAsyncFunction(Object.setPrototypeOf(async function () {}, Function.prototype)); // false   true

The same labels come out of util.format("%O"), and for a function inside an object or an array.

Why util.types.isAsyncFunction changes too. Node.js uses one pair of checks for the label and for util.types. With the old native check the ported Node.js test test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js fails: it inspects a generator function whose prototype is AsyncFunction.prototype and expects [GeneratorFunction (anonymous)] AsyncFunction. #34872 fixed other util.types checks in place in the same way. No builtin module reads util.types.isAsyncFunction.

Why not a better regular expression. Text cannot see builtins, whose source is hidden, and a script can replace Function.prototype.toString and RegExp.prototype[Symbol.match].

The two private bindings. The two text tests were private to inspect.js. A read from the writable node:util/types would let a script change the kind in a label for the first time. The other 20 checks that inspect.js reads from node:util/types stay as they are, as decided in #45291 (comment).

The 2023 note. The note deleted from the ported test came with #4493 (e60b360). It says that the test relies on the text tests and that the native checks were flaky. Two defects were live then. isAsyncFunction answered from the prototype, which this change removes. Both checks called jsExecutable() on host functions, which c49547c (#20972) guards. That guard stays first in both checks. Every assertion of the ported test stays.

Deleted as dead. The two text tests, the comment with the two commented-out names, the note in the ported test, and the AsyncFunctionPrototype.h include, which had one use.

Measurements. Release builds of the merge base ea6a2c7 and of this branch (e19dd58), same WebKit, linux x64.

  • Size: release binary text 107,095,893 to 107,096,401 B. jsFunctionIsAsyncFunction 193 to 86 B, jsDollarLazy 3,959 to 3,998 B, JS2NATIVE_COUNT 262 to 264, bundled inspect.js 75,054 to 75,560 B. No new host function. The growth is the walk that names a function that is a prototype.
  • util.types.isAsyncFunction(async function f() {}): 47 to 24 executed instructions per call (gdb stepi), 55 to 26 static instructions, indirect calls 1 to 0.
  • Heap cells per util.inspect(function f() {}), JIT off, N=300 (bun:jsc heapStats): 28.25 to 19.25. RegExp 1 to 0, Array 10.01 to 6.01, Arguments 7 to 3. util.inspect({ a: 1, b: "x" }): 36.26 to 36.26. util.inspect(class K {}): 29.25 to 29.25.
  • Calls into functions a script can replace, per 2 inspected functions: Function.prototype.toString 6 to 2, RegExp.prototype[Symbol.match] 2 to 0, RegExp.prototype.exec 2 to 0, a replaced util.types.isAsyncFunction 0 to 0. Node.js: 0, 0, 0, 0.
  • First read of require("node:util").inspect in a realm: 1,021 to 1,020 cells (NativeExecutable 64 to 66, FunctionExecutable 90 to 88). require("node:util") alone still does not load inspect.js. node:util/types has 44 keys.
  • util.inspect(fn) over 8 forms whose label was right before, 2e6 iterations, JIT on: 7,296 to 5,705 instructions per call (perf_event_open instructions:u, 10 interleaved runs, spread 0.38% and 0.37%). Wall clock, 10 interleaved runs: median 1,760 to 1,168 ns per call, noise floor 790 ns and 1,135 ns, so the wall clock does not resolve the difference. perf, valgrind and bloaty are not installed here.

The 13 forms where util.types.isAsyncFunction changes (of 76 probed forms). isGeneratorFunction does not change.

  • false to true (7): an async function with Function.prototype, with a null prototype, with Object.create(AsyncFunction.prototype), and with a prototype that has no constructor. An async arrow with Function.prototype. An async method with a prototype that has no constructor. An instance of a subclass of AsyncFunction.
  • true to false (6): a generator function, a plain function, a class and a method with AsyncFunction.prototype. Reflect.construct(Function, [body], AsyncFunction). util.promisify(async function).

The 12 labels of builtin functions that change. The walk covers every function reachable from globalThis and the builtin modules. It does not cover functions that exist only on an instance. Bun.inspect labels changed: 0.

  • 6 Bun.$.ShellPromise methods: [Function: text] AsyncFunction to [AsyncFunction: text].
  • 4 generator methods on the prototype of process.allowedNodeEnvironmentFlags: [Function: values] GeneratorFunction to [GeneratorFunction: values]. Node.js prints [Function: values], its methods are not generators.
  • fs.Dir.prototype[Symbol.asyncIterator]: [AsyncFunction: [Symbol.asyncIterator]] AsyncGeneratorFunction to [AsyncGeneratorFunction: [Symbol.asyncIterator]].
  • AsyncResource.prototype.asyncId: [AsyncFunction: asyncId] Function to [Function: asyncId].

A function on a chain with no constructor. Node.js asks V8 for the name (GetConstructorHelper, linked in the code). V8 gives an ordinary function the name of the intrinsic constructor of its kind. For a function that is or was a prototype, a class, a bound function, and a function made by a subclass, V8 takes a name from own data properties of the chain: a string Symbol.toStringTag, then a named constructor below the function, then Function.

inspect.js knows one of these states: the function is the prototype of the value that it prints. Then it does the same walk, with own data properties only, so no getter runs and no replaced method decides the name. A matrix of 817 shapes (19 kinds of function on 4 chains, never a prototype or made one in 6 ways, through the 3 paths into the code):

  • main throws Error: Invalid object on 817. This branch throws on 0.
  • 350 print the string of Node.js. In this matrix that includes the 152 shapes where the function is the prototype of the printed value.
  • 444 differ and the function was a prototype before. 23 differ and the function is a class or a bound function on a chain with a tag, or it was made by a subclass or by Reflect.construct with another new.target. No occurrence of these shapes is known outside tests.

Outside this matrix, one more prototype shape differs. A function with a null prototype and an own property, used as a prototype, is printed by a nested inspect call that does not know that the value is a prototype. util.inspect(Object.create(Object.assign(Object.setPrototypeOf(async function h() {}, null), { a: 1 }))) prints Object <[AsyncFunction: null prototype]> {}, and Node.js prints Object <[Function: null prototype]> {}.

Not in this change

Suites run with the debug build: the three touched test files (also with BUN_JSC_validateExceptionChecks=1), test/js/node/util/, test/js/bun/util/inspect.test.js, test/bundler/bundler_browser.test.ts, test/js/web/console/console-log.test.ts, test/js/node/assert/assert.test.cjs, and the upstream test/js/node/test/parallel/test-util-types.js, test-process-binding-util.js, test-util-inspect-proxy.js, test-util-inspect-namespace.js, test-util-inspect-getters-accessing-this.js, test-util-inspect-regexp.js, test-util-types-exists.js.


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js

@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 6:05 PM PT - Oct 21st, 2026

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


🧪   To try this PR locally:

bunx bun-pr 45735

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

bun-45735 --bun

@robobun

robobun commented Oct 17, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for review (#45735). The diff is green on every CI lane that ran.

How I reproduced it. On main and on 1.4.3, with Node.js v26.3.0 as the reference:

import util from "node:util";
const o = { *gen() {}, asyncStorage() {} };
console.log(util.inspect(o.gen));          // bun: [Function: gen] GeneratorFunction        node: [GeneratorFunction: gen]
console.log(util.inspect(o.asyncStorage)); // bun: [AsyncFunction: asyncStorage] Function    node: [Function: asyncStorage]
console.log(util.inspect(Object.setPrototypeOf(function f() {}, { __proto__: null })));
// bun: Error: Invalid object    node: [Function: f] Function <[Object: null prototype] {}>

The new tests fail on main (50 in internal-inspect.test.js, 11 in test-util-types.test.js, measured at ea6a2c7) and pass on this branch.

CI on ac2ecc5 (build 130293): 179 jobs passed. The two :darwin: any x64 - test-bun jobs expired in the queue before an agent took them, so they did not run. The same three commits on the earlier base passed both of those jobs (build 129403). A retry of the two expired jobs is enough.

@coderabbitai

coderabbitai Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 4205e481-bd4e-4a49-abe8-4b55cecb38d2
📥 Commits

Reviewing files that changed from the base of the PR and between 14dc79a and e19dd58.

📒 Files selected for processing: 1
  • src/js/internal/util/inspect.js

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

Walkthrough

Native async and generator function checks now support function labels in util.inspect. Tests cover function classification, formatting, prototype behavior, and replaced detection methods.

Changes

Function Kind Classification and Inspection

Layer / File(s) Summary
Native function-kind classification
src/jsc/modules/NodeUtilTypesModule.h, src/jsc/modules/NodeUtilTypesModule.cpp, test/js/node/util/test-util-types.test.js
The native async-function check now uses executable parse mode. The header declares async and generator function checks. Tests cover classifications across function kinds, realms, proxies, and prototype changes.
Function labels in util.inspect
src/js/internal/util/inspect.js, test/js/node/util/node-inspect-tests/internal-inspect.test.js, test/js/node/util/node-inspect-tests/parallel/util-inspect.test.js
Constructor-name lookup now uses native function-kind checks and distinguishes prototype lookup. Tests cover function labels across inspection formats, prototype chains, and replaced detection methods.

Suggested reviewers: cirospaciari

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to e19dd

The change makes util.inspect and util.types.isAsyncFunction label async and generator functions from engine parse mode. No merge-blocking risk is identified from the supplied context.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly identifies the two main changes: engine-based function-kind detection for util.inspect and util.types.isAsyncFunction.
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.
Description check Passed The description clearly explains the problem, the implementation, scope, limitations, and verification results. It does not use the exact template headings, but it provides the required information fo…
Full details: Description check

Explanation

The description clearly explains the problem, the implementation, scope, limitations, and verification results. It does not use the exact template headings, but it provides the required information for what the PR does and how the code was verified.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • 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

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

@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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the C++ side of the change: jsFunctionIsAsyncFunction keeps the isHostFunction() guard ahead of jsExecutable(), and dropping the getPrototype walk removes the only call that could re-enter JS, so no exception scope is needed there; jsFunctionIsGeneratorFunction is unchanged apart from the new header declaration. The behavior change to util.types.isAsyncFunction for re-prototyped and promisify-wrapped functions was also looked at and ruled out as a bug — the new values match the Node.js expectations recorded in test/js/node/util/test-util-types.test.js.

Extended reasoning...

The change replaces the toString-parsing polyfills in src/js/internal/util/inspect.js with $newCppFunction bindings, switches jsFunctionIsAsyncFunction in src/jsc/modules/NodeUtilTypesModule.cpp from a prototype-chain check to the executable's parse mode, and lets internalGetConstructorName accept functions; it touches no auth, crypto, or input-parsing surface. Two confirmed findings about the new constructor-less-chain path are posted inline, so a human look is already signaled; this note only records what else was examined.

Comment thread src/js/internal/util/inspect.js Resolved
Comment thread src/js/internal/util/inspect.js Resolved
@robobun

robobun commented Oct 18, 2026

Copy link
Copy Markdown
Collaborator Author

Related: oven-sh/WebKit#834 (a draft, meant for WebKit/WebKit first) makes Reflect.construct(AsyncFunction, [body], Function) return a function whose prototype is Function.prototype, as CreateDynamicFunction says. Today JavaScriptCore gives it AsyncFunction.prototype.

With the prototype test that this PR removes, util.types.isAsyncFunction answers false for that function once the engine is fixed. Node.js answers true. So this PR has to be in Bun before a WEBKIT_VERSION that has the engine fix.

@robobun
robobun force-pushed the robobun/c7eb8b12/util-inspect-function-kind branch from fb24e84 to 14dc79a Compare October 18, 2026 21:31
Comment thread src/js/internal/util/inspect.js Resolved

@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.

I re-reviewed the two new commits and found no bugs; the constructor-less prototype branch now reads only own data properties through captured primordials, which covers what I raised earlier. Because this changes the observable answer of the public util.types.isAsyncFunction (e.g. util.promisify(asyncFn) now reports false) and the labels of a few builtins, a human look at that semantic shift would still be worthwhile.

What was reviewed:

  • jsFunctionIsAsyncFunction: host-function guard still precedes jsExecutable(); isAsyncFunctionParseMode includes async arrows, methods and async generators; bound functions and Proxies fall to false, matching the new Node-derived test rows.
  • internalGetConstructorName(val, true): the chain walk uses ObjectGetOwnPropertyDescriptor/ObjectGetPrototypeOf/SymbolToStringTag captured at load, no .constructor or Symbol.match lookups run; the subprocess test replaces toString, Symbol.match and util.types before loading node:util and checks zero calls.
  • FunctionPrototypeToString and StringPrototypeMatch keep other callers in inspect.js, so no dead primordials remain; the removed AsyncFunctionPrototype.h include had no other use.
Extended reasoning...

The diff touches src/js/internal/util/inspect.js (function-kind labelling and a new own-data-property name walk for functions inside a prototype chain), src/jsc/modules/NodeUtilTypesModule.cpp/.h (isAsyncFunction now reads the FunctionExecutable parse mode instead of the prototype chain, plus two forward declarations for $newCppFunction), and three test files with Node v26.3.0-cited expectations. No security-sensitive surface is involved beyond tamper-resistance of builtin module code, which the new code honours. The two new commits address both inline comments from the prior run, and the hunt ran dry with no findings. Deferring rather than approving because the PR intentionally changes the result of a public API (util.types.isAsyncFunction) for 13 probed function shapes and leaves part of V8's GetConstructorHelper deliberately unported, which is a product judgement a maintainer should confirm.

Still open from earlier reviews (1):

  • Unresolved: 1 minor or pre-existing.

@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.

I reviewed the three new commits and did not find any bugs; since this changes the answer of the public util.types.isAsyncFunction and ports only part of V8's GetConstructorHelper (the PR itself notes the direct-vs-prototype split is an approximation), a human look at that compat trade-off would still be worthwhile.

What was reviewed:

  • The prototype-walk in internalGetConstructorName now reads only own data descriptors and skips constructor on the receiver itself, which matches V8's GetConstructorHelper (it has the same "B.prototype.constructor = B" exclusion), so the earlier concern about user getters or Symbol.match steering a label no longer applies to the new code.
  • isAsyncFunctionParseMode is the same predicate BunCPUProfiler.cpp uses for the "async " display prefix and covers async arrows, methods and async generator wrapper modes; the host-function guard still precedes jsExecutable() in both native checks, and getFunctionBase picks up the same $newCppFunction bindings.
  • The ruled-out candidate (an own constructor on a prototype function printing differently from Node) is consistent with V8, not a deviation.
  • Tests spawn with bunEnv, drain stdout/stderr concurrently, and use await using.
Extended reasoning...

The diff rewires util.inspect's function-kind detection from source-text heuristics to two native host functions bound with $newCppFunction, changes jsFunctionIsAsyncFunction in NodeUtilTypesModule.cpp to answer from the executable's parse mode instead of the prototype chain, and adds an isPrototype branch to internalGetConstructorName that walks own data properties; it touches no auth, crypto, or input-parsing surface. Both prior inline findings were addressed by commits 11a73ee and 14dc79a (own-descriptor walk, isPrototype flag), and the threads were resolved by this bot's session rather than a person. What keeps this from an approve is that it is a user-visible semantic change to a public node:util/types API plus a deliberately partial port of V8 behavior (the PR description states 467 of 817 probed constructor-less shapes still print a different word than Node), which is a compat judgment call rather than a mechanical change; no CODEOWNERS entry covers these files and no third-party CHANGES_REQUESTED review is outstanding.

@robobun

robobun commented Oct 19, 2026

Copy link
Copy Markdown
Collaborator Author

A note on landing order. #46111 replaces the JS internalGetConstructorName of inspect.js with a native function. That function also names a function receiver, so the two PRs change the same lines and the second one to land has a conflict there.

If #46111 lands first, this PR can drop its hunk in internalGetConstructorName and the isPrototype argument of getConstructorName. Its two kind checks stay as they are.

I ran the 20 rows that this PR pins for a function on a chain with no constructor on the #46111 branch. All 20 print the same name after the label. Two still print the wrong label there ([Function: gen] for a generator method, [AsyncFunction: asyncStorage] for a method), which is the part this PR corrects.

@robobun
robobun force-pushed the robobun/c7eb8b12/util-inspect-function-kind branch from f212527 to e19dd58 Compare October 20, 2026 08:09
util.inspect decided the kind in a function's label from the first
characters of its source text. A generator method printed
`[Function: gen] GeneratorFunction`, and a method named `asyncStorage`
printed `[AsyncFunction: asyncStorage] Function`.

- util.types.isAsyncFunction reads the parse mode of the function's
  executable. Before, it read the current prototype of the function.
- inspect.js binds the native isAsyncFunction and isGeneratorFunction
  with $newCppFunction and no longer tests the source text.
- internalGetConstructorName accepts a function. Before, it threw
  `Error: Invalid object`, so util.inspect threw for a function whose
  prototype chain has no constructor.
…rties

A function that is the prototype of the inspected value fell through to
the lines that name an object. Those lines read `constructor` with a
normal get and match a regular expression through
RegExp.prototype[Symbol.match], so a getter or a replaced method decided
the name. V8 reads own data properties only: a string Symbol.toStringTag,
then a named constructor below the function, then "Function".

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.

2 participants