aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--src/node_api.cc78
-rw-r--r--test/addons-napi/test_exception/test.js30
-rw-r--r--test/addons-napi/test_exception/test_exception.c42
3 files changed, 107 insertions, 43 deletions
diff --git a/src/node_api.cc b/src/node_api.cc
index 3769de6c98..c5b93db959 100644
--- a/src/node_api.cc
+++ b/src/node_api.cc
@@ -157,6 +157,23 @@ struct napi_env__ {
(out) = v8::type::New((buffer), (byte_offset), (length)); \
} while (0)
+#define NAPI_CALL_INTO_MODULE(env, call, handle_exception) \
+ do { \
+ int open_handle_scopes = (env)->open_handle_scopes; \
+ int open_callback_scopes = (env)->open_callback_scopes; \
+ napi_clear_last_error((env)); \
+ call; \
+ CHECK_EQ((env)->open_handle_scopes, open_handle_scopes); \
+ CHECK_EQ((env)->open_callback_scopes, open_callback_scopes); \
+ if (!(env)->last_exception.IsEmpty()) { \
+ handle_exception( \
+ v8::Local<v8::Value>::New((env)->isolate, (env)->last_exception)); \
+ (env)->last_exception.Reset(); \
+ } \
+ } while (0)
+
+#define NAPI_CALL_INTO_MODULE_THROW(env, call) \
+ NAPI_CALL_INTO_MODULE((env), call, (env)->isolate->ThrowException)
namespace {
namespace v8impl {
@@ -336,10 +353,11 @@ class Finalizer {
static void FinalizeBufferCallback(char* data, void* hint) {
Finalizer* finalizer = static_cast<Finalizer*>(hint);
if (finalizer->_finalize_callback != nullptr) {
- finalizer->_finalize_callback(
- finalizer->_env,
- data,
- finalizer->_finalize_hint);
+ NAPI_CALL_INTO_MODULE_THROW(finalizer->_env,
+ finalizer->_finalize_callback(
+ finalizer->_env,
+ data,
+ finalizer->_finalize_hint));
}
Delete(finalizer);
@@ -441,10 +459,11 @@ class Reference : private Finalizer {
bool delete_self = reference->_delete_self;
if (reference->_finalize_callback != nullptr) {
- reference->_finalize_callback(
- reference->_env,
- reference->_finalize_data,
- reference->_finalize_hint);
+ NAPI_CALL_INTO_MODULE_THROW(reference->_env,
+ reference->_finalize_callback(
+ reference->_env,
+ reference->_finalize_data,
+ reference->_finalize_hint));
}
if (delete_self) {
@@ -529,32 +548,17 @@ class CallbackWrapperBase : public CallbackWrapper {
napi_callback cb = reinterpret_cast<napi_callback>(
v8::Local<v8::External>::Cast(
_cbdata->GetInternalField(kInternalFieldIndex))->Value());
- v8::Isolate* isolate = _cbinfo.GetIsolate();
napi_env env = static_cast<napi_env>(
v8::Local<v8::External>::Cast(
_cbdata->GetInternalField(kEnvIndex))->Value());
- // Make sure any errors encountered last time we were in N-API are gone.
- napi_clear_last_error(env);
-
- int open_handle_scopes = env->open_handle_scopes;
- int open_callback_scopes = env->open_callback_scopes;
-
- napi_value result = cb(env, cbinfo_wrapper);
+ napi_value result;
+ NAPI_CALL_INTO_MODULE_THROW(env, result = cb(env, cbinfo_wrapper));
if (result != nullptr) {
this->SetReturnValue(result);
}
-
- CHECK_EQ(env->open_handle_scopes, open_handle_scopes);
- CHECK_EQ(env->open_callback_scopes, open_callback_scopes);
-
- if (!env->last_exception.IsEmpty()) {
- isolate->ThrowException(
- v8::Local<v8::Value>::New(isolate, env->last_exception));
- env->last_exception.Reset();
- }
}
const Info& _cbinfo;
@@ -861,8 +865,10 @@ void napi_module_register_cb(v8::Local<v8::Object> exports,
// one is found.
napi_env env = v8impl::GetEnv(context);
- napi_value _exports =
- mod->nm_register_func(env, v8impl::JsValueFromV8LocalValue(exports));
+ napi_value _exports;
+ NAPI_CALL_INTO_MODULE_THROW(env,
+ _exports = mod->nm_register_func(env,
+ v8impl::JsValueFromV8LocalValue(exports)));
// If register function returned a non-null exports object different from
// the exports object we passed it, set that as the "exports" property of
@@ -3357,19 +3363,17 @@ class Work : public node::AsyncResource {
v8::HandleScope scope(env->isolate);
CallbackScope callback_scope(work);
- work->_complete(env, ConvertUVErrorCode(status), work->_data);
+ NAPI_CALL_INTO_MODULE(env,
+ work->_complete(env, ConvertUVErrorCode(status), work->_data),
+ [env] (v8::Local<v8::Value> local_err) {
+ // If there was an unhandled exception in the complete callback,
+ // report it as a fatal exception. (There is no JavaScript on the
+ // callstack that can possibly handle it.)
+ v8impl::trigger_fatal_exception(env, local_err);
+ });
// Note: Don't access `work` after this point because it was
// likely deleted by the complete callback.
-
- // If there was an unhandled exception in the complete callback,
- // report it as a fatal exception. (There is no JavaScript on the
- // callstack that can possibly handle it.)
- if (!env->last_exception.IsEmpty()) {
- v8::Local<v8::Value> local_err = v8::Local<v8::Value>::New(
- env->isolate, env->last_exception);
- v8impl::trigger_fatal_exception(env, local_err);
- }
}
}
diff --git a/test/addons-napi/test_exception/test.js b/test/addons-napi/test_exception/test.js
index 787b7d78b1..b9311add6c 100644
--- a/test/addons-napi/test_exception/test.js
+++ b/test/addons-napi/test_exception/test.js
@@ -1,10 +1,26 @@
'use strict';
+// Flags: --expose-gc
const common = require('../../common');
-const test_exception = require(`./build/${common.buildType}/test_exception`);
const assert = require('assert');
const theError = new Error('Some error');
+// The test module throws an error during Init, but in order for its exports to
+// not be lost, it attaches them to the error's "bindings" property. This way,
+// we can make sure that exceptions thrown during the module initialization
+// phase are propagated through require() into JavaScript.
+// https://github.com/nodejs/node/issues/19437
+const test_exception = (function() {
+ let resultingException;
+ try {
+ require(`./build/${common.buildType}/test_exception`);
+ } catch (anException) {
+ resultingException = anException;
+ }
+ assert.strictEqual(resultingException.message, 'Error during Init');
+ return resultingException.binding;
+})();
+
{
const throwTheError = () => { throw theError; };
@@ -50,3 +66,15 @@ const theError = new Error('Some error');
'Exception state did not remain clear as expected,' +
` .wasPending() returned ${exception_pending}`);
}
+
+// Make sure that exceptions that occur during finalization are propagated.
+function testFinalize(binding) {
+ let x = test_exception[binding]();
+ x = null;
+ assert.throws(() => { global.gc(); }, /Error during Finalize/);
+
+ // To assuage the linter's concerns.
+ (function() {})(x);
+}
+testFinalize('createExternal');
+testFinalize('createExternalBuffer');
diff --git a/test/addons-napi/test_exception/test_exception.c b/test/addons-napi/test_exception/test_exception.c
index 8b664210e9..61116d0603 100644
--- a/test/addons-napi/test_exception/test_exception.c
+++ b/test/addons-napi/test_exception/test_exception.c
@@ -3,7 +3,7 @@
static bool exceptionWasPending = false;
-napi_value returnException(napi_env env, napi_callback_info info) {
+static napi_value returnException(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
@@ -22,7 +22,7 @@ napi_value returnException(napi_env env, napi_callback_info info) {
return NULL;
}
-napi_value allowException(napi_env env, napi_callback_info info) {
+static napi_value allowException(napi_env env, napi_callback_info info) {
size_t argc = 1;
napi_value args[1];
NAPI_CALL(env, napi_get_cb_info(env, info, &argc, args, NULL, NULL));
@@ -38,23 +38,55 @@ napi_value allowException(napi_env env, napi_callback_info info) {
return NULL;
}
-napi_value wasPending(napi_env env, napi_callback_info info) {
+static napi_value wasPending(napi_env env, napi_callback_info info) {
napi_value result;
NAPI_CALL(env, napi_get_boolean(env, exceptionWasPending, &result));
return result;
}
-napi_value Init(napi_env env, napi_value exports) {
+static void finalizer(napi_env env, void *data, void *hint) {
+ NAPI_CALL_RETURN_VOID(env,
+ napi_throw_error(env, NULL, "Error during Finalize"));
+}
+
+static napi_value createExternal(napi_env env, napi_callback_info info) {
+ napi_value external;
+
+ NAPI_CALL(env,
+ napi_create_external(env, NULL, finalizer, NULL, &external));
+
+ return external;
+}
+
+static char buffer_data[12];
+
+static napi_value createExternalBuffer(napi_env env, napi_callback_info info) {
+ napi_value buffer;
+ NAPI_CALL(env, napi_create_external_buffer(env, sizeof(buffer_data),
+ buffer_data, finalizer, NULL, &buffer));
+ return buffer;
+}
+
+static napi_value Init(napi_env env, napi_value exports) {
napi_property_descriptor descriptors[] = {
DECLARE_NAPI_PROPERTY("returnException", returnException),
DECLARE_NAPI_PROPERTY("allowException", allowException),
DECLARE_NAPI_PROPERTY("wasPending", wasPending),
+ DECLARE_NAPI_PROPERTY("createExternal", createExternal),
+ DECLARE_NAPI_PROPERTY("createExternalBuffer", createExternalBuffer),
};
-
NAPI_CALL(env, napi_define_properties(
env, exports, sizeof(descriptors) / sizeof(*descriptors), descriptors));
+ napi_value error, code, message;
+ NAPI_CALL(env, napi_create_string_utf8(env, "Error during Init",
+ NAPI_AUTO_LENGTH, &message));
+ NAPI_CALL(env, napi_create_string_utf8(env, "", NAPI_AUTO_LENGTH, &code));
+ NAPI_CALL(env, napi_create_error(env, code, message, &error));
+ NAPI_CALL(env, napi_set_named_property(env, error, "binding", exports));
+ NAPI_CALL(env, napi_throw(env, error));
+
return exports;
}