n-api: ensure in-module exceptions are propagated

Whenever we call into an addon, whether it is for a callback, for
module init, or for async work-related reasons, we should make sure
that

* the last error is cleared,
* the scopes before the call are the same as after, and
* if an exception was thrown and captured inside the module, then it is
  re-thrown after the call.

Therefore we should call into the module in a unified fashion. This
change introduces the macro NAPI_CALL_INTO_MODULE() which should be
used whenever invoking a callback provided by the module.

Fixes: https://github.com/nodejs/node/issues/19437
PR-URL: https://github.com/nodejs/node/pull/19537
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>

2 files changed, 66 insertions, 6 deletions

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;
 }