From 173c5bc9d95df3c2909138e819e1bad52db8b28c Mon Sep 17 00:00:00 2001
From: JckXia <ys3xia@uwaterloo.ca>
Date: Thu, 21 Oct 2021 16:11:06 -0400
Subject: [PATCH] Update PR based on review comments

---
 doc/error_handling.md                         |  5 ++-
 napi-inl.h                                    | 36 +++++++++++--------
 napi.h                                        |  1 +
 test/binding.gyp                              |  2 +-
 ...es.cc => error_handling_for_primitives.cc} |  2 +-
 ...es.js => error_handling_for_primitives.js} |  0
 6 files changed, 26 insertions(+), 20 deletions(-)
 rename test/{errorHandlingForPrimitives.cc => error_handling_for_primitives.cc} (99%)
 rename test/{errorHandlingForPrimitives.js => error_handling_for_primitives.js} (100%)

diff --git a/doc/error_handling.md b/doc/error_handling.md
index 7063f5548..6d8b64505 100644
--- 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
index ceff4a550..3d8529572 100644
--- a/napi-inl.h
+++ b/napi-inl.h
@@ -2635,21 +2635,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
index 29a2a47e0..0ab280583 100644
--- 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
index 9eba8c683..44f124b01 100644
--- 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 b/test/error_handling_for_primitives.cc
similarity index 99%
rename from test/errorHandlingForPrimitives.cc
rename to test/error_handling_for_primitives.cc
index 58d3f906f..173293264 100644
--- a/test/errorHandlingForPrimitives.cc
+++ b/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;
-}
\ No newline at end of file
+}
diff --git a/test/errorHandlingForPrimitives.js b/test/error_handling_for_primitives.js
similarity index 100%
rename from test/errorHandlingForPrimitives.js
rename to test/error_handling_for_primitives.js