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.

diff --git a/nginx/ngx_http_js_module.c b/nginx/ngx_http_js_module.c
index 4e2f007739b47a950d94ae0b6a669059c58c756d..d1c631c7045dd7dde6cace4c41f13ec8733af561 100644 (file)
--- 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 5e71837ea3ccc10ea174764cd07207743306f5be..e3b35a6a2c12389ddee6d70f6f4f55de7b538321 100644 (file)
--- 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 1a1e9a4da41b62c9ce3a594bdef85ded4e960f92..aae784423fb9828551530a02ef79644253fd9be2 100644 (file)
--- 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 ee04f93a6696b07f8dbf4b1d290797fd0c793807..c676f6819b5cff7b0596190ac192e81db4dc8867 100644 (file)
--- 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 ae11db04cd76cac859f7ccbcf9ad6ec103892355..8a878428ab2d21aa8f13e25d61ae3b20a5ace6ad 100644 (file)
--- 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 0b1269a492a41880540645e2519d84e5d3074b84..a574060336cdf3c972905509422927ef6273f9d4 100644 (file)
--- 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 7a77b3110ab1728e24fe9f953dad0711b9c10941..cb1af089419c113de266c3e184e9e0d3aae9893e 100644 (file)
--- 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 (file)
index 0000000..b4f99e8
--- /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 (file)
index 0000000..9c246eb
--- /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 (file)
index 0000000..7ae947e
--- /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 (file)
index 0000000..03301be
--- /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 (file)
index 0000000..81dff1f
--- /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 (file)
index 0000000..b2e617b
--- /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 (file)
index 0000000..0a14dfd
--- /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 (file)
index 0000000..eee458b
--- /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 (file)
index 0000000..2752287
--- /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 (file)
index 0000000..af0a00b
--- /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 22e0c4ddfe197bd600da4869319c23b5f0a00967..8781f1c5104eb4ccf0a1d1e84380eaa956bd8ec0 100644 (file)
--- 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"