[njs] Promise: tracking unhandled promise rejection.

Alexander Borisov alexander.borisov at nginx.com
Thu Nov 5 10:16:11 UTC 2020


details:   https://hg.nginx.org/njs/rev/2d1abd2d38b4
branches:  
changeset: 1559:2d1abd2d38b4
user:      Alexander Borisov <alexander.borisov at nginx.com>
date:      Tue Nov 03 15:31:41 2020 +0300
description:
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.

diffstat:

 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(-)

diffs (400 lines):

diff -r c791e3943df1 -r 2d1abd2d38b4 nginx/ngx_http_js_module.c
--- a/nginx/ngx_http_js_module.c	Tue Nov 03 20:14:33 2020 +0300
+++ b/nginx/ngx_http_js_module.c	Tue Nov 03 15:31:41 2020 +0300
@@ -3033,6 +3033,7 @@ ngx_http_js_init_main_conf(ngx_conf_t *c
     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 -r c791e3943df1 -r 2d1abd2d38b4 nginx/ngx_stream_js_module.c
--- a/nginx/ngx_stream_js_module.c	Tue Nov 03 20:14:33 2020 +0300
+++ b/nginx/ngx_stream_js_module.c	Tue Nov 03 15:31:41 2020 +0300
@@ -1478,6 +1478,7 @@ ngx_stream_js_init_main_conf(ngx_conf_t 
     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 -r c791e3943df1 -r 2d1abd2d38b4 src/njs.h
--- a/src/njs.h	Tue Nov 03 20:14:33 2020 +0300
+++ b/src/njs.h	Tue Nov 03 15:31:41 2020 +0300
@@ -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 -r c791e3943df1 -r 2d1abd2d38b4 src/njs_promise.c
--- a/src/njs_promise.c	Tue Nov 03 20:14:33 2020 +0300
+++ b/src/njs_promise.c	Tue Nov 03 15:31:41 2020 +0300
@@ -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_const
     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_pr
 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_pro
     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);
 
@@ -523,6 +539,58 @@ njs_promise_reject(njs_vm_t *vm, njs_pro
 
 
 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, n
         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 -r c791e3943df1 -r 2d1abd2d38b4 src/njs_shell.c
--- a/src/njs_shell.c	Tue Nov 03 20:14:33 2020 +0300
+++ b/src/njs_shell.c	Tue Nov 03 15:31:41 2020 +0300
@@ -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 ar
         "  -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 ar
     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 ar
             opts->quiet = 1;
             break;
 
+        case 'r':
+            opts->unhandled_rejection = NJS_VM_OPT_UNHANDLED_REJECTION_IGNORE;
+            break;
+
         case 's':
             opts->sandbox = 1;
             break;
diff -r c791e3943df1 -r 2d1abd2d38b4 src/njs_vm.c
--- a/src/njs_vm.c	Tue Nov 03 20:14:33 2020 +0300
+++ b/src/njs_vm.c	Tue Nov 03 15:31:41 2020 +0300
@@ -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 -r c791e3943df1 -r 2d1abd2d38b4 src/njs_vm.h
--- a/src/njs_vm.h	Tue Nov 03 20:14:33 2020 +0300
+++ b/src/njs_vm.h	Tue Nov 03 15:31:41 2020 +0300
@@ -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 -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_catch_then_throw_catch.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_catch_then_throw_catch.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,5 @@
+Promise.resolve()
+.then(() => {})
+.catch(() => {})
+.then(() => {nonExsisting()})
+.catch(() => {console.log("Done")});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_catch_throw.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_catch_throw.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,3 @@
+Promise.resolve()
+.then(() => {nonExsisting()})
+.catch(() => {nonExsistingInCatch()});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_finally_throw.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_finally_throw.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,2 @@
+Promise.resolve()
+.finally(() => {nonExsistingInFinally()});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_finally_throw_catch.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_finally_throw_catch.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,3 @@
+Promise.resolve()
+.finally(() => {nonExsistingInFinally()})
+.catch(() => {console.log("Done")});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_reject_catch.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_reject_catch.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,1 @@
+Promise.reject("test").catch((x) => console.log('rejected', x));
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_reject_post_catch.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_reject_post_catch.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,2 @@
+var p = Promise.reject();
+setImmediate(() => {p.catch(() => {})});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_then_throw.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_then_throw.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,2 @@
+Promise.resolve()
+.then(() => {nonExsisting()});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_then_throw_catch.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_then_throw_catch.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,3 @@
+Promise.resolve()
+.then(() => {nonExsisting()})
+.catch(() => {console.log("Done")});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_two_first_then_throw.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_two_first_then_throw.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,6 @@
+Promise.resolve()
+.then(() => {nonExsistingOne()});
+
+Promise.resolve()
+.then(() => {nonExsistingTwo()})
+.catch(() => {});
diff -r c791e3943df1 -r 2d1abd2d38b4 test/js/promise_two_then_throw.js
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/js/promise_two_then_throw.js	Tue Nov 03 15:31:41 2020 +0300
@@ -0,0 +1,5 @@
+Promise.resolve()
+.then(() => {nonExsistingOne()});
+
+Promise.resolve()
+.then(() => {nonExsistingTwo()});
\ No newline at end of file
diff -r c791e3943df1 -r 2d1abd2d38b4 test/njs_expect_test.exp
--- a/test/njs_expect_test.exp	Tue Nov 03 20:14:33 2020 +0300
+++ b/test/njs_expect_test.exp	Tue Nov 03 15:31:41 2020 +0300
@@ -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"


More information about the nginx-devel mailing list