diff --git a/lib/.eslintrc.yaml b/lib/.eslintrc.yaml index 6bfc102b817cfe..f6922779663028 100644 --- a/lib/.eslintrc.yaml +++ b/lib/.eslintrc.yaml @@ -239,3 +239,4 @@ globals: module: false internalBinding: false primordials: false + JSTransferable: false diff --git a/lib/internal/per_context/domexception.js b/lib/internal/per_context/domexception.js index 1dd7a2a7f5a819..051e2d0c7d6153 100644 --- a/lib/internal/per_context/domexception.js +++ b/lib/internal/per_context/domexception.js @@ -3,16 +3,20 @@ const { ErrorCaptureStackTrace, ErrorPrototype, + FunctionPrototypeCall, ObjectDefineProperties, ObjectDefineProperty, ObjectSetPrototypeOf, - SafeWeakMap, SafeMap, SafeSet, + SafeWeakMap, SymbolToStringTag, TypeError, } = primordials; +const kClone = 'nodejs.worker_threads.clone'; +const kDeserialize = 'nodejs.worker_threads.deserialize'; + function throwInvalidThisError(Base, type) { const err = new Base(); const key = 'ERR_INVALID_THIS'; @@ -46,13 +50,23 @@ const disusedNamesSet = new SafeSet() .add('NoDataAllowedError') .add('ValidationError'); -class DOMException { +class DOMException extends JSTransferable { constructor(message = '', name = 'Error') { - ErrorCaptureStackTrace(this); - internalsMap.set(this, { - message: `${message}`, - name: `${name}` - }); + super(); + + let code; + if (disusedNamesSet.has(name)) { + code = 0; + } else { + code = nameToCodeMap.get(name); + code = code === undefined ? 0 : code; + } + + const o = { __proto__: null }; + ErrorCaptureStackTrace(o); + FunctionPrototypeCall(this[kDeserialize], + this, + { name, message, code, stack: o.stack }); } get name() { @@ -76,13 +90,32 @@ class DOMException { if (internals === undefined) { throwInvalidThisError(TypeError, 'DOMException'); } + return internals.code; + } - if (disusedNamesSet.has(internals.name)) { - return 0; + get stack() { + const internals = internalsMap.get(this); + if (internals === undefined) { + throwInvalidThisError(TypeError, 'DOMException'); } + return internals.stack; + } - const code = nameToCodeMap.get(internals.name); - return code === undefined ? 0 : code; + [kClone]() { + const { name, message, code, stack } = this; + return { + data: { name, message, code, stack }, + deserializeInfo: 'internal/per_context/domexception:DOMException', + }; + } + + [kDeserialize]({ name, message, code, stack }) { + internalsMap.set(this, { + name: `${name}`, + message: `${message}`, + code, + stack: `${stack}`, + }); } } @@ -91,7 +124,8 @@ ObjectDefineProperties(DOMException.prototype, { [SymbolToStringTag]: { configurable: true, value: 'DOMException' }, name: { enumerable: true, configurable: true }, message: { enumerable: true, configurable: true }, - code: { enumerable: true, configurable: true } + code: { enumerable: true, configurable: true }, + stack: { enumerable: true, configurable: true } }); for (const { 0: name, 1: codeName, 2: value } of [ diff --git a/lib/internal/webstreams/transfer.js b/lib/internal/webstreams/transfer.js index 985d7e86738f35..39b4e35689d1ac 100644 --- a/lib/internal/webstreams/transfer.js +++ b/lib/internal/webstreams/transfer.js @@ -1,9 +1,7 @@ 'use strict'; const { - ObjectDefineProperties, PromiseResolve, - ReflectConstruct, } = primordials; const { @@ -34,72 +32,6 @@ const { const assert = require('internal/assert'); -const { - makeTransferable, - kClone, - kDeserialize, -} = require('internal/worker/js_transferable'); - -// This class is a bit of a hack. The Node.js implementation of -// DOMException is not transferable/cloneable. This provides us -// with a variant that is. Unfortunately, it means playing around -// a bit with the message, name, and code properties and the -// prototype. We can revisit this if DOMException is ever made -// properly cloneable. -class CloneableDOMException extends DOMException { - constructor(message, name) { - super(message, name); - this[kDeserialize]({ - message: this.message, - name: this.name, - code: this.code, - }); - // eslint-disable-next-line no-constructor-return - return makeTransferable(this); - } - - [kClone]() { - return { - data: { - message: this.message, - name: this.name, - code: this.code, - }, - deserializeInfo: - 'internal/webstreams/transfer:InternalCloneableDOMException' - }; - } - - [kDeserialize]({ message, name, code }) { - ObjectDefineProperties(this, { - message: { - configurable: true, - enumerable: true, - get() { return message; }, - }, - name: { - configurable: true, - enumerable: true, - get() { return name; }, - }, - code: { - configurable: true, - enumerable: true, - get() { return code; }, - }, - }); - } -} - -function InternalCloneableDOMException() { - return makeTransferable( - ReflectConstruct( - CloneableDOMException, - [], - DOMException)); -} -InternalCloneableDOMException[kDeserialize] = () => {}; - class CrossRealmTransformReadableSource { constructor(port) { this[kState] = { @@ -133,7 +65,7 @@ class CrossRealmTransformReadableSource { }; port.onmessageerror = () => { - const error = new CloneableDOMException( + const error = new DOMException( 'Internal transferred ReadableStream error', 'DataCloneError'); port.postMessage({ type: 'error', value: error }); @@ -156,10 +88,6 @@ class CrossRealmTransformReadableSource { try { this[kState].port.postMessage({ type: 'error', value: reason }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); throw error; } finally { @@ -200,7 +128,7 @@ class CrossRealmTransformWritableSink { } }; port.onmessageerror = () => { - const error = new CloneableDOMException( + const error = new DOMException( 'Internal transferred ReadableStream error', 'DataCloneError'); port.postMessage({ type: 'error', value: error }); @@ -229,10 +157,6 @@ class CrossRealmTransformWritableSink { try { this[kState].port.postMessage({ type: 'chunk', value: chunk }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); this[kState].port.close(); throw error; @@ -248,10 +172,6 @@ class CrossRealmTransformWritableSink { try { this[kState].port.postMessage({ type: 'error', value: reason }); } catch (error) { - if (error instanceof DOMException) { - // eslint-disable-next-line no-ex-assign - error = new CloneableDOMException(error.message, error.name); - } this[kState].port.postMessage({ type: 'error', value: error }); throw error; } finally { @@ -294,6 +214,4 @@ module.exports = { newCrossRealmWritableSink, CrossRealmTransformWritableSink, CrossRealmTransformReadableSource, - CloneableDOMException, - InternalCloneableDOMException, }; diff --git a/lib/internal/worker/js_transferable.js b/lib/internal/worker/js_transferable.js index 7bd6c8cafc32e2..58cf89fd49806a 100644 --- a/lib/internal/worker/js_transferable.js +++ b/lib/internal/worker/js_transferable.js @@ -9,9 +9,7 @@ const { StringPrototypeSplit, } = primordials; const { - messaging_deserialize_symbol, messaging_transfer_symbol, - messaging_clone_symbol, messaging_transfer_list_symbol } = internalBinding('symbols'); const { @@ -19,6 +17,9 @@ const { setDeserializerCreateObjectFunction } = internalBinding('messaging'); +const kClone = 'nodejs.worker_threads.clone'; +const kDeserialize = 'nodejs.worker_threads.deserialize'; + function setup() { // Register the handler that will be used when deserializing JS-based objects // from .postMessage() calls. The format of `deserializeInfo` is generally @@ -27,7 +28,7 @@ function setup() { const { 0: module, 1: ctor } = StringPrototypeSplit(deserializeInfo, ':'); const Ctor = require(module)[ctor]; if (typeof Ctor !== 'function' || - typeof Ctor.prototype[messaging_deserialize_symbol] !== 'function') { + typeof Ctor.prototype[kDeserialize] !== 'function') { // Not one of the official errors because one should not be able to get // here without messing with Node.js internals. // eslint-disable-next-line no-restricted-syntax @@ -49,8 +50,8 @@ module.exports = { makeTransferable, setup, JSTransferable, - kClone: messaging_clone_symbol, - kDeserialize: messaging_deserialize_symbol, + kClone, + kDeserialize, kTransfer: messaging_transfer_symbol, kTransferList: messaging_transfer_list_symbol, }; diff --git a/src/api/environment.cc b/src/api/environment.cc index ccdcdefb20fe84..446ae9f315b23c 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -2,6 +2,7 @@ #include "node_context_data.h" #include "node_errors.h" #include "node_internals.h" +#include "node_messaging.h" #include "node_native_module_env.h" #include "node_options-inl.h" #include "node_platform.h" @@ -681,6 +682,8 @@ Maybe InitializePrimordials(Local context) { FIXED_ONE_BYTE_STRING(isolate, "primordials"); Local global_string = FIXED_ONE_BYTE_STRING(isolate, "global"); Local exports_string = FIXED_ONE_BYTE_STRING(isolate, "exports"); + Local js_transferable_string = + FIXED_ONE_BYTE_STRING(isolate, "JSTransferable"); // Create primordials first and make it available to per-context scripts. Local primordials = Object::New(isolate); @@ -696,9 +699,15 @@ Maybe InitializePrimordials(Local context) { nullptr}; for (const char** module = context_files; *module != nullptr; module++) { - std::vector> parameters = { - global_string, exports_string, primordials_string}; - Local arguments[] = {context->Global(), exports, primordials}; + std::vector> parameters = {global_string, + exports_string, + primordials_string, + js_transferable_string}; + Local arguments[] = { + context->Global(), + exports, + primordials, + worker::JSTransferable::GetConstructorFunction(isolate)}; MaybeLocal maybe_fn = native_module::NativeModuleEnv::LookupAndCompile( context, *module, ¶meters, nullptr); diff --git a/src/base_object.h b/src/base_object.h index 1c63da92fd80c0..9884caebe4557a 100644 --- a/src/base_object.h +++ b/src/base_object.h @@ -112,6 +112,9 @@ class BaseObject : public MemoryRetainer { static v8::Local GetConstructorTemplate( Environment* env); + static v8::Local GetConstructorTemplate( + v8::Isolate* isolate); + // Interface for transferring BaseObject instances using the .postMessage() // method of MessagePorts (and, by extension, Workers). // GetTransferMode() returns a transfer mode that indicates how to deal with diff --git a/src/env-inl.h b/src/env-inl.h index 46feb9bfa4ca93..298eb9470ef6bb 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -1289,9 +1289,19 @@ void Environment::set_process_exit_handler( ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) #undef V -v8::Local Environment::context() const { - return PersistentToLocal::Strong(context_); -} +#define V(PropertyName, TypeName) \ + inline v8::Local Environment::PropertyName() const { \ + return get_##PropertyName(isolate()); \ + } \ + inline void Environment::set_##PropertyName(v8::Local value) { \ + set_##PropertyName(isolate(), value); \ + } + PER_ISOLATE_ETERNALS(V) +#undef V + + v8::Local Environment::context() const { + return PersistentToLocal::Strong(context_); + } } // namespace node diff --git a/src/env.cc b/src/env.cc index 3c07e9342fd338..ac39c24f3183ba 100644 --- a/src/env.cc +++ b/src/env.cc @@ -62,6 +62,22 @@ int const Environment::kNodeContextTag = 0x6e6f64; void* const Environment::kNodeContextTagPtr = const_cast( static_cast(&Environment::kNodeContextTag)); +#define V(PropertyName, TypeName) \ + static v8::Eternal PropertyName##_eternal; +PER_ISOLATE_ETERNALS(V) +#undef V + +#define V(PropertyName, TypeName) \ + Local Environment::get_##PropertyName(Isolate* isolate) { \ + return PropertyName##_eternal.Get(isolate); \ + } \ + void Environment::set_##PropertyName(Isolate* isolate, \ + Local value) { \ + PropertyName##_eternal.Set(isolate, value); \ + } +PER_ISOLATE_ETERNALS(V) +#undef V + std::vector IsolateData::Serialize(SnapshotCreator* creator) { Isolate* isolate = creator->GetIsolate(); std::vector indexes; @@ -1354,6 +1370,7 @@ EnvSerializeInfo Environment::Serialize(SnapshotCreator* creator) { id++; \ } while (0); ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) + PER_ISOLATE_ETERNALS(V) #undef V id = 0; @@ -1488,6 +1505,7 @@ void Environment::DeserializeProperties(const EnvSerializeInfo* info) { #define V(PropertyName, TypeName) SetProperty(PropertyName, TypeName, \ templates, template, isolate_) ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V); + PER_ISOLATE_ETERNALS(V) #undef V i = 0; // index to the array @@ -1736,11 +1754,23 @@ bool BaseObject::IsRootNode() const { } Local BaseObject::GetConstructorTemplate(Environment* env) { - Local tmpl = env->base_object_ctor_template(); + return GetConstructorTemplate(env->isolate()); +} + +Local BaseObject::GetConstructorTemplate(Isolate* isolate) { + Local tmpl = + Environment::get_base_object_ctor_template(isolate); if (tmpl.IsEmpty()) { - tmpl = env->NewFunctionTemplate(nullptr); - tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "BaseObject")); - env->set_base_object_ctor_template(tmpl); + tmpl = FunctionTemplate::New(isolate, + nullptr, + Local(), + Local(), + 0, + v8::ConstructorBehavior::kAllow, + v8::SideEffectType::kHasSideEffect, + nullptr); + tmpl->SetClassName(FIXED_ONE_BYTE_STRING(isolate, "BaseObject")); + Environment::set_base_object_ctor_template(isolate, tmpl); } return tmpl; } diff --git a/src/env.h b/src/env.h index 475ca4d8b7cd71..5bfa7bfc5476ba 100644 --- a/src/env.h +++ b/src/env.h @@ -168,15 +168,13 @@ constexpr size_t kFsStatsBufferLength = V(async_id_symbol, "async_id_symbol") \ V(handle_onclose_symbol, "handle_onclose") \ V(no_message_symbol, "no_message_symbol") \ - V(messaging_deserialize_symbol, "messaging_deserialize_symbol") \ V(messaging_transfer_symbol, "messaging_transfer_symbol") \ - V(messaging_clone_symbol, "messaging_clone_symbol") \ V(messaging_transfer_list_symbol, "messaging_transfer_list_symbol") \ V(oninit_symbol, "oninit") \ V(owner_symbol, "owner_symbol") \ V(onpskexchange_symbol, "onpskexchange") \ V(resource_symbol, "resource_symbol") \ - V(trigger_async_id_symbol, "trigger_async_id_symbol") \ + V(trigger_async_id_symbol, "trigger_async_id_symbol") // Strings are per-isolate primitives but Environment proxies them // for the sake of convenience. Strings should be ASCII-only. @@ -455,10 +453,12 @@ constexpr size_t kFsStatsBufferLength = V(x_forwarded_string, "x-forwarded-for") \ V(zero_return_string, "ZERO_RETURN") +#define PER_ISOLATE_ETERNALS(V) \ + V(base_object_ctor_template, v8::FunctionTemplate) + #define ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) \ V(async_wrap_ctor_template, v8::FunctionTemplate) \ V(async_wrap_object_ctor_template, v8::FunctionTemplate) \ - V(base_object_ctor_template, v8::FunctionTemplate) \ V(binding_data_ctor_template, v8::FunctionTemplate) \ V(blob_constructor_template, v8::FunctionTemplate) \ V(blocklist_constructor_template, v8::FunctionTemplate) \ @@ -1327,6 +1327,14 @@ class Environment : public MemoryRetainer { inline void set_ ## PropertyName(v8::Local value); ENVIRONMENT_STRONG_PERSISTENT_VALUES(V) ENVIRONMENT_STRONG_PERSISTENT_TEMPLATES(V) + PER_ISOLATE_ETERNALS(V) +#undef V + +#define V(PropertyName, TypeName) \ + static v8::Local get_##PropertyName(v8::Isolate* isolate); \ + static void set_##PropertyName(v8::Isolate* isolate, \ + v8::Local value); + PER_ISOLATE_ETERNALS(V) #undef V inline v8::Local context() const; diff --git a/src/node_messaging.cc b/src/node_messaging.cc index 6403950e9c8a96..dacef56c151cd5 100644 --- a/src/node_messaging.cc +++ b/src/node_messaging.cc @@ -1136,14 +1136,32 @@ void JSTransferable::New(const FunctionCallbackInfo& args) { new JSTransferable(Environment::GetCurrent(args), args.This()); } +Local JSTransferable::GetConstructorFunction(Isolate* isolate) { + Local tmpl = + FunctionTemplate::New(isolate, + New, + Local(), + Local(), + 0, + v8::ConstructorBehavior::kAllow, + v8::SideEffectType::kHasSideEffect, + nullptr); + tmpl->Inherit(BaseObject::GetConstructorTemplate(isolate)); + tmpl->InstanceTemplate()->SetInternalFieldCount(kInternalFieldCount); + return tmpl->GetFunction(isolate->GetCurrentContext()).ToLocalChecked(); +} + JSTransferable::TransferMode JSTransferable::GetTransferMode() const { // Implement `kClone in this ? kCloneable : kTransferable`. HandleScope handle_scope(env()->isolate()); errors::TryCatchScope ignore_exceptions(env()); bool has_clone; - if (!object()->Has(env()->context(), - env()->messaging_clone_symbol()).To(&has_clone)) { + + Local kClone = + FIXED_ONE_BYTE_STRING(env()->isolate(), "nodejs.worker_threads.clone"); + + if (!object()->Has(env()->context(), kClone).To(&has_clone)) { return TransferMode::kUntransferable; } @@ -1166,8 +1184,12 @@ std::unique_ptr JSTransferable::TransferOrClone( // on the `TransferData` instance as a string. HandleScope handle_scope(env()->isolate()); Local context = env()->isolate()->GetCurrentContext(); - Local method_name = mode == TransferMode::kCloneable ? - env()->messaging_clone_symbol() : env()->messaging_transfer_symbol(); + Local kClone = + FIXED_ONE_BYTE_STRING(env()->isolate(), "nodejs.worker_threads.clone"); + Local method_name = + mode == TransferMode::kCloneable + ? kClone.As() + : env()->messaging_transfer_symbol().As(); Local method; if (!object()->Get(context, method_name).ToLocal(&method)) { @@ -1242,7 +1264,8 @@ Maybe JSTransferable::FinalizeTransferRead( Local data; if (!deserializer->ReadValue(context).ToLocal(&data)) return Nothing(); - Local method_name = env()->messaging_deserialize_symbol(); + Local method_name = FIXED_ONE_BYTE_STRING( + env()->isolate(), "nodejs.worker_threads.deserialize"); Local method; if (!object()->Get(context, method_name).ToLocal(&method)) { return Nothing(); @@ -1459,13 +1482,11 @@ static void InitMessaging(Local target, env->NewFunctionTemplate(MessageChannel)); } - { - Local t = env->NewFunctionTemplate(JSTransferable::New); - t->Inherit(BaseObject::GetConstructorTemplate(env)); - t->InstanceTemplate()->SetInternalFieldCount( - JSTransferable::kInternalFieldCount); - env->SetConstructorFunction(target, "JSTransferable", t); - } + target + ->Set(env->context(), + FIXED_ONE_BYTE_STRING(env->isolate(), "JSTransferable"), + JSTransferable::GetConstructorFunction(env->isolate())) + .Check(); env->SetConstructorFunction( target, diff --git a/src/node_messaging.h b/src/node_messaging.h index 643604e4a99f15..2121e6e0679df4 100644 --- a/src/node_messaging.h +++ b/src/node_messaging.h @@ -322,6 +322,7 @@ class JSTransferable : public BaseObject { public: JSTransferable(Environment* env, v8::Local obj); static void New(const v8::FunctionCallbackInfo& args); + static v8::Local GetConstructorFunction(v8::Isolate* isolate); TransferMode GetTransferMode() const override; std::unique_ptr TransferForMessaging() override; diff --git a/test/parallel/test-errors-cloneabledomexception.js b/test/parallel/test-errors-cloneabledomexception.js new file mode 100644 index 00000000000000..22b73271adf643 --- /dev/null +++ b/test/parallel/test-errors-cloneabledomexception.js @@ -0,0 +1,24 @@ +'use strict'; + +const common = require('../common'); + +const { + ok, + strictEqual, +} = require('assert'); + +const exception = new DOMException('foo', 'AbortError'); +strictEqual(exception.name, 'AbortError'); +strictEqual(exception.message, 'foo'); +strictEqual(exception.code, 20); + +const mc = new MessageChannel(); +mc.port1.onmessage = common.mustCall(({ data }) => { + ok(data instanceof DOMException); + strictEqual(data.name, 'AbortError'); + strictEqual(data.message, 'foo'); + strictEqual(data.code, 20); + strictEqual(data.stack, exception.stack); + mc.port1.close(); +}); +mc.port2.postMessage(exception);