From 7fefdda4b21ad035719f1e378b0018be9795b481 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Wed, 14 Mar 2018 03:14:08 +0100 Subject: [PATCH 1/6] n-api: test uncaught exception in worker --- test/addons-napi/test_async/test-uncaught.js | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 test/addons-napi/test_async/test-uncaught.js diff --git a/test/addons-napi/test_async/test-uncaught.js b/test/addons-napi/test_async/test-uncaught.js new file mode 100644 index 00000000000000..fdcb3203f54410 --- /dev/null +++ b/test/addons-napi/test_async/test-uncaught.js @@ -0,0 +1,18 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const test_async = require(`./build/${common.buildType}/test_async`); + +process.on('uncaughtException', common.mustCall(function(err) { + try { + throw new Error('should not fail'); + } catch (err) { + assert.strictEqual(err.message, 'should not fail'); + } + assert.strictEqual(err.message, 'uncaught'); +})); + +// Successful async execution and completion callback. +test_async.Test(5, {}, common.mustCall(function() { + throw new Error('uncaught'); +})); From e602e9c6a599990e3b123c2101784e13bfee4235 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Wed, 14 Mar 2018 03:05:32 +0100 Subject: [PATCH 2/6] n-api: fix fatalexception handling in workers --- src/node_api.cc | 9 +++++---- src/node_internals.h | 5 +++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 63ce1d8e86955e..d129aa6ef21d4d 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -3354,10 +3354,11 @@ class Work : public node::AsyncResource { // report it as a fatal exception. (There is no JavaScript on the // callstack that can possibly handle it.) if (!env->last_exception.IsEmpty()) { - v8::TryCatch try_catch(env->isolate); - env->isolate->ThrowException( - v8::Local::New(env->isolate, env->last_exception)); - node::FatalException(env->isolate, try_catch); + v8::Local err = v8::Local::New( + env->isolate, env->last_exception); + v8::Local msg = v8::Exception::CreateMessage( + env->isolate, err); + node::FatalException(env->isolate, err, msg); } } } diff --git a/src/node_internals.h b/src/node_internals.h index 2faa6f93475ad7..3d67b4e1d0774f 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -250,6 +250,11 @@ void GetSockOrPeerName(const v8::FunctionCallbackInfo& args) { args.GetReturnValue().Set(err); } +void FatalException(v8::Isolate* isolate, + v8::Local error, + v8::Local message); + + void SignalExit(int signo); #ifdef __POSIX__ void RegisterSignalHandler(int signal, From a575495353c82a11df978151facfdf9df719027c Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Wed, 14 Mar 2018 03:38:36 +0100 Subject: [PATCH 3/6] n-api: add napi_fatal_exception --- doc/api/n-api.md | 13 +++++++++++++ src/node_api.cc | 18 +++++++++++------- src/node_api.h | 2 ++ 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index d67d4280517824..0b687a986ac6f7 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -541,6 +541,19 @@ This API returns true if an exception is pending. This API can be called even if there is a pending JavaScript exception. +#### napi_fatal_exception + +```C +napi_fatal_exception(napi_env env); +``` + +- `[in] env`: The environment that the API is invoked under. + +Trigger an `uncaughtException` in JavaScript. Useful if an async +callback throws an exception with no way to recover. + ### Fatal Errors In the event of an unrecoverable error in a native module, a fatal error can be diff --git a/src/node_api.cc b/src/node_api.cc index d129aa6ef21d4d..c0af104156d8d9 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -954,6 +954,16 @@ napi_status napi_get_last_error_info(napi_env env, return napi_ok; } +void napi_fatal_exception(napi_env env) { + if (!env->last_exception.IsEmpty()) { + v8::Local err = v8::Local::New( + env->isolate, env->last_exception); + v8::Local msg = v8::Exception::CreateMessage( + env->isolate, err); + node::FatalException(env->isolate, err, msg); + } +} + NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, const char* message, @@ -3353,13 +3363,7 @@ class Work : public node::AsyncResource { // If there was an unhandled exception in the complete callback, // report it as a fatal exception. (There is no JavaScript on the // callstack that can possibly handle it.) - if (!env->last_exception.IsEmpty()) { - v8::Local err = v8::Local::New( - env->isolate, env->last_exception); - v8::Local msg = v8::Exception::CreateMessage( - env->isolate, err); - node::FatalException(env->isolate, err, msg); - } + napi_fatal_exception(env); } } diff --git a/src/node_api.h b/src/node_api.h index e9b3645e404530..60b2295ac9c2a7 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -112,6 +112,8 @@ NAPI_EXTERN napi_status napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); +NAPI_EXTERN void napi_fatal_exception(napi_env env); + NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, const char* message, From de3c603d97286bc9b7dd8bff78f09d99acdcc14d Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Wed, 14 Mar 2018 23:00:03 +0100 Subject: [PATCH 4/6] TBA -> REPLACEME --- doc/api/n-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 0b687a986ac6f7..96398d760042e8 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -543,7 +543,7 @@ This API can be called even if there is a pending JavaScript exception. #### napi_fatal_exception ```C napi_fatal_exception(napi_env env); From aa5498bf7f4b4c28ec007e5d269f7a240e8cdeb5 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Thu, 15 Mar 2018 19:01:39 +0100 Subject: [PATCH 5/6] add test and support passing in an error --- doc/api/n-api.md | 3 +- src/node_api.cc | 30 +++++++++++++------ src/node_api.h | 2 +- .../test_fatal_exception/binding.gyp | 8 +++++ test/addons-napi/test_fatal_exception/test.js | 11 +++++++ .../test_fatal_exception.c | 26 ++++++++++++++++ 6 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 test/addons-napi/test_fatal_exception/binding.gyp create mode 100644 test/addons-napi/test_fatal_exception/test.js create mode 100644 test/addons-napi/test_fatal_exception/test_fatal_exception.c diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 96398d760042e8..1871b1905cd46e 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -546,10 +546,11 @@ This API can be called even if there is a pending JavaScript exception. added: REPLACEME --> ```C -napi_fatal_exception(napi_env env); +napi_status napi_fatal_exception(napi_env env, napi_value err); ``` - `[in] env`: The environment that the API is invoked under. +- `[in] err`: The error you want to pass to `uncaughtException`. Trigger an `uncaughtException` in JavaScript. Useful if an async callback throws an exception with no way to recover. diff --git a/src/node_api.cc b/src/node_api.cc index c0af104156d8d9..3b98dc475f594b 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -167,6 +167,14 @@ struct napi_env__ { (out) = v8::type::New((buffer), (byte_offset), (length)); \ } while (0) +#define TRIGGER_FATAL_EXCEPTION(env, local_err) \ + do { \ + v8::Local local_msg = v8::Exception::CreateMessage( \ + env->isolate, (local_err)); \ + node::FatalException((env)->isolate, (local_err), local_msg); \ + } while (0) + + namespace { namespace v8impl { @@ -954,14 +962,14 @@ napi_status napi_get_last_error_info(napi_env env, return napi_ok; } -void napi_fatal_exception(napi_env env) { - if (!env->last_exception.IsEmpty()) { - v8::Local err = v8::Local::New( - env->isolate, env->last_exception); - v8::Local msg = v8::Exception::CreateMessage( - env->isolate, err); - node::FatalException(env->isolate, err, msg); - } +napi_status napi_fatal_exception(napi_env env, napi_value err) { + NAPI_PREAMBLE(env); + CHECK_ARG(env, err); + + v8::Local local_err = v8impl::V8LocalValueFromJsValue(err); + TRIGGER_FATAL_EXCEPTION(env, local_err); + + return napi_clear_last_error(env); } NAPI_NO_RETURN void napi_fatal_error(const char* location, @@ -3363,7 +3371,11 @@ class Work : public node::AsyncResource { // If there was an unhandled exception in the complete callback, // report it as a fatal exception. (There is no JavaScript on the // callstack that can possibly handle it.) - napi_fatal_exception(env); + if (!env->last_exception.IsEmpty()) { + v8::Local local_err = v8::Local::New( + env->isolate, env->last_exception); + TRIGGER_FATAL_EXCEPTION(env, local_err); + } } } diff --git a/src/node_api.h b/src/node_api.h index 60b2295ac9c2a7..d2a0886d5a058c 100644 --- a/src/node_api.h +++ b/src/node_api.h @@ -112,7 +112,7 @@ NAPI_EXTERN napi_status napi_get_last_error_info(napi_env env, const napi_extended_error_info** result); -NAPI_EXTERN void napi_fatal_exception(napi_env env); +NAPI_EXTERN napi_status napi_fatal_exception(napi_env env, napi_value err); NAPI_EXTERN NAPI_NO_RETURN void napi_fatal_error(const char* location, size_t location_len, diff --git a/test/addons-napi/test_fatal_exception/binding.gyp b/test/addons-napi/test_fatal_exception/binding.gyp new file mode 100644 index 00000000000000..f4dc0a71ea2817 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/binding.gyp @@ -0,0 +1,8 @@ +{ + "targets": [ + { + "target_name": "test_fatal_exception", + "sources": [ "test_fatal_exception.c" ] + } + ] +} diff --git a/test/addons-napi/test_fatal_exception/test.js b/test/addons-napi/test_fatal_exception/test.js new file mode 100644 index 00000000000000..f02b9bce1e8169 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/test.js @@ -0,0 +1,11 @@ +'use strict'; +const common = require('../../common'); +const assert = require('assert'); +const test_fatal = require(`./build/${common.buildType}/test_fatal_exception`); + +process.on('uncaughtException', common.mustCall(function(err) { + assert.strictEqual(err.message, 'fatal error'); +})); + +const err = new Error('fatal error'); +test_fatal.Test(err); diff --git a/test/addons-napi/test_fatal_exception/test_fatal_exception.c b/test/addons-napi/test_fatal_exception/test_fatal_exception.c new file mode 100644 index 00000000000000..fd81c56d856db8 --- /dev/null +++ b/test/addons-napi/test_fatal_exception/test_fatal_exception.c @@ -0,0 +1,26 @@ +#include +#include "../common.h" + +napi_value Test(napi_env env, napi_callback_info info) { + napi_value err; + size_t argc = 1; + + NAPI_CALL(env, napi_get_cb_info(env, info, &argc, &err, NULL, NULL)); + + NAPI_CALL(env, napi_fatal_exception(env, err)); + + return NULL; +} + +napi_value Init(napi_env env, napi_value exports) { + napi_property_descriptor properties[] = { + DECLARE_NAPI_PROPERTY("Test", Test), + }; + + NAPI_CALL(env, napi_define_properties( + env, exports, sizeof(properties) / sizeof(*properties), properties)); + + return exports; +} + +NAPI_MODULE(NODE_GYP_MODULE_NAME, Init) From 5ff7ba41f16731313e9f1d122fd7f718e94b5cc1 Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Thu, 15 Mar 2018 20:56:23 +0100 Subject: [PATCH 6/6] use inline fun instead of macro --- src/node_api.cc | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/node_api.cc b/src/node_api.cc index 3b98dc475f594b..ed97f39e553135 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -167,13 +167,6 @@ struct napi_env__ { (out) = v8::type::New((buffer), (byte_offset), (length)); \ } while (0) -#define TRIGGER_FATAL_EXCEPTION(env, local_err) \ - do { \ - v8::Local local_msg = v8::Exception::CreateMessage( \ - env->isolate, (local_err)); \ - node::FatalException((env)->isolate, (local_err), local_msg); \ - } while (0) - namespace { namespace v8impl { @@ -297,6 +290,13 @@ v8::Local V8LocalValueFromJsValue(napi_value v) { return local; } +static inline void trigger_fatal_exception( + napi_env env, v8::Local local_err) { + v8::Local local_msg = + v8::Exception::CreateMessage(env->isolate, local_err); + node::FatalException(env->isolate, local_err, local_msg); +} + static inline napi_status V8NameFromPropertyDescriptor(napi_env env, const napi_property_descriptor* p, v8::Local* result) { @@ -967,7 +967,7 @@ napi_status napi_fatal_exception(napi_env env, napi_value err) { CHECK_ARG(env, err); v8::Local local_err = v8impl::V8LocalValueFromJsValue(err); - TRIGGER_FATAL_EXCEPTION(env, local_err); + v8impl::trigger_fatal_exception(env, local_err); return napi_clear_last_error(env); } @@ -3374,7 +3374,7 @@ class Work : public node::AsyncResource { if (!env->last_exception.IsEmpty()) { v8::Local local_err = v8::Local::New( env->isolate, env->last_exception); - TRIGGER_FATAL_EXCEPTION(env, local_err); + v8impl::trigger_fatal_exception(env, local_err); } } }