util.inspect: do not throw when an error's stack, name, message or errors getter throws

RO
12d ago
+245-13

Problem

  • assert.deepStrictEqual(AbortSignal.abort().reason, new Error()) throws TypeError: The DOMException.name getter can only be used on instances of DOMException, not an AssertionError. It also leaves Error.stackTraceLimit at 0.
  • AssertionError sets the limit to 0, builds its message with util.inspect, then restores the limit. formatError (src/js/internal/util/inspect.ts:1945) reads stack, name, message and errors with no guard.

Fix

  • getStackString, formatError, removeDuplicateErrorKeys and getStackFrames read each property once, in a try. A name, message or stack that cannot be a string does not throw.
  • An unreadable stack gives Object.prototype.toString.call(err), as in Node.
  • The name goes into the first line without String.prototype.replace, so $& in a name stays as it is.
  • Verified: test/js/node/util/node-inspect-tests/internal-inspect.test.js (15 tests fail on main), test/js/node/assert/assert.spec.ts (2 of 10 rows fail on main), and the other util.inspect and node:assert suites.

Background

  • util.inspect formats a value as an error when it inherits from Error.prototype. So the four properties can be getters that throw.
  • AssertionError shows two errors as copies (copyError): plain objects with the prototype of the original. The name and message getters of DOMException.prototype throw for such a copy.
  • No other place was weighed: Node guards the same reads in formatError.

Downsides

  • The embedded JS grows by 580 bytes.
  • None found for speed. Checked the bytecode: 6 fewer property reads for each error, and one regular expression less.
  • A Symbol as the own name or message still throws: the engine throws when util.inspect lists the own keys.
Notes

Merged with main at 11c367163a. #45631 renamed inspect.js to inspect.ts, typed it, and calls the primordials through .$call on the prototype method. The guards of this PR are in the renamed file in that form, and getStackString is typed string | undefined. The numbers below are from that merge. The diffstat in the gate block is from the earlier inspect.js.

Why it is alone. This part was split out of #45255 on self-review. It needs no decision, and #45255 asks a question. #45255 has the same commits.

How to reproduce. The error of a failed require() (a ResolveMessage) as an operand does the same.

const assert = require("node:assert");
Error.stackTraceLimit = 10;
try {
  assert.deepStrictEqual(AbortSignal.abort().reason, new Error("y"));
} catch (e) {
  console.log(e.constructor.name, e.code, Error.stackTraceLimit);
}
// Bun 1.4.3 and main: TypeError undefined 0
// Node v26.3.0 and this PR: AssertionError ERR_ASSERTION 10

Measurements. Main 11c367163a against this PR on one debug binary. Node is v26.3.0.

  • Failed comparisons that throw something other than an AssertionError or leave Error.stackTraceLimit at 0, over 96 cells (8 kinds of error, as an operand or as a cause, 6 assert calls): main 24, this PR 0, Node 0. This count is from the first base, 47ba1c8d85.
  • Bytecode instructions: formatError 289 to 322, getStackString 13 to 21, removeDuplicateErrorKeys 60 to 63, getStackFrames 86 to 89.
  • Property reads (get_by_id): formatError 16 to 11, getStackString 2 to 1. Regular expression literals in formatError: 1 to 0.
  • The first-line rename was one String.prototype.replace call with a regular expression. It is one startsWith call, and one slice call for a stack that starts with Error: .
  • Embedded JS from the release codegen (bundle-modules.ts --debug=OFF): 1 file changed, +580 bytes.
  • valgrind, perf and bloaty are not installed in the build container, so there is no dynamic instruction count.

What util.inspect prints now, as Node v26.3.0.

  • an own stack getter that throws: [Error: m]
  • own stack and name getters that throw: [object Error]
  • { __proto__: DOMException.prototype }: [object DOMException]
  • an enumerable name getter that throws, on an object with a stack: the stack, then { name: [Getter] }

What it prints where Node v26.3.0 throws or prints other text.

  • an error with a cause that inherits from DOMException.prototype, or a cause with stack and name getters that throw: the error, then [cause]: [object DOMException] or [cause]: [object Error]. Node throws from getStackFrames.
  • a native error with an own name or message getter that throws, or a name or message whose toString throws: the stack. Node prints [object Error]. V8 formats the stack when it is first read, so the read itself throws there.
  • a stack that is a Symbol: [Symbol(s)] { stack: Symbol(s) }. Node prints the Symbol as a value below Error: m.
  • a name with a replacement pattern, set after the stack was read (e.name = "A$&B"): the first line is A$&B: m. Main prints AError: B: m.

Not changed.

  • An operand with an own getter that throws, or a Proxy with a trap that throws, still leaves Error.stackTraceLimit at 0. Node does the same.
  • Object.getOwnPropertyNames(e), Object.getOwnPropertySymbols(e), Reflect.ownKeys(e) and "stack" in e throw TypeError: Cannot convert a symbol to a string for an error whose own name or message is a Symbol. Node returns the keys and prints [object Error]. This is in the engine, not in util.inspect.
  • util.inspect throws for any object with a Symbol.toStringTag getter that throws: formatRaw reads it with no guard (inspect.ts:1359). Node guards that read.

[human-review] gate passed · iteration 0 · 4 files touched

fails on main (without fix)
ASAN without fix: BUILD FAILED (no junit output)
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/util/node-inspect-tests/internal-inspect.test.js
ninja: Entering directory `/workspace/bun/build/debug'
[0/2] cargo plan → /workspace/bun/build/debug/rust-target/plan.json
FAILED: [code=1] rust-target/plan.json /workspace/bun/build/debug/rust-target/plan.json 
/workspace/bun/build/release/bun /workspace/bun/scripts/build/stream.ts cargo --console /workspace/bun/build/release/bun /workspace/bun/scripts/build/rust/plan.ts /workspace/bun/build/debug/rust-target/plan.input.json /workspace/bun/build/debug/rust-target/plan.json
^[[1m^[[91merror^[[0m: failed to load manifest for workspace member `/workspace/bun/src/api`
referenced by workspace at `/workspace/bun/Cargo.toml`

Caused by:
  failed to read `/workspace/bun/src/api/Cargo.toml`

Caused by:
  No such file or directory (os error 2)
error: /opt/rust/bin/cargo build -p bun_runtime --lib … exited with 101
ninja: error: rebuilding 'build.ninja': subcommand failed
error: script "bd" exited with code 1
__F:-1:S:0

release without fix: 15 failed, 1 skipped
bun test v1.4.3-canary.1 (367d939d9)

test/js/node/util/node-inspect-tests/internal-inspect.test.js:
(pass) no assertion failures [4.70ms]
(pass) boxed BigInt/Symbol with no prototype are still formatted as boxed primitives [0.29ms]
(pass) util.inspect reports the non-index keys of an array [1.97ms]
(pass) util.inspect reports the non-index keys of a typed array [3.52ms]
(pass) util.inspect of a large Array does not visit every element [12.92ms]
(pass) util.inspect of a large Uint8Array does not visit every element [9.89ms]
160 | // A value is formatted as an error when it inherits from Error.prototype, so its `stack`, `name`,
161 | // `message` and `errors` can be getters that throw. The `name` and `message` of DOMException.prototype
162 | // throw for a receiver that is not a DOMException. The expected text is what node v26.3.0 prints.
163 | const throwingGetter = {
164 |   get() {
165 |     throw new RangeError("getter");
                                       ^
RangeError: getter
      at get (/workspace/bun/test/js/node/util/node-inspect-tests/internal-inspect.test.js:165:34)
      at getStackString (internal:util/inspect:1089:15)
      at formatError (internal
... (truncated)
passes on PR (with fix)
ASAN with fix: 1 skipped
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/pr_gate.xml" test/js/node/util/node-inspect-tests/internal-inspect.test.js
bun test v1.4.4 (367d939d9)

test/js/node/util/node-inspect-tests/internal-inspect.test.js:
(pass) no assertion failures [326.10ms]
(pass) boxed BigInt/Symbol with no prototype are still formatted as boxed primitives [17.44ms]
(pass) util.inspect reports the non-index keys of an array [108.32ms]
(pass) util.inspect reports the non-index keys of a typed array [48.81ms]
(pass) util.inspect of a large Array does not visit every element [135.94ms]
(pass) util.inspect of a large Uint8Array does not visit every element [166.58ms]
(pass) util.inspect of an object with an own stack getter that throws [15.55ms]
(pass) util.inspect of an object with own stack and name getters that throw [6.56ms]
(pass) util.inspect of an object with a stack getter that throws and Error.prototype as its prototype [5.68ms]
(pass) util.inspect of an object with stack, name and message getters that throw and Error.prototype as its prototype [6.48ms]
(pass) util.inspect of an object with DOMException.protot
... (truncated)

release with fix: 1 skipped
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     6e1ed12225
  features     lto, baseline

23 deps, 146 codegen, 1176 objects in 2271ms

ninja: Entering directory `/workspace/bun/build/release'
[1/4] fetch lolhtml
[lolhtml] up to date
[2/4] fetch rust-argon2
[rust-argon2] up to date
[2/4] cargo plan → /workspace/bun/build/release/rust-target/plan.json
265 units: 193 lib, 16 proc-macro (host), 19 custom-build (host), 15 run custom-build, 17 lib (host), 4 run custom-build (host), 1 rlib
[3/4] reconfigure
[1/1530] mkdir stamps
[2/1530] mkdir codegen
[3/1530] gen bake.{client,server,error}.js
-> bake.client.js, bake.server.js, bake.error.js
[4/1530] install /workspace/bun
bun install v1.4.3-canary.1 (367d939d9)

Checked 22 installs across 44 packages (no changes) [732.00ms]
[5/1530] install /workspace/bun/packages/bun-error
bun install v1.4.3-canary.1 (367d939d9)

Checked 1 install across 2 packages (no changes) [51.00ms]
[6/1530] install /workspace/bun/src/node-fallbacks
bun install v1.4.3-canary.1 (367d939d9)

Checked 111 installs acro
... (truncated)
diff hotspot
src/js/internal/util/inspect.js                    |  58 +++++++++--
 test/js/node/assert/assert.spec.ts                 |  29 +++++-
 test/js/node/inherited-errors.ts                   |  53 ++++++++++
 .../node-inspect-tests/internal-inspect.test.js    | 116 +++++++++++++++++++++
 4 files changed, 244 insertions(+), 12 deletions(-)

gate history · 1 passed · 0 rejected · iteration 0

evidence per changed file
file                                                      reads  edits  tests
src/js/internal/util/inspect.js                               4      5     28
test/js/node/assert/assert.spec.ts                            3      4     41
test/js/node/inherited-errors.ts                              0      1     26
…s/node/util/node-inspect-tests/internal-inspect.test.js      4      4     26