diff --git a/napi-inl.h b/napi-inl.h index 98c578ed1..8fa028e03 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -3370,9 +3370,32 @@ inline napi_value ObjectWrap::ConstructorCallbackWrapper( } T* instance; + auto removeFailedWrap = [](const CallbackInfo& info) { + napi_status status = napi_remove_wrap(info.Env(), info.This(), nullptr); + NAPI_FATAL_IF_FAILED(status, + "ObjectWrap::ConstructorCallbackWrapper", + "Failed to remove wrap from failed ObjectWrap instance construction"); + }; napi_value wrapper = details::WrapCallback([&] { CallbackInfo callbackInfo(env, info); +#ifdef NAPI_CPP_EXCEPTIONS + try { + instance = new T(callbackInfo); + } catch (const Error& e) { + removeFailedWrap(callbackInfo); + // re-throw the error after removing the failed wrap. + throw e; + } +#else instance = new T(callbackInfo); + if (callbackInfo.Env().IsExceptionPending()) { + // We need to clear the exception so that `napi_remove_wrap()` might work. + Error e = callbackInfo.Env().GetAndClearPendingException(); + removeFailedWrap(callbackInfo); + e.ThrowAsJavaScriptException(); + delete instance; + } +# endif // NAPI_CPP_EXCEPTIONS return callbackInfo.This(); }); diff --git a/test/binding.cc b/test/binding.cc index 0999ec7c4..ec4a3645a 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -48,6 +48,7 @@ Object InitThreadSafeFunction(Env env); #endif Object InitTypedArray(Env env); Object InitObjectWrap(Env env); +Object InitObjectWrapConstructorException(Env env); Object InitObjectReference(Env env); Object InitVersionManagement(Env env); Object InitThunkingManual(Env env); @@ -100,6 +101,8 @@ Object Init(Env env, Object exports) { #endif exports.Set("typedarray", InitTypedArray(env)); exports.Set("objectwrap", InitObjectWrap(env)); + exports.Set("objectwrapConstructorException", + InitObjectWrapConstructorException(env)); exports.Set("objectreference", InitObjectReference(env)); exports.Set("version_management", InitVersionManagement(env)); exports.Set("thunking_manual", InitThunkingManual(env)); diff --git a/test/binding.gyp b/test/binding.gyp index a21ef8e95..69f1256e5 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -43,6 +43,7 @@ 'threadsafe_function/threadsafe_function.cc', 'typedarray.cc', 'objectwrap.cc', + 'objectwrap_constructor_exception.cc', 'objectreference.cc', 'version_management.cc', 'thunking_manual.cc', diff --git a/test/index.js b/test/index.js index 2269aed8a..ee1095942 100644 --- a/test/index.js +++ b/test/index.js @@ -47,6 +47,7 @@ let testModules = [ 'typedarray', 'typedarray-bigint', 'objectwrap', + 'objectwrap_constructor_exception', 'objectreference', 'version_management' ]; diff --git a/test/objectwrap_constructor_exception.cc b/test/objectwrap_constructor_exception.cc new file mode 100644 index 000000000..d7e1bd517 --- /dev/null +++ b/test/objectwrap_constructor_exception.cc @@ -0,0 +1,26 @@ +#include + +class ConstructorExceptionTest : + public Napi::ObjectWrap { +public: + ConstructorExceptionTest(const Napi::CallbackInfo& info) : + Napi::ObjectWrap(info) { + Napi::Error error = Napi::Error::New(info.Env(), "an exception"); +#ifdef NAPI_DISABLE_CPP_EXCEPTIONS + error.ThrowAsJavaScriptException(); +#else + throw error; +#endif // NAPI_DISABLE_CPP_EXCEPTIONS + } + + static void Initialize(Napi::Env env, Napi::Object exports) { + const char* name = "ConstructorExceptionTest"; + exports.Set(name, DefineClass(env, name, {})); + } +}; + +Napi::Object InitObjectWrapConstructorException(Napi::Env env) { + Napi::Object exports = Napi::Object::New(env); + ConstructorExceptionTest::Initialize(env, exports); + return exports; +} diff --git a/test/objectwrap_constructor_exception.js b/test/objectwrap_constructor_exception.js new file mode 100644 index 000000000..db64e2e05 --- /dev/null +++ b/test/objectwrap_constructor_exception.js @@ -0,0 +1,19 @@ +'use strict'; +const buildType = process.config.target_defaults.default_configuration; +const assert = require('assert'); + +const test = (binding) => { + const { ConstructorExceptionTest } = binding.objectwrapConstructorException; + let gotException = false; + try { + new ConstructorExceptionTest(); + } catch (anException) { + gotException = true; + } + global.gc(); + + assert.strictEqual(gotException, true); +} + +test(require(`./build/${buildType}/binding.node`)); +test(require(`./build/${buildType}/binding_noexcept.node`));