Skip to content

Commit 685c215

Browse files
committed
Add support for Error::Fatal
* Added static method `Error::Fatal` to invoke `napi_fatal_error` * Added `node_internals.cc/h` to shim missing internal functions * Added a test for `Error::Fatal` * Replaced usage of assert with calls to `Error::Fatal`
1 parent 92d14c0 commit 685c215

File tree

8 files changed

+65
-19
lines changed

8 files changed

+65
-19
lines changed

napi-inl.h

+19-11
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

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

12-
#include <cassert>
1312
#include <cstring>
1413

1514
namespace Napi {
@@ -42,6 +41,13 @@ namespace details {
4241

4342
#endif // NAPI_CPP_EXCEPTIONS
4443

44+
#define NAPI_FATAL_IF_FAILED(status, location, message) \
45+
do { \
46+
if ((status) != napi_ok) { \
47+
Error::Fatal((location), (message)); \
48+
} \
49+
} while (0)
50+
4551
// For use in JS to C++ callback wrappers to catch any Napi::Error exceptions
4652
// and rethrow them as JavaScript exceptions before returning from the callback.
4753
template <typename Callable>
@@ -1418,24 +1424,24 @@ inline Error Error::New(napi_env env) {
14181424

14191425
const napi_extended_error_info* info;
14201426
status = napi_get_last_error_info(env, &info);
1421-
assert(status == napi_ok);
1427+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_last_error_info");
14221428

14231429
if (status == napi_ok) {
14241430
if (info->error_code == napi_pending_exception) {
14251431
status = napi_get_and_clear_last_exception(env, &error);
1426-
assert(status == napi_ok);
1432+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
14271433
}
14281434
else {
14291435
const char* error_message = info->error_message != nullptr ?
14301436
info->error_message : "Error in native callback";
14311437

14321438
bool isExceptionPending;
14331439
status = napi_is_exception_pending(env, &isExceptionPending);
1434-
assert(status == napi_ok);
1440+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_is_exception_pending");
14351441

14361442
if (isExceptionPending) {
14371443
status = napi_get_and_clear_last_exception(env, &error);
1438-
assert(status == napi_ok);
1444+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_get_and_clear_last_exception");
14391445
}
14401446

14411447
napi_value message;
@@ -1444,7 +1450,7 @@ inline Error Error::New(napi_env env) {
14441450
error_message,
14451451
std::strlen(error_message),
14461452
&message);
1447-
assert(status == napi_ok);
1453+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_string_utf8");
14481454

14491455
if (status == napi_ok) {
14501456
switch (info->error_code) {
@@ -1458,7 +1464,7 @@ inline Error Error::New(napi_env env) {
14581464
status = napi_create_error(env, message, &error);
14591465
break;
14601466
}
1461-
assert(status == napi_ok);
1467+
NAPI_FATAL_IF_FAILED(status, "Error::New", "napi_create_error");
14621468
}
14631469
}
14641470
}
@@ -1474,6 +1480,10 @@ inline Error Error::New(napi_env env, const std::string& message) {
14741480
return Error::New<Error>(env, message.c_str(), message.size(), napi_create_error);
14751481
}
14761482

1483+
inline NAPI_NO_RETURN void Error::Fatal(const char* location, const char* message) {
1484+
napi_fatal_error(location, message);
1485+
}
1486+
14771487
inline Error::Error() : ObjectReference(), _message(nullptr) {
14781488
}
14791489

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

14841494
// Avoid infinite recursion in the failure case.
14851495
// Don't try to construct & throw another Error instance.
1486-
assert(status == napi_ok);
1496+
NAPI_FATAL_IF_FAILED(status, "Error::Error", "napi_create_reference");
14871497
}
14881498
}
14891499

@@ -1661,9 +1671,7 @@ inline Reference<T>::Reference(const Reference<T>& other)
16611671
// Copying is a limited scenario (currently only used for Error object) and always creates a
16621672
// strong reference to the given value even if the incoming reference is weak.
16631673
napi_status status = napi_create_reference(_env, value, 1, &_ref);
1664-
1665-
// TODO - Switch to napi_fatal_error() once it exists.
1666-
assert(status == napi_ok);
1674+
NAPI_FATAL_IF_FAILED(status, "Reference<T>::Reference", "napi_create_reference");
16671675
}
16681676
}
16691677

napi.h

+2
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,8 @@ namespace Napi {
10671067
static Error New(napi_env env, const char* message);
10681068
static Error New(napi_env env, const std::string& message);
10691069

1070+
static NAPI_NO_RETURN void Fatal(const char* location, const char* message);
1071+
10701072
Error();
10711073
Error(napi_env env, napi_value value);
10721074

src/node_api.cc

+5
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,11 @@ napi_status napi_get_last_error_info(napi_env env,
823823
return napi_ok;
824824
}
825825

826+
NAPI_NO_RETURN void napi_fatal_error(const char* location,
827+
const char* message) {
828+
node::FatalError(location, message);
829+
}
830+
826831
napi_status napi_create_function(napi_env env,
827832
const char* utf8name,
828833
napi_callback cb,

src/node_api.h

+9
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@
3737
# define NAPI_MODULE_EXPORT __attribute__((visibility("default")))
3838
#endif
3939

40+
#ifdef __GNUC__
41+
#define NAPI_NO_RETURN __attribute__((noreturn))
42+
#else
43+
#define NAPI_NO_RETURN
44+
#endif
45+
4046

4147
typedef void (*napi_addon_register_func)(napi_env env,
4248
napi_value exports,
@@ -104,6 +110,9 @@ NAPI_EXTERN napi_status
104110
napi_get_last_error_info(napi_env env,
105111
const napi_extended_error_info** result);
106112

113+
NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location,
114+
const char* message);
115+
107116
// Getters for defined singletons
108117
NAPI_EXTERN napi_status napi_get_undefined(napi_env env, napi_value* result);
109118
NAPI_EXTERN napi_status napi_get_null(napi_env env, napi_value* result);

test/error.cc

+5
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,10 @@ void CatchAndRethrowErrorThatEscapesScope(const CallbackInfo& info) {
154154

155155
#endif // NAPI_CPP_EXCEPTIONS
156156

157+
void ThrowFatalError(const CallbackInfo& info) {
158+
Error::Fatal("Error::ThrowFatalError", "This is a fatal error");
159+
}
160+
157161
} // end anonymous namespace
158162

159163
Object InitError(Env env) {
@@ -169,5 +173,6 @@ Object InitError(Env env) {
169173
exports["throwErrorThatEscapesScope"] = Function::New(env, ThrowErrorThatEscapesScope);
170174
exports["catchAndRethrowErrorThatEscapesScope"] =
171175
Function::New(env, CatchAndRethrowErrorThatEscapesScope);
176+
exports["throwFatalError"] = Function::New(env, ThrowFatalError);
172177
return exports;
173178
}

test/error.js

+17-3
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,18 @@
22
const buildType = process.config.target_defaults.default_configuration;
33
const assert = require('assert');
44

5-
test(require(`./build/${buildType}/binding.node`));
6-
test(require(`./build/${buildType}/binding_noexcept.node`));
5+
if (process.argv[2] === 'fatal') {
6+
const binding = require(process.argv[3]);
7+
binding.error.throwFatalError();
8+
return;
9+
}
10+
11+
test(`./build/${buildType}/binding.node`);
12+
test(`./build/${buildType}/binding_noexcept.node`);
13+
14+
function test(bindingPath) {
15+
const binding = require(bindingPath);
716

8-
function test(binding) {
917
assert.throws(() => binding.error.throwApiError('test'), err => {
1018
return err instanceof Error && err.message.includes('Invalid');
1119
});
@@ -56,4 +64,10 @@ function test(binding) {
5664
assert.throws(() => binding.error.catchAndRethrowErrorThatEscapesScope('test'), err => {
5765
return err instanceof Error && err.message === 'test' && err.caught;
5866
});
67+
68+
const p = require('./napi_child').spawnSync(
69+
process.execPath, [ __filename, 'fatal', bindingPath ]);
70+
assert.ifError(p.error);
71+
assert.ok(p.stderr.toString().includes(
72+
'FATAL ERROR: Error::ThrowFatalError This is a fatal error'));
5973
}

test/index.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,7 @@ if (typeof global.gc === 'function') {
2424
console.log('\nAll tests passed!');
2525
} else {
2626
// Make it easier to run with the correct (version-dependent) command-line args.
27-
const args = [ '--expose-gc', __filename ];
28-
if (require('../index').isNodeApiBuiltin) {
29-
args.splice(0, 0, '--napi-modules');
30-
}
31-
const child = require('child_process').spawnSync(process.argv[0], args, {
27+
const child = require('./napi_child').spawnSync(process.argv[0], [ '--expose-gc', __filename ], {
3228
stdio: 'inherit',
3329
});
3430
process.exitCode = child.status;

test/napi_child.js

+7
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Makes sure that child processes are spawned appropriately.
2+
exports.spawnSync = function(command, args, options) {
3+
if (require('../index').isNodeApiBuiltin) {
4+
args.splice(0, 0, '--napi-modules');
5+
}
6+
return require('child_process').spawnSync(command, args, options);
7+
};

0 commit comments

Comments
 (0)