node icon indicating copy to clipboard operation
node copied to clipboard

Add a `[[ErrorData]]` internal slot to DOMException

Open petamoriken opened this issue 8 months ago • 1 comments

Fixes #56497

Error.isError, implemented in V8 13.6 (#58070), must return true for DOMException.

petamoriken avatar May 03 '25 12:05 petamoriken

Maybe a better way to fix this is to call isolate->SetJSApiWrapperNativeErrorCallback(IsNodeError) with IsNodeError looking something like this:

bool IsNodeError(Isolate* isolate, Local<Object> obj) {
  return IsDOMException(isolate, obj); // implementation of IsDOMException left as an exercise to the reader
}

Probably means implementing DOMException as an ObjectTemplate/FunctionTemplate in C++ land.

bnoordhuis avatar May 04 '25 09:05 bnoordhuis

IMO, this PR should be merged before Node.js v24 is released. How about that?

petamoriken avatar May 06 '25 11:05 petamoriken

Like @bnoordhuis suggested, for IsDOMException, we can use the private symbol is_dom_exception introduced in https://github.com/nodejs/node/pull/57372 to fast check if an object is a dom exception.

legendecas avatar May 12 '25 13:05 legendecas

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.10%. Comparing base (5d3e1b5) to head (d342ccd). Report is 112 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #58138      +/-   ##
==========================================
+ Coverage   89.64%   90.10%   +0.45%     
==========================================
  Files         630      630              
  Lines      186470   186623     +153     
  Branches    36305    36628     +323     
==========================================
+ Hits       167160   168151     +991     
+ Misses      12073    11252     -821     
+ Partials     7237     7220      -17     
Files with missing lines Coverage Δ
lib/internal/per_context/domexception.js 77.57% <100.00%> (+1.75%) :arrow_up:

... and 101 files with indirect coverage changes

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar May 12 '25 16:05 codecov[bot]

It appears that we should wait for #57372.

petamoriken avatar May 15 '25 15:05 petamoriken