Added unwrapping logic to handle graceful error handling for primitives

Author: JckXia

napi-inl.h

@@ -2595,12 +2595,73 @@ inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullp
   if (value != nullptr) {
     napi_status status = napi_create_reference(env, value, 1, &_ref);
 
+    // Creates a wrapper object containg the error value (primitive types) and
+    // create a reference to this wrapper
+    if (status != napi_ok) {
+      napi_value wrappedErrorObj;
+      status = napi_create_object(env, &wrappedErrorObj);
+
+      NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_object");
+
+      status = napi_set_property(env,
+                                 wrappedErrorObj,
+                                 String::From(env, "errorVal"),
+                                 Value::From(env, value));
+      NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property");
+
+      status = napi_set_property(env,
+                                 wrappedErrorObj,
+                                 String::From(env, "isWrapObject"),
+                                 Value::From(env, value));
+      NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property");
+
+      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);
+  }
+  // Most likely will mess up thread execution
+
+  napi_value refValue;
+  napi_status status = napi_get_reference_value(_env, _ref, &refValue);
+  NAPI_THROW_IF_FAILED(_env, status, Object());
+
+  // We are wrapping this object
+  bool isWrappedObject = false;
+  napi_has_property(
+      _env, refValue, String::From(_env, "isWrapObject"), &isWrappedObject);
+  // Don't care about status
+
+  if (isWrappedObject == true) {
+    napi_value unwrappedValue;
+    status = napi_get_property(
+        _env, refValue, String::From(_env, "errorVal"), &unwrappedValue);
+    NAPI_THROW_IF_FAILED(_env, status, Object());
+    return Object(_env, unwrappedValue);
+  }
+
+  return Object(_env, refValue);
+}
+// template<typename T>
+// inline T Error::Value() const {
+//   // if (_ref == nullptr) {
+//   //   return T(_env, nullptr);
+//   // }
+
+//   // napi_value value;
+//   // napi_status status = napi_get_reference_value(_env, _ref, &value);
+//   // NAPI_THROW_IF_FAILED(_env, status, T());
+//   return nullptr;
+// }
+
 inline Error::Error(Error&& other) : ObjectReference(std::move(other)) {
 }
 
@@ -2651,6 +2712,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

@@ -4,11 +4,11 @@
 #include <node_api.h>
 #include <functional>
 #include <initializer_list>
+#include <iostream>
 #include <memory>
 #include <mutex>
 #include <string>
 #include <vector>
-
 // VS2015 RTM has bugs with constexpr, so require min of VS2015 Update 3 (known good version)
 #if !defined(_MSC_VER) || _MSC_FULL_VER >= 190024210
 #define NAPI_HAS_CONSTEXPR 1
@@ -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,7 @@ namespace Napi {
    /// !endcond
 
   private:
-    mutable std::string _message;
+   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',
+        'errorHandlingForPrimitives.cc',
         'external.cc',
         'function.cc',
         'function_reference.cc',

test/errorHandlingForPrimitives.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/errorHandlingForPrimitives.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');
+}