node:events, node:assert: throw the error itself when it inherits from Error.prototype

RO
12d ago
+675-40

Problem

  • With no "error" listener, emitter.emit("error", x) throws a new ERR_UNHANDLED_ERROR, not x, when Error.isError(x) is false: an error of a class written without class, a DOMException, a Proxy of an Error. assert(false, x) throws a new AssertionError.
  • The cause is Error.isError in emitError (src/js/node/events.ts:323) and innerOk (src/js/internal/assert/utils.ts:20). The other assert calls ask instanceof Error and replace an error of another realm.

Fix

  • internal/shared gets Node's internal isError: Error.isError(e), or Error.prototype is in its prototype chain.
  • emitError and seven node:assert places call it. It replaces four other predicates in src/js.
  • The util.inspect commits are #45254. copyError needs them.
  • Verified: test/js/node/events/event-emitter.test.ts, test/js/node/assert/assert.spec.ts and three more files (76 tests fail on main), 235 Node tests. Self-reviewed: see Notes.

Background

  • For @Jarred-Sumner: #15698 asked for Error.isError because instanceof can throw. The helper asks Error.isError first, then walks the chain. See Notes.
  • JavaScriptCore's instanceof never returns for a chain that cycles through a Proxy. So the helper walks the chain and stops after 1000 Proxies.
  • Considered a two-line patch: node:assert keeps two predicates. #32898 fixes only DOMException.

Downsides

  • Not as Node: emit throws a native error of another realm as itself. A chain with more than 1000 Proxies is no error (Node: RangeError).
  • An old-style error has a .stack that starts with a bare Error (#43390). node:test run() reports only its text.
  • Cost: 1 more JS call on an unhandled emit and on a failing assertion. 513 bytes of embedded JS.
Notes

Merged with main at 11c367163a. #45631 typed the files of this PR and renamed inspect.js and node-shims.js to .ts. The changes are in those files in that form. events.ts keeps the shared binding of main and takes isError from it. iter/utils.ts and repl/node-shims.ts use the shared helper in place of the typed private copies of main.

Self-review. It asked for a re-cut before this PR was opened, and this is the re-cut. The util.inspect guards became #45254, so that they do not wait for the question below. emit calls the helper and not a bare instanceof. The Bun.sql and zlib edits are out. The costs it named are in Downsides and in this block.

The predicate question.

  • #15698 (review): "use Error.isError instead" and "instanceof can throw, can we do Error.isError(message) instead". #19121 then changed emit from er instanceof Error to Error.isError(er).
  • Error.isError alone cannot fix this bug: it is false for every error that user code makes without new Error() or class extends Error. jsonwebtoken, axios, bluebird and verror make such errors. assert.fail, the comparisons and assert.match have asked message instanceof Error since #15698.
  • What the helper does about the throw: a native error never reaches the prototype walk. The walk uses Object.getPrototypeOf and Error.prototype, both captured at load. So a user-defined Error[Symbol.hasInstance] is not called, and a replaced globalThis.Error changes nothing. A Proxy with a getPrototypeOf trap that throws still throws, as in Node. The test row "a getPrototypeOf trap that throws propagates" pins that. #36913 kept the same formula in util.isError.
  • The other choice: a walk that stops at the first Proxy and treats it as no error. It cannot throw. A Proxy of an Error is then wrapped, which Node does not do.

Not as in Node, on purpose. Node's emit asks only er instanceof Error, so it wraps a native error of another realm (vm.runInNewContext("new Error()")) in ERR_UNHANDLED_ERROR. Bun 1.2.11 and later throw it as itself, and this PR keeps that. With Node's line, domain.run, domain.bind and a res.end() callback that throws would start to wrap such an error, which Node and Bun 1.4.3 do not do (12 of 138 probed domain cells, 1 res.end row). The test row "a native error of another realm is thrown as itself" pins it.

The second difference is the bound. A prototype chain can have no end only through a Proxy, for example const x = new Proxy({}, { getPrototypeOf: () => x }). Node throws a RangeError for x instanceof Error. JavaScriptCore loops for ever in JSObject::defaultHasInstance: on Bun 1.4.3 assert.fail(x) and assert.strictEqual(1, 2, x) never return. The helper counts the Proxies on the chain and answers false after 1000. So emit("error", x) throws the ERR_UNHANDLED_ERROR wrapper and the assert calls throw an AssertionError, where Node throws the RangeError. No call hangs. The count never starts for a chain without a Proxy. The test rows "a prototype chain that cycles through a Proxy is no error" and "a message whose prototype chain cycles through a Proxy is no error" pin it. For const o = {}; Object.setPrototypeOf(o, new Proxy(o, {})) the same calls throw a RangeError in Bun too: util.inspect throws it when it builds the message.

Measurements. Main 11c367163a against this PR on one debug binary. Node is v26.3.0. The numbers are for this PR with #45254. The rows that count probe cells or calls are from the first base, 47ba1c8d85.

  • Substitute errors per 10,000 calls: emit 1 (main 10,000, Node 1), assert.ok 1 (main 10,000, Node 1), for 8 kinds of error each. 12 fail, comparison and match calls with an error of another realm: 1 (main 10,000, Node 1).
  • Paths that never reach the predicate: 0 of 3 hot functions changed bytecode (emit 113 instructions, ok 13, strictEqual 37).
  • Changed functions: emitError 72 to 71 instructions, innerOk 39 to 38, innerFail 21 to 22, fail 75 to 76, internalMatch 95 to 96. The new isError is 31.
  • An unhandled emit of a native error: 1 JS call and 1 host call (was 1 host call). A failing assertion with a string message: 1 JS call and 1 host call (was 1 instanceof, no call).
  • Module load: +1 function per VM that loads internal/shared. Top-level bytecode: internal/shared 324 to 339 instructions, node:events +2, node:assert +12, internal/assert/utils +12.
  • Embedded JS from the release codegen (bundle-modules.ts --debug=OFF): 9 files, +513 bytes for the commits of this PR alone. With #45254: +1,093 bytes. No new host function, no new module, no native line.
  • Rows that differ from Node over 15 kinds of value: emit 9 to 1 (the other realm), assert.ok 8 to 0, assert.fail 1 to 0. A 100-cell probe of 18 assert calls: 28 to 0.
  • node:domain over 138 probed cells: 50 more equal Node, 0 fewer.
  • Error.isError( in src/js: 15 to 10. Copies of Node's isError formula: 5 to 2.
  • valgrind, perf and bloaty are not installed in the build container, so there is no dynamic instruction count and no size from full release builds.

More on the downsides.

  • Error.captureStackTrace(this, F) on an object that is no native error writes a stack that starts with Error, not with the name and the message. On Bun 1.4.3 the wrapper carried the text. #43390 changes the header.
  • assert.deepStrictEqual(new DOMException("m", "AbortError"), new DOMException("m", "TimeoutError")) does not throw: the native comparator knows only native errors. console.log and the uncaught printer show an old-style error as an object (#35723).
  • A failing assertion whose message is not an error makes 1 more JS call and 1 more host call before it builds the AssertionError.
  • node:test run(): t.assert.ok(false, x) with an old-style error now throws x, and serializeRunError reports only String(x) for a value that Error.isError rejects. A test that does throw x gets the same report on main.

Left as they are.

  • src/js/node/test.ts:707 (serializeRunError): it needs guarded reads of message, stack, code, name and cause first, and #45003 moves it to another file. The PR that merges second points it at the shared helper.
  • src/js/node/assert.ts:561 and src/js/internal/assert/assertion_error.ts:301-303: Node has a bare instanceof Error in the same lines (expected instanceof Error in expectedException, and the stack of the actual of assert.throws).
  • src/js/bun/ffi.ts (6 places), src/js/bun/sql.ts:448 and src/js/internal/streams/writable.ts:635: the value always comes from Bun's own code.
  • src/js/node/zlib.ts:474: only an overridden destroy() reaches it.
  • src/js/internal/util/inspect.ts:1495 and :1839: the same formula, written inline. No change.
  • The native EventEmitter of process, process.emitWarning, and the Node 26 message rules of assert (ERR_AMBIGUOUS_ARGUMENT, function messages).

What consumers see for an old-style error that is now thrown as itself. It is what they see today for a direct throw x. The uncaught printer shows name, message, code and stack. A worker.on("error") clone keeps the own properties, as in Node. node:test run() reports the message text only.

Tests. Each expectation is what Node v26.3.0 prints for the same code, but for the row of the other realm.

  • test/js/node/events/event-emitter.test.ts: 36 tests, 34 fail on main. 10 kinds of error through emit, EventEmitterAsyncResource and a domain. readable.destroy(x), writable.destroy(x) and captureRejections to uncaughtException. Replaced globals.
  • test/js/node/assert/assert.spec.ts: 18 calls with 11 kinds of error each, the text of assert.throws, the copy of a cause, a chain that cycles through a Proxy, replaced globals. 25 tests fail on main.
  • test/js/node/stream/node-stream.test.js: pipeToSync, pipeTo, share, shareSync, push of node:stream/iter.
  • test/js/node/test_runner/fixtures/20-hook-signal-and-assert-ok.js: t.assert.ok.
  • test/js/node/test/parallel/test-internal-util-helpers.js now runs against this helper.

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/stream/node-stream.test.js