lib: make domexception a native error by legendecas · Pull Request #58691 · nodejs/node

legendecas · 2025-06-12T10:26:20Z

deps: V8: cherry-pick e3df60f3f5ab

Original commit message:

[objects] allow host defined serializer of JSError

Allow host defined serializer and deserializer of JSError in
ValueSerializer API. This allows hosts that implement DOMException
in JS to support `Error.isError` proposal and `structuredClone`.

Refs: https://github.com/nodejs/node/pull/58691
Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net>
Cr-Commit-Position: refs/heads/main@{#100894}

Refs: v8/v8@e3df60f

lib: make domexception a native error

isolate->SetJSApiWrapperNativeErrorCallback(IsNodeError) can not be used because V8 only invokes the callback for native objects:

node/deps/v8/src/builtins/builtins-error.cc

Lines 72 to 74 in 708477b

    
           IsJSErrorMap(obj_map) || 
        
           (IsJSApiWrapperObjectMap(obj_map) && 
        
            isolate->IsJSApiWrapperNativeError(Cast<JSReceiver>(obj))));

Setting this callback also requires embedders like electron to decide if they need to override blink's callback. So it is simpler to implement this in JS only.

Refs: #58138
Fixes: #56497

nodejs-github-bot · 2025-06-12T10:26:25Z

Review requested:

@nodejs/web-standards
@nodejs/v8-update
@nodejs/gyp

codecov · 2025-06-12T11:16:45Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.10%. Comparing base (5584cc5) to head (27dfaaa).
⚠️ Report is 636 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58691      +/-   ##
==========================================
- Coverage   90.13%   90.10%   -0.04%     
==========================================
  Files         639      639              
  Lines      188192   188209      +17     
  Branches    36916    36906      -10     
==========================================
- Hits       169633   169580      -53     
- Misses      11324    11369      +45     
- Partials     7235     7260      +25

Files with missing lines	Coverage Δ
lib/internal/per_context/domexception.js	`82.43% <100.00%> (+1.58%)`	⬆️

... and 41 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

legendecas · 2025-06-12T12:40:26Z

Fixing structuredClone support in https://chromium-review.googlesource.com/c/v8/v8/+/6637876.

lib/internal/per_context/domexception.js

Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: nodejs/node#58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894}

Original commit message: [objects] allow host defined serializer of JSError Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: nodejs#58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894} Refs: v8/v8@e3df60f

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com>

nodejs-github-bot · 2025-06-19T10:58:35Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67528/

nodejs-github-bot · 2025-06-19T12:16:37Z

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=benchmark-ubuntu2204-intel-64,v8test=v8test/6618/

nodejs-github-bot · 2025-06-19T12:18:36Z

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-ppc64le,v8test=v8test/6618/

nodejs-github-bot · 2025-06-19T12:19:38Z

V8 CI: https://ci.nodejs.org/job/node-test-commit-v8-linux/nodes=rhel8-s390x,v8test=v8test/6618/

nodejs-github-bot · 2025-06-19T20:32:59Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67542/

nodejs-github-bot · 2025-06-20T10:16:51Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67560/

jazelly

was following this. lgtm

nodejs-github-bot · 2025-06-20T12:12:57Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67566/

mcollina

lgtm

nodejs-github-bot · 2025-06-20T15:44:48Z

CI: https://ci.nodejs.org/job/node-test-pull-request/67572/

nodejs-github-bot · 2025-06-21T10:43:39Z

Landed in 11222f1...e679e38

Original commit message: [objects] allow host defined serializer of JSError Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: #58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894} Refs: v8/v8@e3df60f PR-URL: #58691 Fixes: #56497 Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com> PR-URL: #58691 Fixes: #56497 Refs: v8/v8@e3df60f Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

alexsch01 · 2025-06-21T11:40:33Z

Would this qualify for a backport in NodeJS 24?

lib/internal/per_context/domexception.js

Original commit message: [objects] allow host defined serializer of JSError Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: #58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894} Refs: v8/v8@e3df60f PR-URL: #58691 Fixes: #56497 Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com> PR-URL: #58691 Fixes: #56497 Refs: v8/v8@e3df60f Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs/node#58691

aduh95 · 2025-07-21T17:54:40Z

We're getting a test failure with this change on v22.x-staging:

=== release test-structuredClone-domexception ===                   
Path: parallel/test-structuredClone-domexception
node:assert:95
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

false !== true

    at assertDOMException (…/test/parallel/test-structuredClone-domexception.js:7:10)
    at Object.<anonymous> (…/test/parallel/test-structuredClone-domexception.js:19:3)
    at Module._compile (node:internal/modules/cjs/loader:1688:14)

node/test/parallel/test-structuredClone-domexception.js

Line 7 in 5457443

assert.strictEqual(actual instanceof DOMException, true);

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com> PR-URL: nodejs#58691 Fixes: nodejs#56497 Refs: v8/v8@e3df60f Refs: nodejs#58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

Original commit message: [objects] allow host defined serializer of JSError Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: nodejs#58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894} Refs: v8/v8@e3df60f PR-URL: nodejs#58691 Backport-PR-URL: nodejs#59957 Fixes: nodejs#56497 Refs: nodejs#58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com> PR-URL: nodejs#58691 Backport-PR-URL: nodejs#59957 Fixes: nodejs#56497 Refs: v8/v8@e3df60f Refs: nodejs#58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

legendecas · 2025-09-21T14:51:07Z

Backport open at #59957.

Original commit message: [objects] allow host defined serializer of JSError Allow host defined serializer and deserializer of JSError in ValueSerializer API. This allows hosts that implement DOMException in JS to support `Error.isError` proposal and `structuredClone`. Refs: #58691 Change-Id: I022821c9abd659970c4d449b3c69c5fb54d0618a Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/6637876 Reviewed-by: Camillo Bruni <cbruni@chromium.org> Commit-Queue: Chengzhong Wu <cwu631@bloomberg.net> Cr-Commit-Position: refs/heads/main@{#100894} Refs: v8/v8@e3df60f PR-URL: #58691 Backport-PR-URL: #59957 Fixes: #56497 Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com> PR-URL: #58691 Backport-PR-URL: #59957 Fixes: #56497 Refs: v8/v8@e3df60f Refs: #58138 Reviewed-By: Jason Zhang <xzha4350@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Ethan Arrowood <ethan@arrowood.dev> Reviewed-By: James M Snell <jasnell@gmail.com>

nodejs-github-bot added the needs-ci PRs that need a full CI run. label Jun 12, 2025

legendecas marked this pull request as draft June 12, 2025 10:51

jasnell reviewed Jun 12, 2025

View reviewed changes

lib/internal/per_context/domexception.js Show resolved Hide resolved

legendecas force-pushed the dom-exception-iserror branch 2 times, most recently from 40c22d3 to b8d3f25 Compare June 12, 2025 16:46

legendecas and others added 2 commits June 19, 2025 11:54

lib: make domexception a native error

27dfaaa

Co-Authored-By: Kenta Moriuchi <moriken@kimamass.com>

legendecas force-pushed the dom-exception-iserror branch from b8d3f25 to 27dfaaa Compare June 19, 2025 10:57

legendecas marked this pull request as ready for review June 19, 2025 10:57

jazelly approved these changes Jun 20, 2025

View reviewed changes

legendecas added commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 20, 2025

mcollina approved these changes Jun 20, 2025

View reviewed changes

RafaelGSS approved these changes Jun 20, 2025

View reviewed changes

Ethan-Arrowood approved these changes Jun 20, 2025

View reviewed changes

jasnell approved these changes Jun 20, 2025

View reviewed changes

legendecas added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 21, 2025

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 21, 2025

nodejs-github-bot closed this Jun 21, 2025

legendecas deleted the dom-exception-iserror branch June 21, 2025 10:53

ExE-Boss reviewed Jun 21, 2025

View reviewed changes

lib/internal/per_context/domexception.js Show resolved Hide resolved

This was referenced Jun 23, 2025

2025-06-24, Version 24.3.0 (Current) #58804

Closed

2025-06-24, Version 24.3.0 (Current) #58813

Merged

zloirock added a commit to zloirock/core-js that referenced this pull request Jun 25, 2025

mark Error.isError as fixed in Node 24.3

4db6a6c

nodejs/node#58691

aduh95 added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Jul 21, 2025

Renegade334 mentioned this pull request Sep 11, 2025

DOMException message not printed in node test runner #59852

Closed

legendecas added backport-open-v22.x Indicate that the PR has an open backport and removed backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. labels Sep 21, 2025

github-actions bot mentioned this pull request Sep 22, 2025

2025-09-24, Version 22.20.0 'Jod' (LTS) #59973

Merged

richardlau added backported-to-v22.x PRs backported to the v22.x-staging branch. and removed backport-open-v22.x Indicate that the PR has an open backport labels Sep 24, 2025

	IsJSErrorMap(obj_map) \|\|
	(IsJSApiWrapperObjectMap(obj_map) &&
	isolate->IsJSApiWrapperNativeError(Cast<JSReceiver>(obj))));

Uh oh!

Conversation

legendecas commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

deps: V8: cherry-pick e3df60f3f5ab

lib: make domexception a native error

Uh oh!

nodejs-github-bot commented Jun 12, 2025 • edited by legendecas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

legendecas commented Jun 12, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jun 19, 2025

Uh oh!

nodejs-github-bot commented Jun 19, 2025

Uh oh!

nodejs-github-bot commented Jun 19, 2025

Uh oh!

nodejs-github-bot commented Jun 19, 2025

Uh oh!

nodejs-github-bot commented Jun 19, 2025

Uh oh!

nodejs-github-bot commented Jun 20, 2025

Uh oh!

jazelly left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 20, 2025

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jun 20, 2025

Uh oh!

nodejs-github-bot commented Jun 21, 2025

Uh oh!

alexsch01 commented Jun 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

aduh95 commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

legendecas commented Sep 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

legendecas commented Jun 12, 2025 •

edited

Loading

nodejs-github-bot commented Jun 12, 2025 •

edited by legendecas

Loading

codecov bot commented Jun 12, 2025 •

edited

Loading

alexsch01 commented Jun 21, 2025 •

edited

Loading

aduh95 commented Jul 21, 2025 •

edited

Loading