From 0bca4b3e0ec7d5cacbe431af1c4c1ce49ddc9596 Mon Sep 17 00:00:00 2001 From: Alexander Borisov Date: Tue, 3 Nov 2020 15:31:41 +0300 Subject: [PATCH] Promise: tracking unhandled promise rejection. By default, promises should finish processing normally for .then(), .catch(), .finally() and so on. The patch adds the ability to report unhandled exception from promises to the user. This closes #346 issue on GitHub. --- nginx/ngx_http_js_module.c | 1 + nginx/ngx_stream_js_module.c | 1 + src/njs.h | 23 ++++--- src/njs_promise.c | 74 ++++++++++++++++++++++- src/njs_shell.c | 8 +++ src/njs_vm.c | 22 +++++++ src/njs_vm.h | 2 + test/js/promise_catch_then_throw_catch.js | 5 ++ test/js/promise_catch_throw.js | 3 + test/js/promise_finally_throw.js | 2 + test/js/promise_finally_throw_catch.js | 3 + test/js/promise_reject_catch.js | 1 + test/js/promise_reject_post_catch.js | 2 + test/js/promise_then_throw.js | 2 + test/js/promise_then_throw_catch.js | 3 + test/js/promise_two_first_then_throw.js | 6 ++ test/js/promise_two_then_throw.js | 5 ++ test/njs_expect_test.exp | 30 +++++++++ 18 files changed, 184 insertions(+), 9 deletions(-) create mode 100644 test/js/promise_catch_then_throw_catch.js create mode 100644 test/js/promise_catch_throw.js create mode 100644 test/js/promise_finally_throw.js create mode 100644 test/js/promise_finally_throw_catch.js create mode 100644 test/js/promise_reject_catch.js create mode 100644 test/js/promise_reject_post_catch.js create mode 100644 test/js/promise_then_throw.js create mode 100644 test/js/promise_then_throw_catch.js create mode 100644 test/js/promise_two_first_then_throw.js create mode 100644 test/js/promise_two_then_throw.js diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c index 4e2f0077..d1c631c7 100644 --- a/nginx/ngx_http_js_module.c +++ b/nginx/ngx_http_js_module.c @@ -3033,6 +3033,7 @@ ngx_http_js_init_main_conf(ngx_conf_t *cf, void *conf) njs_vm_opt_init(&options); options.backtrace = 1; + options.unhandled_rejection = NJS_VM_OPT_UNHANDLED_REJECTION_THROW; options.ops = &ngx_http_js_ops; options.argv = ngx_argv; options.argc = ngx_argc; diff --git a/nginx/ngx_stream_js_module.c b/nginx/ngx_stream_js_module.c index 5e71837e..e3b35a6a 100644 --- a/nginx/ngx_stream_js_module.c +++ b/nginx/ngx_stream_js_module.c @@ -1478,6 +1478,7 @@ ngx_stream_js_init_main_conf(ngx_conf_t *cf, void *conf) njs_vm_opt_init(&options); options.backtrace = 1; + options.unhandled_rejection = NJS_VM_OPT_UNHANDLED_REJECTION_THROW; options.ops = &ngx_stream_js_ops; options.argv = ngx_argv; options.argc = ngx_argc; diff --git a/src/njs.h b/src/njs.h index 1a1e9a4d..aae78442 100644 --- a/src/njs.h +++ b/src/njs.h @@ -195,18 +195,24 @@ typedef struct { char **argv; njs_uint_t argc; +#define NJS_VM_OPT_UNHANDLED_REJECTION_IGNORE 0 +#define NJS_VM_OPT_UNHANDLED_REJECTION_THROW 1 + /* - * accumulative - enables "accumulative" mode to support incremental compiling. + * accumulative - enables "accumulative" mode to support incremental compiling. * (REPL). Allows starting parent VM without cloning. - * disassemble - enables disassemble. - * backtrace - enables backtraces. - * quiet - removes filenames from backtraces. To produce comparable + * disassemble - enables disassemble. + * backtrace - enables backtraces. + * quiet - removes filenames from backtraces. To produce comparable test262 diffs. - * sandbox - "sandbox" mode. Disables file access. - * unsafe - enables unsafe language features: + * sandbox - "sandbox" mode. Disables file access. + * unsafe - enables unsafe language features: * - Function constructors. - * module - ES6 "module" mode. Script mode is default. - * ast - print AST. + * module - ES6 "module" mode. Script mode is default. + * ast - print AST. + * unhandled_rejection IGNORE | THROW - tracks unhandled promise rejections: + * - throwing inside a Promise without a catch block. + * - throwing inside in a finally or catch block. */ uint8_t trailer; /* 1 bit */ @@ -219,6 +225,7 @@ typedef struct { uint8_t unsafe; /* 1 bit */ uint8_t module; /* 1 bit */ uint8_t ast; /* 1 bit */ + uint8_t unhandled_rejection; } njs_vm_opt_t; diff --git a/src/njs_promise.c b/src/njs_promise.c index ee04f93a..c676f681 100644 --- a/src/njs_promise.c +++ b/src/njs_promise.c @@ -12,6 +12,11 @@ typedef enum { NJS_PROMISE_REJECTED } njs_promise_type_t; +typedef enum { + NJS_PROMISE_HANDLE = 0, + NJS_PROMISE_REJECT +} njs_promise_rejection_type_t; + typedef struct { njs_promise_type_t state; njs_value_t result; @@ -51,6 +56,8 @@ static njs_int_t njs_promise_value_constructor(njs_vm_t *vm, njs_value_t *value, njs_value_t *dst); static njs_int_t njs_promise_capability_executor(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t retval); +static njs_int_t njs_promise_host_rejection_tracker(njs_vm_t *vm, + njs_promise_t *promise, njs_promise_rejection_type_t operation); static njs_int_t njs_promise_resolve_function(njs_vm_t *vm, njs_value_t *args, njs_uint_t nargs, njs_index_t retval); static njs_promise_t *njs_promise_resolve(njs_vm_t *vm, @@ -497,6 +504,7 @@ njs_promise_fulfill(njs_vm_t *vm, njs_promise_t *promise, njs_value_t *value) njs_inline njs_value_t * njs_promise_reject(njs_vm_t *vm, njs_promise_t *promise, njs_value_t *reason) { + njs_int_t ret; njs_queue_t queue; njs_promise_data_t *data; @@ -505,6 +513,14 @@ njs_promise_reject(njs_vm_t *vm, njs_promise_t *promise, njs_value_t *reason) data->result = *reason; data->state = NJS_PROMISE_REJECTED; + if (!data->is_handled) { + ret = njs_promise_host_rejection_tracker(vm, promise, + NJS_PROMISE_REJECT); + if (njs_slow_path(ret != NJS_OK)) { + return njs_value_arg(&njs_value_null); + } + } + if (njs_queue_is_empty(&data->reject_queue)) { return njs_value_arg(&njs_value_undefined); @@ -522,6 +538,58 @@ njs_promise_reject(njs_vm_t *vm, njs_promise_t *promise, njs_value_t *reason) } +static njs_int_t +njs_promise_host_rejection_tracker(njs_vm_t *vm, njs_promise_t *promise, + njs_promise_rejection_type_t operation) +{ + uint32_t i, length; + njs_value_t *value; + njs_promise_data_t *data; + + if (vm->options.unhandled_rejection + == NJS_VM_OPT_UNHANDLED_REJECTION_IGNORE) + { + return NJS_OK; + } + + if (vm->promise_reason == NULL) { + vm->promise_reason = njs_array_alloc(vm, 1, 0, NJS_ARRAY_SPARE); + if (njs_slow_path(vm->promise_reason == NULL)) { + return NJS_ERROR; + } + } + + data = njs_data(&promise->value); + + if (operation == NJS_PROMISE_REJECT) { + if (vm->promise_reason != NULL) { + return njs_array_add(vm, vm->promise_reason, &data->result); + } + + } else { + value = vm->promise_reason->start; + length = vm->promise_reason->length; + + for (i = 0; i < length; i++) { + if (memcmp(&value[i], &data->result, sizeof(njs_value_t)) == 0) { + length--; + + if (i < length) { + memmove(&value[i], &value[i + 1], + sizeof(njs_value_t) * (length - i)); + } + + break; + } + } + + vm->promise_reason->length = length; + } + + return NJS_OK; +} + + static njs_int_t njs_promise_invoke_then(njs_vm_t *vm, njs_value_t *promise, njs_value_t *args, njs_int_t nargs) @@ -880,7 +948,11 @@ njs_promise_perform_then(njs_vm_t *vm, njs_value_t *value, if (data->state == NJS_PROMISE_REJECTED) { njs_set_data(&arguments[0], rejected_reaction, 0); - /* TODO: HostPromiseRejectionTracker */ + ret = njs_promise_host_rejection_tracker(vm, promise, + NJS_PROMISE_HANDLE); + if (njs_slow_path(ret != NJS_OK)) { + return ret; + } } else { njs_set_data(&arguments[0], fulfilled_reaction, 0); diff --git a/src/njs_shell.c b/src/njs_shell.c index ae11db04..8a878428 100644 --- a/src/njs_shell.c +++ b/src/njs_shell.c @@ -35,6 +35,7 @@ typedef struct { uint8_t safe; uint8_t version; uint8_t ast; + uint8_t unhandled_rejection; char *file; char *command; @@ -270,6 +271,7 @@ main(int argc, char **argv) vm_options.argv = opts.argv; vm_options.argc = opts.argc; vm_options.ast = opts.ast; + vm_options.unhandled_rejection = opts.unhandled_rejection; if (opts.interactive) { ret = njs_interactive_shell(&opts, &vm_options); @@ -315,6 +317,7 @@ njs_get_options(njs_opts_t *opts, int argc, char **argv) " -f disabled denormals mode.\n" " -p set path prefix for modules.\n" " -q disable interactive introduction prompt.\n" + " -r ignore unhandled promise rejection.\n" " -s sandbox mode.\n" " -t script|module source code type (script is default).\n" " -v print njs version and exit.\n" @@ -324,6 +327,7 @@ njs_get_options(njs_opts_t *opts, int argc, char **argv) ret = NJS_DONE; opts->denormals = 1; + opts->unhandled_rejection = NJS_VM_OPT_UNHANDLED_REJECTION_THROW; for (i = 1; i < argc; i++) { @@ -393,6 +397,10 @@ njs_get_options(njs_opts_t *opts, int argc, char **argv) opts->quiet = 1; break; + case 'r': + opts->unhandled_rejection = NJS_VM_OPT_UNHANDLED_REJECTION_IGNORE; + break; + case 's': opts->sandbox = 1; break; diff --git a/src/njs_vm.c b/src/njs_vm.c index 0b1269a4..a5740603 100644 --- a/src/njs_vm.c +++ b/src/njs_vm.c @@ -505,6 +505,8 @@ static njs_int_t njs_vm_handle_events(njs_vm_t *vm) { njs_int_t ret; + njs_str_t str; + njs_value_t string; njs_event_t *ev; njs_queue_t *promise_events, *posted_events; njs_queue_link_t *link; @@ -530,6 +532,26 @@ njs_vm_handle_events(njs_vm_t *vm) } } + if (vm->options.unhandled_rejection + == NJS_VM_OPT_UNHANDLED_REJECTION_THROW) + { + if (vm->promise_reason != NULL && vm->promise_reason->length != 0) { + ret = njs_value_to_string(vm, &string, + &vm->promise_reason->start[0]); + if (njs_slow_path(ret != NJS_OK)) { + return ret; + } + + njs_string_get(&string, &str); + njs_vm_error(vm, "unhandled promise rejection: %V", &str); + + njs_mp_free(vm->mem_pool, vm->promise_reason); + vm->promise_reason = NULL; + + return NJS_ERROR; + } + } + for ( ;; ) { link = njs_queue_first(posted_events); diff --git a/src/njs_vm.h b/src/njs_vm.h index 7a77b311..cb1af089 100644 --- a/src/njs_vm.h +++ b/src/njs_vm.h @@ -220,6 +220,8 @@ struct njs_vm_s { njs_regex_context_t *regex_context; njs_regex_match_data_t *single_match_data; + njs_array_t *promise_reason; + /* * MemoryError is statically allocated immutable Error object * with the InternalError prototype. diff --git a/test/js/promise_catch_then_throw_catch.js b/test/js/promise_catch_then_throw_catch.js new file mode 100644 index 00000000..b4f99e89 --- /dev/null +++ b/test/js/promise_catch_then_throw_catch.js @@ -0,0 +1,5 @@ +Promise.resolve() +.then(() => {}) +.catch(() => {}) +.then(() => {nonExsisting()}) +.catch(() => {console.log("Done")}); \ No newline at end of file diff --git a/test/js/promise_catch_throw.js b/test/js/promise_catch_throw.js new file mode 100644 index 00000000..9c246eb2 --- /dev/null +++ b/test/js/promise_catch_throw.js @@ -0,0 +1,3 @@ +Promise.resolve() +.then(() => {nonExsisting()}) +.catch(() => {nonExsistingInCatch()}); \ No newline at end of file diff --git a/test/js/promise_finally_throw.js b/test/js/promise_finally_throw.js new file mode 100644 index 00000000..7ae947e5 --- /dev/null +++ b/test/js/promise_finally_throw.js @@ -0,0 +1,2 @@ +Promise.resolve() +.finally(() => {nonExsistingInFinally()}); \ No newline at end of file diff --git a/test/js/promise_finally_throw_catch.js b/test/js/promise_finally_throw_catch.js new file mode 100644 index 00000000..03301be2 --- /dev/null +++ b/test/js/promise_finally_throw_catch.js @@ -0,0 +1,3 @@ +Promise.resolve() +.finally(() => {nonExsistingInFinally()}) +.catch(() => {console.log("Done")}); \ No newline at end of file diff --git a/test/js/promise_reject_catch.js b/test/js/promise_reject_catch.js new file mode 100644 index 00000000..81dff1f5 --- /dev/null +++ b/test/js/promise_reject_catch.js @@ -0,0 +1 @@ +Promise.reject("test").catch((x) => console.log('rejected', x)); \ No newline at end of file diff --git a/test/js/promise_reject_post_catch.js b/test/js/promise_reject_post_catch.js new file mode 100644 index 00000000..b2e617b7 --- /dev/null +++ b/test/js/promise_reject_post_catch.js @@ -0,0 +1,2 @@ +var p = Promise.reject(); +setImmediate(() => {p.catch(() => {})}); \ No newline at end of file diff --git a/test/js/promise_then_throw.js b/test/js/promise_then_throw.js new file mode 100644 index 00000000..0a14dfde --- /dev/null +++ b/test/js/promise_then_throw.js @@ -0,0 +1,2 @@ +Promise.resolve() +.then(() => {nonExsisting()}); \ No newline at end of file diff --git a/test/js/promise_then_throw_catch.js b/test/js/promise_then_throw_catch.js new file mode 100644 index 00000000..eee458bc --- /dev/null +++ b/test/js/promise_then_throw_catch.js @@ -0,0 +1,3 @@ +Promise.resolve() +.then(() => {nonExsisting()}) +.catch(() => {console.log("Done")}); \ No newline at end of file diff --git a/test/js/promise_two_first_then_throw.js b/test/js/promise_two_first_then_throw.js new file mode 100644 index 00000000..2752287b --- /dev/null +++ b/test/js/promise_two_first_then_throw.js @@ -0,0 +1,6 @@ +Promise.resolve() +.then(() => {nonExsistingOne()}); + +Promise.resolve() +.then(() => {nonExsistingTwo()}) +.catch(() => {}); diff --git a/test/js/promise_two_then_throw.js b/test/js/promise_two_then_throw.js new file mode 100644 index 00000000..af0a00b3 --- /dev/null +++ b/test/js/promise_two_then_throw.js @@ -0,0 +1,5 @@ +Promise.resolve() +.then(() => {nonExsistingOne()}); + +Promise.resolve() +.then(() => {nonExsistingTwo()}); \ No newline at end of file diff --git a/test/njs_expect_test.exp b/test/njs_expect_test.exp index 22e0c4dd..8781f1c5 100644 --- a/test/njs_expect_test.exp +++ b/test/njs_expect_test.exp @@ -1047,3 +1047,33 @@ njs_run {"./test/js/fs_promises_009.js"} \ njs_run {"./test/js/promise_then_throw_finally_catch.js"} \ "Done" + +njs_run {"./test/js/promise_catch_throw.js"} \ +"Error: unhandled promise rejection: ReferenceError: \"nonExsistingInCatch\" is not defined" + +njs_run {"./test/js/promise_then_throw.js"} \ +"Error: unhandled promise rejection: ReferenceError: \"nonExsisting\" is not defined" + +njs_run {"./test/js/promise_then_throw_catch.js"} \ +"Done" + +njs_run {"./test/js/promise_catch_then_throw_catch.js"} \ +"Done" + +njs_run {"./test/js/promise_finally_throw.js"} \ +"Error: unhandled promise rejection: ReferenceError: \"nonExsistingInFinally\" is not defined" + +njs_run {"./test/js/promise_finally_throw_catch.js"} \ +"Done" + +njs_run {"./test/js/promise_two_then_throw.js"} \ +"Error: unhandled promise rejection: ReferenceError: \"nonExsistingOne\" is not defined" + +njs_run {"./test/js/promise_two_first_then_throw.js"} \ +"Error: unhandled promise rejection: ReferenceError: \"nonExsistingOne\" is not defined" + +njs_run {"./test/js/promise_reject_catch.js"} \ +"rejected test" + +njs_run {"./test/js/promise_reject_post_catch.js"} \ +"Error: unhandled promise rejection: undefined" -- 2.47.3