Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Error::Fatal #70

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 19 additions & 11 deletions napi-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

// Note: Do not include this file directly! Include "napi.h" instead.

#include <cassert>
#include <cstring>

namespace Napi {
Expand Down Expand Up @@ -42,6 +41,13 @@ namespace details {

#endif // NAPI_CPP_EXCEPTIONS

#define NAPI_FATAL_IF_FAILED(status, location, message) \
do { \
if ((status) != napi_ok) { \
Error::Fatal((location), (message)); \
} \
} while (0)

// For use in JS to C++ callback wrappers to catch any Napi::Error exceptions
// and rethrow them as JavaScript exceptions before returning from the callback.
template <typename Callable>
Expand Down Expand Up @@ -1418,24 +1424,24 @@ inline Error Error::New(napi_env env) {

const napi_extended_error_info* info;
status = napi_get_last_error_info(env, &info);
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_last_error_info");

if (status == napi_ok) {
if (info->error_code == napi_pending_exception) {
status = napi_get_and_clear_last_exception(env, &error);
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
}
else {
const char* error_message = info->error_message != nullptr ?
info->error_message : "Error in native callback";

bool isExceptionPending;
status = napi_is_exception_pending(env, &isExceptionPending);
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_is_exception_pending");

if (isExceptionPending) {
status = napi_get_and_clear_last_exception(env, &error);
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
}

napi_value message;
Expand All @@ -1444,7 +1450,7 @@ inline Error Error::New(napi_env env) {
error_message,
std::strlen(error_message),
&message);
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_string_utf8");

if (status == napi_ok) {
switch (info->error_code) {
Expand All @@ -1458,7 +1464,7 @@ inline Error Error::New(napi_env env) {
status = napi_create_error(env, nullptr, message, &error);
break;
}
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_error");
}
}
}
Expand All @@ -1474,6 +1480,10 @@ 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 NAPI_NO_RETURN void Error::Fatal(const char* location, const char* message) {
napi_fatal_error(location, message);
}

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

Expand All @@ -1483,7 +1493,7 @@ inline Error::Error(napi_env env, napi_value value) : ObjectReference(env, nullp

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

Expand Down Expand Up @@ -1661,9 +1671,7 @@ inline Reference<T>::Reference(const Reference<T>& other)
// Copying is a limited scenario (currently only used for Error object) and always creates a
// strong reference to the given value even if the incoming reference is weak.
napi_status status = napi_create_reference(_env, value, 1, &_ref);

// TODO - Switch to napi_fatal_error() once it exists.
assert(status == napi_ok);
NAPI_FATAL_IF_FAILED(status, "Reference<T>::Reference", "napi_create_reference");
}
}

Expand Down
2 changes: 2 additions & 0 deletions napi.h
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,8 @@ namespace Napi {
static Error New(napi_env env, const char* message);
static Error New(napi_env env, const std::string& message);

static NAPI_NO_RETURN void Fatal(const char* location, const char* message);

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

Expand Down
6 changes: 6 additions & 0 deletions src/node_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "uv.h"
#include "node_api.h"
#include "node_internals.h"
#include "util.h"

#define NAPI_VERSION 1

Expand Down Expand Up @@ -823,6 +824,11 @@ napi_status napi_get_last_error_info(napi_env env,
return napi_ok;
}

NAPI_NO_RETURN void napi_fatal_error(const char* location,
const char* message) {
node::FatalError(location, message);
}

napi_status napi_create_function(napi_env env,
const char* utf8name,
napi_callback cb,
Expand Down
9 changes: 9 additions & 0 deletions src/node_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@
# define NAPI_MODULE_EXPORT __attribute__((visibility("default")))
#endif

#ifdef __GNUC__
#define NAPI_NO_RETURN __attribute__((noreturn))
#else
#define NAPI_NO_RETURN
#endif


typedef void (*napi_addon_register_func)(napi_env env,
napi_value exports,
Expand Down Expand Up @@ -104,6 +110,9 @@ NAPI_EXTERN napi_status
napi_get_last_error_info(napi_env env,
const napi_extended_error_info** result);

NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location,
const char* message);

// Getters for defined singletons
NAPI_EXTERN napi_status napi_get_undefined(napi_env env, napi_value* result);
NAPI_EXTERN napi_status napi_get_null(napi_env env, napi_value* result);
Expand Down
5 changes: 5 additions & 0 deletions test/error.cc
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,10 @@ void CatchAndRethrowErrorThatEscapesScope(const CallbackInfo& info) {

#endif // NAPI_CPP_EXCEPTIONS

void ThrowFatalError(const CallbackInfo& info) {
Error::Fatal("Error::ThrowFatalError", "This is a fatal error");
}

} // end anonymous namespace

Object InitError(Env env) {
Expand All @@ -169,5 +173,6 @@ Object InitError(Env env) {
exports["throwErrorThatEscapesScope"] = Function::New(env, ThrowErrorThatEscapesScope);
exports["catchAndRethrowErrorThatEscapesScope"] =
Function::New(env, CatchAndRethrowErrorThatEscapesScope);
exports["throwFatalError"] = Function::New(env, ThrowFatalError);
return exports;
}
20 changes: 17 additions & 3 deletions test/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,18 @@
const buildType = process.config.target_defaults.default_configuration;
const assert = require('assert');

test(require(`./build/${buildType}/binding.node`));
test(require(`./build/${buildType}/binding_noexcept.node`));
if (process.argv[2] === 'fatal') {
const binding = require(process.argv[3]);
binding.error.throwFatalError();
return;
}

test(`./build/${buildType}/binding.node`);
test(`./build/${buildType}/binding_noexcept.node`);

function test(bindingPath) {
const binding = require(bindingPath);

function test(binding) {
assert.throws(() => binding.error.throwApiError('test'), err => {
return err instanceof Error && err.message.includes('Invalid');
});
Expand Down Expand Up @@ -56,4 +64,10 @@ function test(binding) {
assert.throws(() => binding.error.catchAndRethrowErrorThatEscapesScope('test'), err => {
return err instanceof Error && err.message === 'test' && err.caught;
});

const p = require('./napi_child').spawnSync(
process.execPath, [ __filename, 'fatal', bindingPath ]);
assert.ifError(p.error);
assert.ok(p.stderr.toString().includes(
'FATAL ERROR: Error::ThrowFatalError This is a fatal error'));
}
6 changes: 1 addition & 5 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,7 @@ if (typeof global.gc === 'function') {
console.log('\nAll tests passed!');
} else {
// Make it easier to run with the correct (version-dependent) command-line args.
const args = [ '--expose-gc', __filename ];
if (require('../index').isNodeApiBuiltin) {
args.splice(0, 0, '--napi-modules');
}
const child = require('child_process').spawnSync(process.argv[0], args, {
const child = require('./napi_child').spawnSync(process.argv[0], [ '--expose-gc', __filename ], {
stdio: 'inherit',
});
process.exitCode = child.status;
Expand Down
7 changes: 7 additions & 0 deletions test/napi_child.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
// Makes sure that child processes are spawned appropriately.
exports.spawnSync = function(command, args, options) {
if (require('../index').isNodeApiBuiltin) {
args.splice(0, 0, '--napi-modules');
}
return require('child_process').spawnSync(command, args, options);
};