Skip to content

Commit

Permalink
Added unwrapping logic to handle graceful error handling for primitives
Browse files Browse the repository at this point in the history
  • Loading branch information
JckXia committed Oct 23, 2021
1 parent 4663453 commit ed4d1c5
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 2 deletions.
62 changes: 62 additions & 0 deletions napi-inl.h
Expand Up @@ -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)) {
}

Expand Down Expand Up @@ -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()) {
Expand Down
6 changes: 4 additions & 2 deletions napi.h
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -1718,7 +1720,7 @@ namespace Napi {
/// !endcond

private:
mutable std::string _message;
mutable std::string _message;
};

class TypeError : public Error {
Expand Down
2 changes: 2 additions & 0 deletions test/binding.cc
Expand Up @@ -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);
Expand Down Expand Up @@ -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));
Expand Down
1 change: 1 addition & 0 deletions test/binding.gyp
Expand Up @@ -25,6 +25,7 @@
'dataview/dataview_read_write.cc',
'env_cleanup.cc',
'error.cc',
'errorHandlingForPrimitives.cc',
'external.cc',
'function.cc',
'function_reference.cc',
Expand Down
13 changes: 13 additions & 0 deletions 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;
}
29 changes: 29 additions & 0 deletions 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');
}

0 comments on commit ed4d1c5

Please sign in to comment.