Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: expose DOMException to internalBinding('message') for testing #28072

Closed
wants to merge 1 commit into from
Closed

src: expose DOMException to internalBinding('message') for testing #28072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions src/node_messaging.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,27 +186,30 @@ uint32_t Message::AddWASMModule(WasmModuleObject::TransferrableModule&& mod) {

namespace {

void ThrowDataCloneException(Local<Context> context, Local<String> message) {
MaybeLocal<Function> GetDOMException(Local<Context> context) {
Isolate* isolate = context->GetIsolate();
Local<Value> argv[] = {
message,
FIXED_ONE_BYTE_STRING(isolate, "DataCloneError")
};
Local<Value> exception;

Local<Object> per_context_bindings;
Local<Value> domexception_ctor_val;
if (!GetPerContextExports(context).ToLocal(&per_context_bindings) ||
!per_context_bindings->Get(context,
FIXED_ONE_BYTE_STRING(isolate, "DOMException"))
.ToLocal(&domexception_ctor_val)) {
return;
return MaybeLocal<Function>();
}

CHECK(domexception_ctor_val->IsFunction());
Local<Function> domexception_ctor = domexception_ctor_val.As<Function>();
if (!domexception_ctor->NewInstance(context, arraysize(argv), argv)
.ToLocal(&exception)) {
return domexception_ctor;
}

void ThrowDataCloneException(Local<Context> context, Local<String> message) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@addaleax I think ideally we'd want to do this in JS instead of throwing the error in nested C++ helpers?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Isolate* isolate = context->GetIsolate();
Local<Value> argv[] = {message,
FIXED_ONE_BYTE_STRING(isolate, "DataCloneError")};
Local<Value> exception;
Local<Function> domexception_ctor;
if (!GetDOMException(context).ToLocal(&domexception_ctor) ||
!domexception_ctor->NewInstance(context, arraysize(argv), argv)
.ToLocal(&exception)) {
return;
}
isolate->ThrowException(exception);
Expand Down Expand Up @@ -900,6 +903,15 @@ static void InitMessaging(Local<Object> target,
env->SetMethod(target, "receiveMessageOnPort", MessagePort::ReceiveMessage);
env->SetMethod(target, "moveMessagePortToContext",
MessagePort::MoveToContext);

{
Local<Function> domexception = GetDOMException(context).ToLocalChecked();
target
->Set(context,
FIXED_ONE_BYTE_STRING(env->isolate(), "DOMException"),
domexception)
.Check();
}
}

} // anonymous namespace
Expand Down
17 changes: 2 additions & 15 deletions test/wpt/test-url.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,29 +3,16 @@
// Flags: --expose-internals

require('../common');
const assert = require('assert');
const { WPTRunner } = require('../common/wpt');

const { internalBinding } = require('internal/test/binding');
const { DOMException } = internalBinding('messaging');
const runner = new WPTRunner('url');

// Copy global descriptors from the global object
runner.copyGlobalsFromObject(global, ['URL', 'URLSearchParams']);
// Needed by urlsearchparams-constructor.any.js
let DOMException;
runner.defineGlobal('DOMException', {
get() {
// A 'hack' to get the DOMException constructor since we don't have it
// on the global object.
if (DOMException === undefined) {
const port = new (require('worker_threads').MessagePort)();
const ab = new ArrayBuffer(1);
try {
port.postMessage(ab, [ab, ab]);
} catch (err) {
DOMException = err.constructor;
}
assert.strictEqual(DOMException.name, 'DOMException');
}
return DOMException;
}
});
Expand Down