Skip to content

Commit

Permalink
Make Error a persistent reference (#32)
Browse files Browse the repository at this point in the history
A Napi::Error instance must hold a persistent reference to the JS
error object so that it can escape from handle scopes when thrown
as a C++ exception.

Fixes: #31
  • Loading branch information
jasongin authored May 4, 2017
1 parent 97bc4c1 commit 1f1bb5e
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 106 deletions.
59 changes: 49 additions & 10 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -1381,6 +1381,8 @@ inline void Buffer<T>::EnsureInfo() const {
////////////////////////////////////////////////////////////////////////////////

inline Error Error::New(napi_env env) {
HandleScope scope(env);

napi_status status;
napi_value error = nullptr;
if (Napi::Env(env).IsExceptionPending()) {
Expand Down Expand Up @@ -1432,16 +1434,49 @@ inline Error Error::New(napi_env env, const std::string& message) {
return Error::New<Error>(env, message.c_str(), message.size(), napi_create_error);
}

inline Error::Error() : Object(), _message(nullptr) {
inline Error::Error() : ObjectReference(), _message(nullptr) {
}

inline Error::Error(napi_env env, napi_value value) : Object(env, value) {
inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullptr) {
if (value != nullptr) {
napi_status status = napi_create_reference(env, value, 1, &_ref);

// Avoid infinite recursion in the failure case.
// Don't try to construct & throw another Error instance.
assert(status == napi_ok);
}
}

inline Error::Error(Error&& other) : ObjectReference(std::move(other)) {
}

inline Error& Error::operator =(Error&& other) {
static_cast<Reference<Object>*>(this)->operator=(std::move(other));
return *this;
}

inline Error::Error(const Error& other) : Error(other.Env(), other.Value()) {
}

inline Error& Error::operator =(Error& other) {
Reset();

_env = other.Env();
HandleScope scope(_env);

napi_value value = other.Value();
if (value != nullptr) {
napi_status status = napi_create_reference(_env, value, 1, &_ref);
if (status != napi_ok) throw Error::New(_env);
}

return *this;
}

inline const std::string& Error::Message() const NAPI_NOEXCEPT {
if (_message.size() == 0 && _env != nullptr) {
try {
_message = (*this)["message"].As<String>();
_message = Get("message").As<String>();
}
catch (...) {
// Catch all errors here, to include e.g. a std::bad_alloc from
Expand All @@ -1453,8 +1488,10 @@ inline const std::string& Error::Message() const NAPI_NOEXCEPT {
}

inline void Error::ThrowAsJavaScriptException() const {
if (_value != nullptr) {
napi_throw(_env, _value);
HandleScope scope(_env);
if (!IsEmpty()) {
napi_status status = napi_throw(_env, Value());
if (status != napi_ok) throw Error::New(_env);
}
}

Expand Down Expand Up @@ -1532,8 +1569,8 @@ inline Reference<T>::Reference() : _env(nullptr), _ref(nullptr), _suppressDestru
}

template <typename T>
inline Reference<T>::Reference(napi_env env, napi_ref persistent)
: _env(env), _ref(persistent) {
inline Reference<T>::Reference(napi_env env, napi_ref ref)
: _env(env), _ref(ref) {
}

template <typename T>
Expand All @@ -1557,6 +1594,7 @@ inline Reference<T>::Reference(Reference<T>&& other) {

template <typename T>
inline Reference<T>& Reference<T>::operator =(Reference<T>&& other) {
Reset();
_env = other._env;
_ref = other._ref;
other._env = nullptr;
Expand Down Expand Up @@ -2626,7 +2664,7 @@ inline void AsyncWorker::WorkComplete() {
OnOK();
}
else {
OnError(_error.Value());
OnError(_error);
}
}

Expand All @@ -2639,11 +2677,12 @@ inline void AsyncWorker::OnOK() {
}

inline void AsyncWorker::OnError(Error e) {
_callback.MakeCallback(Env().Undefined(), std::vector<napi_value>({ e }));
HandleScope scope(Env());
_callback.MakeCallback(Env().Undefined(), std::vector<napi_value>({ e.Value() }));
}

inline void AsyncWorker::SetError(Error error) {
_error.Reset(error, 1);
_error = error;
}

inline void AsyncWorker::OnExecute(napi_env env, void* this_pointer) {
Expand Down
193 changes: 99 additions & 94 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -443,99 +443,6 @@ namespace Napi {
void EnsureInfo() const;
};

/*
* The NAPI Error class wraps a JavaScript Error object in a way that enables it
* to traverse a C++ stack and be thrown and caught as a C++ exception.
*
* If a NAPI API call fails without executing any JavaScript code (for example due
* to an invalid argument), then the NAPI wrapper automatically converts and throws
* the error as a C++ exception of type Napi::Error.
*
* If a JavaScript function called by C++ code via NAPI throws a JavaScript exception,
* then the NAPI wrapper automatically converts and throws it as a C++ exception of type
* Napi::Error.
*
* If a C++ exception of type Napi::Error escapes from a NAPI C++ callback, then
* the NAPI wrapper automatically converts and throws it as a JavaScript exception.
*
* Catching a C++ exception of type Napi::Error also clears the JavaScript exception.
* Of course it may be then re-thrown, which restores the JavaScript exception.
*
* Example 1 - Throwing an exception:
*
* Napi::Env env = ...
* throw env.NewError("Example exception");
* // Following C++ statements will not be executed.
* // The exception will bubble up as a C++ exception of type Napi::Error,
* // until it is either caught while still in C++, or else automatically
* // re-thrown as a JavaScript exception when the callback returns to JavaScript.
*
* Example 2 - Ignoring a NAPI exception:
*
* Napi::Function jsFunctionThatThrows = someObj.AsFunction();
* jsFunctionThatThrows({ arg1, arg2 });
* // Following C++ statements will not be executed.
* // The exception will bubble up as a C++ exception of type Napi::Error,
* // until it is either caught while still in C++, or else automatically
* // re-thrown as a JavaScript exception when the callback returns to JavaScript.
*
* Example 3 - Handling a NAPI exception:
*
* Napi::Function jsFunctionThatThrows = someObj.AsFunction();
* try {
* jsFunctionThatThrows({ arg1, arg2 });
* }
* catch (const Napi::Error& e) {
* cerr << "Caught JavaScript exception: " + e.what();
* // Since the exception was caught here, it will not be re-thrown as
* // a JavaScript exception.
* }
*/
class Error : public Object, public std::exception {
public:
static Error New(napi_env env);
static Error New(napi_env env, const char* message);
static Error New(napi_env env, const std::string& message);

Error();
Error(napi_env env, napi_value value);

const std::string& Message() const NAPI_NOEXCEPT;
void ThrowAsJavaScriptException() const;

const char* what() const NAPI_NOEXCEPT override;

protected:
typedef napi_status (*create_error_fn)(napi_env envb, napi_value msg, napi_value* result);

template <typename TError>
static TError New(napi_env env,
const char* message,
size_t length,
create_error_fn create_error);

private:
mutable std::string _message;
};

class TypeError : public Error {
public:
static TypeError New(napi_env env, const char* message);
static TypeError New(napi_env env, const std::string& message);

TypeError();
TypeError(napi_env env, napi_value value);
};

class RangeError : public Error {
public:
static RangeError New(napi_env env, const char* message);
static RangeError New(napi_env env, const std::string& message);

RangeError();
RangeError(napi_env env, napi_value value);
};

/*
* Holds a counted reference to a value; initially a weak reference unless otherwise specified.
* May be changed to/from a strong reference by adjusting the refcount. The referenced value
Expand Down Expand Up @@ -660,6 +567,104 @@ namespace Napi {
ObjectReference Persistent(Object value);
FunctionReference Persistent(Function value);

/*
* The NAPI Error class wraps a JavaScript Error object in a way that enables it
* to traverse a C++ stack and be thrown and caught as a C++ exception.
*
* If a NAPI API call fails without executing any JavaScript code (for example due
* to an invalid argument), then the NAPI wrapper automatically converts and throws
* the error as a C++ exception of type Napi::Error.
*
* If a JavaScript function called by C++ code via NAPI throws a JavaScript exception,
* then the NAPI wrapper automatically converts and throws it as a C++ exception of type
* Napi::Error.
*
* If a C++ exception of type Napi::Error escapes from a NAPI C++ callback, then
* the NAPI wrapper automatically converts and throws it as a JavaScript exception.
*
* Catching a C++ exception of type Napi::Error also clears the JavaScript exception.
* Of course it may be then re-thrown, which restores the JavaScript exception.
*
* Example 1 - Throwing a N-API exception:
*
* Napi::Env env = ...
* throw Napi::Error::New(env, "Example exception");
* // Following C++ statements will not be executed.
* // The exception will bubble up as a C++ exception of type Napi::Error,
* // until it is either caught while still in C++, or else automatically
* // re-thrown as a JavaScript exception when the callback returns to JavaScript.
*
* Example 2 - Ignoring a N-API exception:
*
* Napi::Function jsFunctionThatThrows = someObj.As<Napi::Function>();
* jsFunctionThatThrows({ arg1, arg2 });
* // Following C++ statements will not be executed.
* // The exception will bubble up as a C++ exception of type Napi::Error,
* // until it is either caught while still in C++, or else automatically
* // re-thrown as a JavaScript exception when the callback returns to JavaScript.
*
* Example 3 - Handling a N-API exception:
*
* Napi::Function jsFunctionThatThrows = someObj.As<Napi::Function>();
* try {
* jsFunctionThatThrows({ arg1, arg2 });
* } catch (const Napi::Error& e) {
* cerr << "Caught JavaScript exception: " + e.Message();
* // Since the exception was caught here, it will not be re-thrown as
* // a JavaScript exception.
* }
*/
class Error : public ObjectReference, public std::exception {
public:
static Error New(napi_env env);
static Error New(napi_env env, const char* message);
static Error New(napi_env env, const std::string& message);

Error();
Error(napi_env env, napi_value value);

// An error can be moved or copied.
Error(Error&& other);
Error& operator =(Error&& other);
Error(const Error&);
Error& operator =(Error&);

const std::string& Message() const NAPI_NOEXCEPT;
void ThrowAsJavaScriptException() const;

const char* what() const NAPI_NOEXCEPT override;

protected:
typedef napi_status (*create_error_fn)(napi_env envb, napi_value msg, napi_value* result);

template <typename TError>
static TError New(napi_env env,
const char* message,
size_t length,
create_error_fn create_error);

private:
mutable std::string _message;
};

class TypeError : public Error {
public:
static TypeError New(napi_env env, const char* message);
static TypeError New(napi_env env, const std::string& message);

TypeError();
TypeError(napi_env env, napi_value value);
};

class RangeError : public Error {
public:
static RangeError New(napi_env env, const char* message);
static RangeError New(napi_env env, const std::string& message);

RangeError();
RangeError(napi_env env, napi_value value);
};

class CallbackInfo {
public:
CallbackInfo(napi_env env, napi_callback_info info);
Expand Down Expand Up @@ -985,7 +990,7 @@ namespace Napi {

napi_env _env;
napi_async_work _work;
Reference<Error> _error;
Error _error;
};

} // namespace Napi
Expand Down
24 changes: 22 additions & 2 deletions test/error.cc
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ Value CatchError(const CallbackInfo& info) {
try {
thrower({});
} catch (const Error& e) {
return e;
return e.Value();
}
return info.Env().Null();
}
Expand All @@ -50,11 +50,28 @@ void CatchAndRethrowError(const CallbackInfo& info) {
try {
thrower({});
} catch (Error& e) {
e["caught"] = Boolean::New(info.Env(), true);
e.Set("caught", Boolean::New(info.Env(), true));
throw e;
}
}

void ThrowErrorThatEscapesScope(const CallbackInfo& info) {
HandleScope scope(info.Env());

std::string message = info[0].As<String>().Utf8Value();
throw Error::New(info.Env(), message);
}

void CatchAndRethrowErrorThatEscapesScope(const CallbackInfo& info) {
HandleScope scope(info.Env());
try {
ThrowErrorThatEscapesScope(info);
} catch (Error& e) {
e.Set("caught", Boolean::New(info.Env(), true));
throw e;
}
}

} // end anonymous namespace

Object InitError(Env env) {
Expand All @@ -66,5 +83,8 @@ Object InitError(Env env) {
exports["catchErrorMessage"] = Function::New(env, CatchErrorMessage);
exports["doNotCatch"] = Function::New(env, DoNotCatch);
exports["catchAndRethrowError"] = Function::New(env, CatchAndRethrowError);
exports["throwErrorThatEscapesScope"] = Function::New(env, ThrowErrorThatEscapesScope);
exports["catchAndRethrowErrorThatEscapesScope"] =
Function::New(env, CatchAndRethrowErrorThatEscapesScope);
return exports;
}
8 changes: 8 additions & 0 deletions test/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,11 @@ assert.strictEqual(err.message, 'test');
const msg = binding.error.catchErrorMessage(
() => { throw new TypeError('test'); });
assert.strictEqual(msg, 'test');

assert.throws(() => binding.error.throwErrorThatEscapesScope('test'), err => {
return err instanceof Error && err.message === 'test';
});

assert.throws(() => binding.error.catchAndRethrowErrorThatEscapesScope('test'), err => {
return err instanceof Error && err.message === 'test' && err.caught;
});

0 comments on commit 1f1bb5e

Please sign in to comment.