Merge pull request #1075 from JckXia/handle-error-thrown

Add logic to handle graceful error handling when primitive errors are thrown by JS functions

doc/error_handling.md

@@ -14,6 +14,9 @@ If C++ exceptions are enabled (for more info see: [Setup](setup.md)), then the
 `Napi::Error` class extends `std::exception` and enables integrated
 error-handling for C++ exceptions and JavaScript exceptions.
 
+Note, that due to limitations of the Node-API, if one attempts to cast the error object wrapping a primitive inside a C++ addon, the wrapped object
+will be received instead. (With property `4bda9e7e-4913-4dbc-95de-891cbf66598e-errorVal` containing the primitive value thrown)
+
 The following sections explain the approach for each case:
 
 - [Handling Errors With C++ Exceptions](#exceptions)

napi-inl.h

@@ -2593,14 +2593,80 @@ inline Error::Error() : ObjectReference() {
 
 inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullptr) {
   if (value != nullptr) {
+    // Attempting to create a reference on the error object.
+    // If it's not a Object/Function/Symbol, this call will return an error
+    // status.
     napi_status status = napi_create_reference(env, value, 1, &_ref);
 
+    if (status != napi_ok) {
+      napi_value wrappedErrorObj;
+
+      // Create an error object
+      status = napi_create_object(env, &wrappedErrorObj);
+      NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_object");
+
+      // property flag that we attach to show the error object is wrapped
+      napi_property_descriptor wrapObjFlag = {
+          ERROR_WRAP_VALUE,  // Unique GUID identifier since Symbol isn't a
+                             // viable option
+          nullptr,
+          nullptr,
+          nullptr,
+          nullptr,
+          Value::From(env, value),
+          napi_enumerable,
+          nullptr};
+
+      status = napi_define_properties(env, wrappedErrorObj, 1, &wrapObjFlag);
+      NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties");
+
+      // Create a reference on the newly wrapped object
+      status = napi_create_reference(env, wrappedErrorObj, 1, &_ref);
+    }
+
     // Avoid infinite recursion in the failure case.
-    // Don't try to construct & throw another Error instance.
     NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_reference");
   }
 }
 
+inline Object Error::Value() const {
+  if (_ref == nullptr) {
+    return Object(_env, nullptr);
+  }
+
+  napi_value refValue;
+  napi_status status = napi_get_reference_value(_env, _ref, &refValue);
+  NAPI_THROW_IF_FAILED(_env, status, Object());
+
+  napi_valuetype type;
+  status = napi_typeof(_env, refValue, &type);
+  NAPI_THROW_IF_FAILED(_env, status, Object());
+
+  // If refValue isn't a symbol, then we proceed to whether the refValue has the
+  // wrapped error flag
+  if (type != napi_symbol) {
+    // We are checking if the object is wrapped
+    bool isWrappedObject = false;
+
+    status = napi_has_property(
+        _env, refValue, String::From(_env, ERROR_WRAP_VALUE), &isWrappedObject);
+
+    // Don't care about status
+    if (isWrappedObject) {
+      napi_value unwrappedValue;
+      status = napi_get_property(_env,
+                                 refValue,
+                                 String::From(_env, ERROR_WRAP_VALUE),
+                                 &unwrappedValue);
+      NAPI_THROW_IF_FAILED(_env, status, Object());
+
+      return Object(_env, unwrappedValue);
+    }
+  }
+
+  return Object(_env, refValue);
+}
+
 inline Error::Error(Error&& other) : ObjectReference(std::move(other)) {
 }
 
@@ -2651,6 +2717,7 @@ inline const std::string& Error::Message() const NAPI_NOEXCEPT {
   return _message;
 }
 
+// we created an object on the &_ref
 inline void Error::ThrowAsJavaScriptException() const {
   HandleScope scope(_env);
   if (!IsEmpty()) {

napi.h

@@ -1699,6 +1699,8 @@ namespace Napi {
     const std::string& Message() const NAPI_NOEXCEPT;
     void ThrowAsJavaScriptException() const;
 
+    Object Value() const;
+
 #ifdef NAPI_CPP_EXCEPTIONS
     const char* what() const NAPI_NOEXCEPT override;
 #endif // NAPI_CPP_EXCEPTIONS
@@ -1718,7 +1720,9 @@ namespace Napi {
    /// !endcond
 
   private:
-    mutable std::string _message;
+   const char* ERROR_WRAP_VALUE =
+       "4bda9e7e-4913-4dbc-95de-891cbf66598e-errorVal";
+   mutable std::string _message;
   };
 
   class TypeError : public Error {

test/binding.cc

@@ -31,6 +31,7 @@ Object InitDate(Env env);
 Object InitDataView(Env env);
 Object InitDataViewReadWrite(Env env);
 Object InitEnvCleanup(Env env);
+Object InitErrorHandlingPrim(Env env);
 Object InitError(Env env);
 Object InitExternal(Env env);
 Object InitFunction(Env env);
@@ -113,6 +114,7 @@ Object Init(Env env, Object exports) {
   exports.Set("env_cleanup", InitEnvCleanup(env));
 #endif
   exports.Set("error", InitError(env));
+  exports.Set("errorHandlingPrim", InitErrorHandlingPrim(env));
   exports.Set("external", InitExternal(env));
   exports.Set("function", InitFunction(env));
   exports.Set("functionreference", InitFunctionReference(env));

test/binding.gyp

@@ -25,6 +25,7 @@
         'dataview/dataview_read_write.cc',
         'env_cleanup.cc',
         'error.cc',
+        'error_handling_for_primitives.cc',
         'external.cc',
         'function.cc',
         'function_reference.cc',

test/error_handling_for_primitives.cc

@@ -0,0 +1,13 @@
+#include <napi.h>
+
+namespace {
+void Test(const Napi::CallbackInfo& info) {
+  info[0].As<Napi::Function>().Call({});
+}
+
+}  // namespace
+Napi::Object InitErrorHandlingPrim(Napi::Env env) {
+  Napi::Object exports = Napi::Object::New(env);
+  exports.Set("errorHandlingPrim", Napi::Function::New<Test>(env));
+  return exports;
+}

test/error_handling_for_primitives.js

@@ -0,0 +1,29 @@
+'use strict';
+
+const assert = require('assert');
+
+module.exports = require('./common').runTest((binding) => {
+  test(binding.errorHandlingPrim);
+});
+
+function canThrow (binding, errorMessage, errorType) {
+  try {
+    binding.errorHandlingPrim(() => {
+      throw errorMessage;
+    });
+  } catch (e) {
+    // eslint-disable-next-line valid-typeof
+    assert(typeof e === errorType);
+    assert(e === errorMessage);
+  }
+}
+
+function test (binding) {
+  canThrow(binding, '404 server not found!', 'string');
+  canThrow(binding, 42, 'number');
+  canThrow(binding, Symbol.for('newSym'), 'symbol');
+  canThrow(binding, false, 'boolean');
+  canThrow(binding, BigInt(123), 'bigint');
+  canThrow(binding, () => { console.log('Logger shutdown incorrectly'); }, 'function');
+  canThrow(binding, { status: 403, errorMsg: 'Not authenticated' }, 'object');
+}