From ed4d1c51c46fecc28cb9b0552952264a9ed11b0b Mon Sep 17 00:00:00 2001 From: JckXia Date: Mon, 20 Sep 2021 21:57:38 -0400 Subject: [PATCH 1/7] Added unwrapping logic to handle graceful error handling for primitives --- napi-inl.h | 62 ++++++++++++++++++++++++++++++ napi.h | 6 ++- test/binding.cc | 2 + test/binding.gyp | 1 + test/errorHandlingForPrimitives.cc | 13 +++++++ test/errorHandlingForPrimitives.js | 29 ++++++++++++++ 6 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 test/errorHandlingForPrimitives.cc create mode 100644 test/errorHandlingForPrimitives.js diff --git a/napi-inl.h b/napi-inl.h index 5eff0b915..351e03593 100644 --- a/napi-inl.h +++ b/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 +// 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()) { diff --git a/napi.h b/napi.h index e1ddd0087..950a41d87 100644 --- a/napi.h +++ b/napi.h @@ -4,11 +4,11 @@ #include #include #include +#include #include #include #include #include - // 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 { diff --git a/test/binding.cc b/test/binding.cc index 389f61b97..57be51592 100644 --- a/test/binding.cc +++ b/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)); diff --git a/test/binding.gyp b/test/binding.gyp index 2969dc1f9..9eba8c683 100644 --- a/test/binding.gyp +++ b/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', diff --git a/test/errorHandlingForPrimitives.cc b/test/errorHandlingForPrimitives.cc new file mode 100644 index 000000000..58d3f906f --- /dev/null +++ b/test/errorHandlingForPrimitives.cc @@ -0,0 +1,13 @@ +#include + +namespace { +void Test(const Napi::CallbackInfo& info) { + info[0].As().Call({}); +} + +} // namespace +Napi::Object InitErrorHandlingPrim(Napi::Env env) { + Napi::Object exports = Napi::Object::New(env); + exports.Set("errorHandlingPrim", Napi::Function::New(env)); + return exports; +} \ No newline at end of file diff --git a/test/errorHandlingForPrimitives.js b/test/errorHandlingForPrimitives.js new file mode 100644 index 000000000..105c2b605 --- /dev/null +++ b/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'); +} From c89f0bfb0b4df5e47b4c34b05a4379fa21caded7 Mon Sep 17 00:00:00 2001 From: JckXia Date: Mon, 20 Sep 2021 22:09:07 -0400 Subject: [PATCH 2/7] Remove un-necessary comment/iostream and updated docs to reflect on limitations with this impl --- doc/error_handling.md | 4 ++++ napi-inl.h | 14 +------------- napi.h | 1 - 3 files changed, 5 insertions(+), 14 deletions(-) diff --git a/doc/error_handling.md b/doc/error_handling.md index 642c223e5..709d5aedd 100644 --- a/doc/error_handling.md +++ b/doc/error_handling.md @@ -14,6 +14,10 @@ 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 attempt to cast the error object thrown as a primitive, an +wrapped object will be received instead. (With properties ```isWrapObject``` and ```errorVal``` containing the primitive value thrown) + + The following sections explain the approach for each case: - [Handling Errors With C++ Exceptions](#exceptions) diff --git a/napi-inl.h b/napi-inl.h index 351e03593..befcc5871 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2628,13 +2628,12 @@ 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 + // We are checking if the object is wrapped bool isWrappedObject = false; napi_has_property( _env, refValue, String::From(_env, "isWrapObject"), &isWrappedObject); @@ -2650,17 +2649,6 @@ inline Object Error::Value() const { return Object(_env, refValue); } -// template -// 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)) { } diff --git a/napi.h b/napi.h index 950a41d87..29a2a47e0 100644 --- a/napi.h +++ b/napi.h @@ -4,7 +4,6 @@ #include #include #include -#include #include #include #include From 02bcfbccfdfc891d911b01b1a0747121c7424836 Mon Sep 17 00:00:00 2001 From: JckXia Date: Tue, 28 Sep 2021 15:47:05 -0400 Subject: [PATCH 3/7] Update doc and appending GUID to object property --- doc/error_handling.md | 4 ++-- napi-inl.h | 15 ++++++++++----- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/doc/error_handling.md b/doc/error_handling.md index 709d5aedd..7063f5548 100644 --- a/doc/error_handling.md +++ b/doc/error_handling.md @@ -14,8 +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 attempt to cast the error object thrown as a primitive, an -wrapped object will be received instead. (With properties ```isWrapObject``` and ```errorVal``` containing the primitive value thrown) +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) The following sections explain the approach for each case: diff --git a/napi-inl.h b/napi-inl.h index befcc5871..ceff4a550 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2609,10 +2609,12 @@ inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullp 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)); + status = napi_set_property( + env, + wrappedErrorObj, + String::From(env, + "4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject"), + Value::From(env, value)); NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); status = napi_create_reference(env, wrappedErrorObj, 1, &_ref); @@ -2636,7 +2638,10 @@ inline Object Error::Value() const { // We are checking if the object is wrapped bool isWrappedObject = false; napi_has_property( - _env, refValue, String::From(_env, "isWrapObject"), &isWrappedObject); + _env, + refValue, + String::From(_env, "4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject"), + &isWrappedObject); // Don't care about status if (isWrappedObject == true) { From 173c5bc9d95df3c2909138e819e1bad52db8b28c Mon Sep 17 00:00:00 2001 From: JckXia Date: Thu, 21 Oct 2021 16:11:06 -0400 Subject: [PATCH 4/7] 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 #include #include + // 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(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 From a0b3fe919794be59858e2aa0795364ac662f3bfc Mon Sep 17 00:00:00 2001 From: JckXia Date: Sun, 24 Oct 2021 12:30:36 -0400 Subject: [PATCH 5/7] Replace magic value with symbol --- napi-inl.h | 74 +++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 56 insertions(+), 18 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 3d8529572..681677d32 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2603,20 +2603,45 @@ inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullp 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, - "4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject"), - Value::From(env, value)); - NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); + napi_property_descriptor errValDesc = { + "errorVal", // const char* utf8Name + String::From(env, "errorVal"), // napi_value name + nullptr, // Method + nullptr, // getter + nullptr, // setter + Value::From(env, value), // napi_value value + napi_enumerable, + nullptr}; + + status = napi_define_properties(env, wrappedErrorObj, 1, &errValDesc); + NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); + + MaybeOrValue result = Symbol::For(env, "isWrapObject"); + napi_property_descriptor wrapObjFlag = {nullptr, + nullptr, + nullptr, + nullptr, + nullptr, + Value::From(env, value), + napi_enumerable, + nullptr}; +#ifdef NODE_ADDON_API_ENABLE_MAYBE + Symbol uniqueSymb; + if (result.IsJust()) { + uniqueSymb = result.Unwrap(); + } + + wrapObjFlag.name = uniqueSymb; + status = napi_define_properties(env, wrappedErrorObj, 1, &wrapObjFlag); + NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); +#else + + wrapObjFlag.name = result; + status = napi_define_properties(env, wrappedErrorObj, 1, &wrapObjFlag); + NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); + +#endif status = napi_create_reference(env, wrappedErrorObj, 1, &_ref); } @@ -2642,11 +2667,23 @@ inline Object Error::Value() const { 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); + + MaybeOrValue result = Symbol::For(_env, "isWrapObject"); + +#ifdef NODE_ADDON_API_ENABLE_MAYBE + Symbol uniqueSymb; + if (result.IsJust()) { + uniqueSymb = result.Unwrap(); + } + status = napi_has_property(_env, refValue, uniqueSymb, &isWrappedObject); + NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); + +#else + + status = napi_has_property(_env, refValue, result, &isWrappedObject); + NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); +#endif + // Don't care about status if (isWrappedObject == true) { @@ -2654,6 +2691,7 @@ inline Object Error::Value() const { status = napi_get_property( _env, refValue, String::From(_env, "errorVal"), &unwrappedValue); NAPI_THROW_IF_FAILED(_env, status, Object()); + return Object(_env, unwrappedValue); } } From 691813842e82416ed67cad3ff877f5e8d6a6d896 Mon Sep 17 00:00:00 2001 From: JckXia Date: Sun, 14 Nov 2021 17:21:51 -0500 Subject: [PATCH 6/7] Refactor code. Using hard coded string instead of using symbol --- napi-inl.h | 80 ++++++++++++++++-------------------------------------- napi.h | 2 ++ 2 files changed, 26 insertions(+), 56 deletions(-) diff --git a/napi-inl.h b/napi-inl.h index 681677d32..bd54feb67 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -2593,60 +2593,38 @@ 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); - // 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); + // Create an error object + status = napi_create_object(env, &wrappedErrorObj); NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_object"); - napi_property_descriptor errValDesc = { - "errorVal", // const char* utf8Name - String::From(env, "errorVal"), // napi_value name - nullptr, // Method - nullptr, // getter - nullptr, // setter - Value::From(env, value), // napi_value value + // 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, &errValDesc); - NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); - - MaybeOrValue result = Symbol::For(env, "isWrapObject"); - napi_property_descriptor wrapObjFlag = {nullptr, - nullptr, - nullptr, - nullptr, - nullptr, - Value::From(env, value), - napi_enumerable, - nullptr}; - -#ifdef NODE_ADDON_API_ENABLE_MAYBE - Symbol uniqueSymb; - if (result.IsJust()) { - uniqueSymb = result.Unwrap(); - } - - wrapObjFlag.name = uniqueSymb; status = napi_define_properties(env, wrappedErrorObj, 1, &wrapObjFlag); NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); -#else - wrapObjFlag.name = result; - status = napi_define_properties(env, wrappedErrorObj, 1, &wrapObjFlag); - NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_define_properties"); - -#endif + // 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"); } } @@ -2664,32 +2642,22 @@ inline Object Error::Value() const { 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; - MaybeOrValue result = Symbol::For(_env, "isWrapObject"); - -#ifdef NODE_ADDON_API_ENABLE_MAYBE - Symbol uniqueSymb; - if (result.IsJust()) { - uniqueSymb = result.Unwrap(); - } - status = napi_has_property(_env, refValue, uniqueSymb, &isWrappedObject); - NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); - -#else - - status = napi_has_property(_env, refValue, result, &isWrappedObject); - NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_set_property"); -#endif + status = napi_has_property( + _env, refValue, String::From(_env, ERROR_WRAP_VALUE), &isWrappedObject); // Don't care about status - - if (isWrappedObject == true) { + if (isWrappedObject) { napi_value unwrappedValue; - status = napi_get_property( - _env, refValue, String::From(_env, "errorVal"), &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); diff --git a/napi.h b/napi.h index 0ab280583..286430643 100644 --- a/napi.h +++ b/napi.h @@ -1720,6 +1720,8 @@ namespace Napi { /// !endcond private: + const char* ERROR_WRAP_VALUE = + "4bda9e7e-4913-4dbc-95de-891cbf66598e-errorVal"; mutable std::string _message; }; From e0567d098a3012ed615b12795bcdb547aaee590b Mon Sep 17 00:00:00 2001 From: JckXia Date: Sun, 14 Nov 2021 17:23:59 -0500 Subject: [PATCH 7/7] Update documents --- doc/error_handling.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/error_handling.md b/doc/error_handling.md index 6d8b64505..6882d599f 100644 --- a/doc/error_handling.md +++ b/doc/error_handling.md @@ -15,7 +15,7 @@ If C++ exceptions are enabled (for more info see: [Setup](setup.md)), then the 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 properties `4b3d96fd-fb87-4951-a979-eb4f9d2f2ce9-isWrapObject` and `errorVal` containing the primitive value thrown) +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: