Update PR based on review comments

nodejs · Oct 21, 2021 · 17bff94 · 17bff94
1 parent a8e4119
commit 17bff94
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 20 deletions.
diff --git a/doc/error_handling.md b/doc/error_handling.md
@@ -14,9 +14,8 @@ 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 N-API, if one attempts to cast the error object wrapping a primitive inside a C++ addon, the wrapped object
-will be received instead. (With properties ```4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject``` and ```errorVal``` containing the primitive value thrown)
-
+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 properties `4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject` and `errorVal` containing the primitive value thrown)
 
 The following sections explain the approach for each case:
 

diff --git a/napi-inl.h b/napi-inl.h
@@ -2623,21 +2623,27 @@ inline Object Error::Value() const {
   napi_status status = napi_get_reference_value(_env, _ref, &refValue);
   NAPI_THROW_IF_FAILED(_env, status, Object());
 
-  // We are checking if the object is wrapped
-  bool isWrappedObject = false;
-  napi_has_property(
-      _env,
-      refValue,
-      String::From(_env, "4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-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);
+  napi_valuetype type;
+  status = napi_typeof(_env, refValue, &type);
+  NAPI_THROW_IF_FAILED(_env, status, Object());
+
+  if (type != napi_symbol) {
+    // We are checking if the object is wrapped
+    bool isWrappedObject = false;
+    napi_has_property(
+        _env,
+        refValue,
+        String::From(_env, "4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-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);

diff --git a/napi.h b/napi.h
@@ -8,6 +8,7 @@
 #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

diff --git a/test/binding.gyp b/test/binding.gyp
@@ -25,7 +25,7 @@
         'dataview/dataview_read_write.cc',
         'env_cleanup.cc',
         'error.cc',
-        'errorHandlingForPrimitives.cc',
+        'error_handling_for_primitives.cc',
         'external.cc',
         'function.cc',
         'function_reference.cc',

diff --git a/test/errorHandlingForPrimitives.cc → test/error_handling_for_primitives.cc b/test/errorHandlingForPrimitives.cc → test/error_handling_for_primitives.cc
@@ -10,4 +10,4 @@ Napi::Object InitErrorHandlingPrim(Napi::Env env) {
   Napi::Object exports = Napi::Object::New(env);
   exports.Set("errorHandlingPrim", Napi::Function::New<Test>(env));
   return exports;
-}
+}
diff --git a/test/errorHandlingForPrimitives.js → test/error_handling_for_primitives.js b/test/errorHandlingForPrimitives.js → test/error_handling_for_primitives.js