src: expose DOMException to internalBinding('message') for testing#28072
src: expose DOMException to internalBinding('message') for testing#28072joyeecheung wants to merge 1 commit intonodejs:masterfrom
Conversation
Instead of using a hack to get it in the test.
| return domexception_ctor; | ||
| } | ||
|
|
||
| void ThrowDataCloneException(Local<Context> context, Local<String> message) { |
There was a problem hiding this comment.
@addaleax I think ideally we'd want to do this in JS instead of throwing the error in nested C++ helpers?
There was a problem hiding this comment.
@joyeecheung We need to have this in C++, because the V8 ValueSerializer API requires it. I also don’t see any reason to move the error throwing to JS?
There was a problem hiding this comment.
I see, this is required by ValueSerializer::Delegate::ThrowDataCloneError - but do we need to throw errors this way ourselves? As this is just doing new DOMException(message, 'DataCloneError') - I believe in general we prefer throwing errors from JS.
| return domexception_ctor; | ||
| } | ||
|
|
||
| void ThrowDataCloneException(Local<Context> context, Local<String> message) { |
There was a problem hiding this comment.
@joyeecheung We need to have this in C++, because the V8 ValueSerializer API requires it. I also don’t see any reason to move the error throwing to JS?
addaleax
added
test
joyeecheung
added
author ready
|
@nodejs/testing This needs one more approval to land |
|
Landed in 890223d, thanks for taking care of this! |
Instead of using a hack to get it in the test. PR-URL: nodejs#28072 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
Instead of using a hack to get it in the test. PR-URL: #28072 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com>
4 participants
Instead of using a hack to get it in the test.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes