[robobun]

Problem

• assert.partialDeepStrictEqual(new DOMException("boom", "AbortError"), new DOMException("other", "NotFoundError")) passes. Node v26 throws ERR_ASSERTION. Same for two DOMExceptions that differ only in message, only in name, or only in cause, and for an expected DOMException that has a cause when the actual one has none. The same holds when the DOMExceptions are nested in objects or arrays, and for AbortSignal reasons compared against a DOMException with a different name or message.
• assert.partialDeepStrictEqual is native (PartialDeepEqual::compareBranch, src/jsc/modules/NodeUtilTypesModule.cpp). Its error arm keys on inherits<JSC::ErrorInstance>(). A DOMException is a WebCore::JSDOMException wrapper, not an ErrorInstance, so the pair falls through to the generic tail of compareBranch: the Object.prototype.toString tags match ([object DOMException] on both sides) and objectSubset walks expected's own enumerable properties, of which a DOMException has none (name and message are accessors on DOMException.prototype, cause is an own non-enumerable property). Nothing is compared, so every pair is reported as equal.
• Smaller, in the error comparison itself (errorSubset, shared by the Error arm and the new DOMException arm): cause was only looked at when expected had it as an own property, and was then always compared. So partialDeepStrictEqual(new Error("x", { cause: 1 }), new Error("x", { cause: undefined })) threw (node passes) and an expected error whose cause comes from a prototype getter was never compared (node compares it).

Fix

• Add a DOMException arm to compareBranch, next to the URL and KeyObject arms: if either side is a JSDOMException, both must be, and the pair goes through the existing errorSubset.
• errorSubset is the right comparison because it reads the error properties with an ordinary [[Get]], which runs the DOMException prototype accessors, and applies node's own-cause rule. That is what node's error arm does: node treats anything instanceof Error as an error there and reads the same properties, with the same partial-mode leniency (an undefined expected value or an empty expected message is ignored). DOMException's prototype chain goes through Error.prototype, so node takes that arm for it.
• The arm is a separate arm, ahead of the ErrorInstance one, rather than a widening of the ErrorInstance check: node rejects a DOMException against a plain Error with the same name and message (its tag check runs before its error arm), and errorSubset alone would accept that pair. Requiring both sides to be DOMExceptions keeps that rejection, and the ordering keeps it even if JSDOMException later becomes an ErrorInstance (DOMException: inherit from ErrorInstance for [[ErrorData]] and .stack #32898).
• In errorSubset, move cause into the loop that reads message, name and errors, and leave only the presence check in the own-property block. That is node's structure exactly: isEnumerableOrIdentical for each of the four properties, then the separate own-cause presence rule (comparisons.js). It fixes the two cause divergences above for plain Errors as well as DOMExceptions.
• Not changed here: assert.deepStrictEqual / util.isDeepStrictEqual have the same DOMException blind spot in Bun__deepEquals (bindings.cpp), a different implementation that also backs Bun.deepEquals and the expect() matchers. That is fixed separately in deepEquals: compare DOMException name, message and cause #39308; the two changes do not overlap.
• Verified with test/js/node/assert/assert.test.cjs, two new blocks in the partialDeepStrictEqual describe. DOMException: 14 of its cases fail on the unfixed binary (USE_SYSTEM_BUN=1, every "rejects" case whose operands are both DOMExceptions); the accepted cases and the mixed pairs (DOMException vs {}, vs a same-shaped plain object, vs an Error in both directions, expected with an extra own property) pin the behavior that was already correct. error cause: 3 of its cases fail on the unfixed binary (undefined expected cause, and the two inherited-cause cases); the rest pin the existing cause rules. All 58 tests in the file pass with the fix. Every expectation was checked against Node v26.3.0.
• Also run with the fix: test/js/node/assert/ plus the vendored test-assert.js, test-assert-class.js and test-assert-deep-with-error.js (486 pass), test-assert-typedarray-deepequal.js and test-util-isDeepStrictEqual.js, and the test file under BUN_JSC_validateExceptionChecks=1.

Background

• assert.partialDeepStrictEqual(actual, expected) is node's "partial" deep equality: expected has to be recursively contained in actual (own enumerable properties of expected must exist and match on actual, arrays are matched as an in-order subsequence, and so on). Bun implements it natively in compareBranch, which is a chain of arms, one per kind of value (Map, typed array, KeyObject, URL, Error, Date, ...), each of which first checks that both sides are of that kind and then compares the kind's internal state before falling back to the own-property subset walk. A pair that matches no arm ends in the Object.prototype.toString tag check plus that walk.
• errorSubset is the body of the Error arm. Node's rules for errors in partial mode, which it implements: message, name, cause and errors are read with [[Get]] on both sides and compared unless the expected value is undefined (or, for message, empty); an own cause on expected, even one set to undefined, requires an own cause on actual; then the normal own-property subset walk runs.
• JSDOMException is the JS wrapper for WebCore's DOMException. Its prototype's prototype is Error.prototype, so instanceof Error is true, but the object itself is a plain DOM wrapper: name/message/code are prototype getters over the wrapped C++ object, and the constructor stores an options cause as an own non-enumerable property, the same shape node's DOMException has.

Comparison table, fixed debug build vs Node v26.3.0

Each line is assert.partialDeepStrictEqual(actual, expected). The first ten lines pass on the unfixed binary; everything else was already correct.

diff name+msg (should throw)                                           THROWS
same name diff msg (should throw)                                      THROWS
same msg diff name (should throw)                                      THROWS
diff cause (should throw)                                              THROWS
expected has cause, actual not (should throw)                          THROWS
actual empty msg, expected msg (should throw)                          THROWS
nested in object diff (should throw)                                   THROWS
subclass diff msg (should throw)                                       THROWS
AbortSignal.abort().reason vs different DOMException (should throw)    THROWS
in array subsequence: no match (should throw)                          THROWS
actual has cause, expected not (should pass)                           passes
identical (should pass)                                                passes
expected cause subset of actual cause (should pass)                    passes
expected empty msg (should pass)                                       passes
actual extra own prop (should pass)                                    passes
nested in object same (should pass)                                    passes
subclass vs base same name/msg (should pass)                           passes
AbortSignal.abort().reason vs equal DOMException (should pass)         passes
DOMException vs {} (should throw)                                      THROWS
{} vs DOMException (should throw)                                      THROWS
DOMException vs {name, message} object (should throw)                  THROWS
DOMException vs Error same msg (should throw)                          THROWS
Error vs DOMException same msg (should throw)                          THROWS
expected extra own prop (should throw)                                 THROWS

Bun with the fix and node agree on every line.

[robobun]

Status

Reproduced on main (83d65fa) and release 1.4.0: assert.partialDeepStrictEqual(new DOMException("boom", "AbortError"), new DOMException("other", "NotFoundError")) does not throw; Node v26.3.0 throws ERR_ASSERTION.

Fix in src/jsc/modules/NodeUtilTypesModule.cpp: a DOMException arm in PartialDeepEqual::compareBranch that reuses errorSubset, plus (from review) errorSubset now reads cause the way it reads message/name/errors, matching node for plain Errors too. The sibling deepStrictEqual / isDeepStrictEqual blind spot lives in Bun__deepEquals and is handled in #39308.

Proof: test/js/node/assert/assert.test.cjs. On the unfixed binary 14 cases in the new DOMException block and 3 in the new error cause block fail; with the fix the file passes (58/58), as does test/js/node/assert/ plus the vendored node assert tests (486/486). Every expectation was checked against Node v26.3.0.

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 8 minutes

Limit details: You’ve used all 5 included reviews currently available under your plan.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: b52616ac-d8a0-45a3-8c55-36dd385767c8

📥 Commits

Reviewing files that changed from the base of the PR and between a23762f and ea227a1.

📒 Files selected for processing (2)

• src/jsc/modules/NodeUtilTypesModule.cpp
• test/js/node/assert/assert.test.cjs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 51b52b40-21b2-4bcc-98be-c4bd4a52caac

📥 Commits

Reviewing files that changed from the base of the PR and between 83d65fa and a23762f.

📒 Files selected for processing (2)

• src/jsc/modules/NodeUtilTypesModule.cpp
• test/js/node/assert/assert.test.cjs

Included review availability: Your plan includes up to 5 reviews per rolling hour; 0 remain after this review.

---

Walkthrough

The partial deep equality implementation now handles DOMException values separately from regular errors. New tests cover property matching, nested values, circular causes, subclasses, arrays, and AbortSignal reasons.

Changes

DOMException partial equality

Layer / File(s)	Summary
DOMException comparison handling src/jsc/modules/NodeUtilTypesModule.cpp	The comparison logic detects DOMException operands, rejects mixed DOMException and Error comparisons, and applies cycle-guarded error-subset comparison to two DOMException values.
DOMException assertion coverage test/js/node/assert/assert.test.cjs	Parameterized tests cover accepted and rejected comparisons across names, messages, causes, enumerable properties, subclasses, nested values, arrays, circular references, AbortSignal reasons, and identical instances.

Suggested reviewers: cirospaciari

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and concisely identifies the primary change to DOMException comparison in assert.partialDeepStrictEqual.
Description check	✅ Passed	The description explains the problem, implementation, scope, and verification results in sufficient detail, although it does not use the template headings.

---

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

[robobun]

^{Updated 11:08 PM PT - Aug 15th, 2026}

⏳ @robobun, your commit ea227a1 is still building in Build #99199, but has 2 failures so far (All Failures):

• test/js/bun/cron/cron.test.ts - code 1 on 🐧 25.04 x64
• test/bake/deinitialization.test.ts - pid 7484 segmentation fault at address 0x0 on 🪟 11 aarch64

[claude]

LGTM — the new compareBranch arm follows the adjacent URL/KeyObject/Error arms exactly and reuses errorSubset unchanged. Both inline notes describe pre-existing behavior on main (the cause: undefined edge case in errorSubset, and the sibling Bun__deepEquals blind spot in bindings.cpp); neither is introduced or worsened here, so they're fine as follow-ups.

Extended reasoning...

Overview

Adds a 12-line DOMException arm to PartialDeepEqual::compareBranch in src/jsc/modules/NodeUtilTypesModule.cpp, plus a JSDOMException.h include. The arm mirrors the existing URL/KeyObject/Error arms byte-for-byte in shape (inherits<> on both sides → both-or-neither → withCycleGuard + existing helper) and routes to the unmodified errorSubset. The test file gains a 35-case test.each matrix covering name/message/cause differences, subclass, nesting, AbortSignal reasons, mixed-type pairs, own props, and cycle handling — all verified against Node v26 per the PR description, with 14 cases confirmed failing on the unfixed binary.

Security risks

None. This is a pure comparison function in node:assert; no I/O, no allocation beyond what errorSubset already does, no user-controlled paths or shell/network surface. inherits<WebCore::JSDOMException>() is a non-throwing ClassInfo walk, so no new exception-check obligations are introduced before the withCycleGuard call (which handles its own).

Level of scrutiny

Low-to-medium. The native change is mechanical — it copies the exact pattern of the four arms immediately surrounding it, reuses an existing helper without modification, and the placement rationale (ahead of and separate from the ErrorInstance arm so DOMException-vs-Error is rejected, matching Node's tag check) is documented in the code comment and PR body. The test matrix is unusually thorough for a compat fix and pins both the newly-fixed and already-correct behavior.

Other factors

The two inline findings are both pre-existing on main: (1) errorSubset's cause: undefined handling diverges from Node's kPartial for the ErrorInstance arm too — this PR doesn't touch errorSubset; (2) the sibling assert.deepStrictEqual / util.isDeepStrictEqual path in Bun__deepEquals (bindings.cpp) has the same DOMException blind spot — that fix lives in a large shared template also backing Bun.deepEquals and jest matchers, so scoping this PR to compareBranch is reasonable. Neither is a regression; the PR is a strict improvement. The author ran the change under BUN_JSC_validateExceptionChecks=1 and the full test/js/node/assert/ suite plus vendored Node assert tests.

[claude]

LGTM — both prior review points are addressed (the cause handling in errorSubset now matches Node's structure, and the sibling deepStrictEqual blind spot is documented as deferred to #39308).

What was reviewed:

• The new DOMException arm mirrors the adjacent URL/KeyObject arms exactly; inherits<>() is non-throwing so no exception check is needed there, and placement ahead of the Error arm preserves the DOMException-vs-Error rejection.
• The errorSubset refactor: traced the loop's [[Get]] on cause for both Error (own non-enumerable data property) and DOMException (own non-enumerable via constructor at JSDOMException.cpp:165) — both are read correctly; vm.propertyNames->cause is a valid CommonIdentifier used elsewhere in bindings.
• Test matrix covers the cause edge cases from the earlier review (defined actual vs undefined expected, inherited getters) plus mixed-type pairs, subclasses, nesting, and cycles.

Extended reasoning...

Overview

Two files: src/jsc/modules/NodeUtilTypesModule.cpp (adds a JSDOMException arm to PartialDeepEqual::compareBranch and restructures errorSubset's cause handling) and test/js/node/assert/assert.test.cjs (~50 new test cases across two test.each blocks). Net C++ logic change is ~15 lines.

Security risks

None. This is assert.partialDeepStrictEqual, a Node-compat testing/assertion API. No untrusted-input parsing, no allocation sizing, no auth/crypto surface. The new arm only calls inherits<>() (non-throwing type check) and reuses the existing errorSubset/withCycleGuard machinery.

Level of scrutiny

Low-to-medium. It's native C++ in the JSC bindings, so exception-scope discipline matters — but the change follows the exact pattern of the neighboring URL and KeyObject arms (check both sides' type, require both, delegate to a shared body via withCycleGuard). The errorSubset refactor moves cause into the existing property loop and strips the redundant compare from the own-property block; every get() and getOwnPropertySlot retains its RETURN_IF_EXCEPTION. The PR description states it was run under BUN_JSC_validateExceptionChecks=1.

Other factors

This is my second pass. In the first, I flagged (a) the sibling Bun__deepEquals blind spot — author responded it's a separate implementation handled in #39308 and updated the PR description accordingly, which satisfies REVIEW.md's "if a site is intentionally excluded, say so"; and (b) a pre-existing cause: undefined divergence in errorSubset — author fixed it in d8dc16c and added the error cause test block (3 cases fail on the unfixed binary per the description). The comment-cop nag about the multi-line arm comment was also addressed (collapsed to one line). All threads are resolved.

Test coverage is thorough: name/message/cause variations, empty message leniency, subclass instances, nested in objects/arrays, mixed DOMException-vs-Error/plain-object pairs in both directions, own enumerable extras, AbortSignal.abort().reason, self-referencing causes, and same-instance identity. Assertions use toThrow(assert.AssertionError) (specific class, not bare toThrow()). The description states 14 DOMException cases and 3 cause cases fail under USE_SYSTEM_BUN=1 and all 58 pass with the fix, plus the vendored Node assert suites (486 pass).

I verified vm.propertyNames->cause is a real CommonIdentifier (used in bindings.cpp, ErrorCode.cpp, JSDOMException.cpp) and that Bun's DOMException constructor stores cause as an own non-enumerable property (JSDOMException.cpp:165), so the errorSubset get() and getOwnPropertySlot paths both see it as intended.