Promise: do not track handled rejections with no list

Previously, when a handled promise was reported and the rejected promise
list was NULL, the trackers fell through and added it as unhandled.  In
the interactive shell, handling a promise after its rejection had been
reported caused another, false unhandled rejection.  The QuickJS nginx
module could do the same while formatting a rejection reason.

The fix is to return for every handled notification, removing the promise
from the list only when the list exists.  Apply this to both engines in the
shell and nginx module.

diff --git a/external/njs_shell.c b/external/njs_shell.c
index 1b9582a5410fcd6e1b642f400db28896dfdd92bb..276a2964e0e150cea061312741fb0e850cec0d1a 100644 (file)
--- a/external/njs_shell.c
+++ b/external/njs_shell.c
@@ -1024,20 +1024,23 @@ njs_rejection_tracker(njs_vm_t *vm, njs_external_ptr_t external,
 
     console = external;
 
-    if (is_handled && console->rejected_promises != NULL) {
-        rejected_promise = console->rejected_promises->start;
-        length = console->rejected_promises->items;
+    if (is_handled) {
+        if (console->rejected_promises != NULL) {
+            rejected_promise = console->rejected_promises->start;
+            length = console->rejected_promises->items;
 
-        promise_obj = njs_value_ptr(promise);
+            promise_obj = njs_value_ptr(promise);
 
-        for (i = 0; i < length; i++) {
-            if (njs_value_ptr(njs_value_arg(&rejected_promise[i].u.njs.promise))
-                == promise_obj)
-            {
-                njs_arr_remove(console->rejected_promises,
-                               &rejected_promise[i]);
+            for (i = 0; i < length; i++) {
+                if (njs_value_ptr(njs_value_arg(
+                                      &rejected_promise[i].u.njs.promise))
+                    == promise_obj)
+                {
+                    njs_arr_remove(console->rejected_promises,
+                                   &rejected_promise[i]);
 
-                break;
+                    break;
+                }
             }
         }
 
@@ -2536,22 +2539,24 @@ njs_qjs_rejection_tracker(JSContext *ctx, JSValueConst promise,
 
     console = opaque;
 
-    if (is_handled && console->rejected_promises != NULL) {
-        rejected_promise = console->rejected_promises->start;
-        length = console->rejected_promises->items;
+    if (is_handled) {
+        if (console->rejected_promises != NULL) {
+            rejected_promise = console->rejected_promises->start;
+            length = console->rejected_promises->items;
 
-        promise_obj = JS_VALUE_GET_PTR(promise);
+            promise_obj = JS_VALUE_GET_PTR(promise);
 
-        for (i = 0; i < length; i++) {
-            if (JS_VALUE_GET_PTR(rejected_promise[i].u.qjs.promise)
-                == promise_obj)
-            {
-                JS_FreeValue(ctx, rejected_promise[i].u.qjs.promise);
-                JS_FreeValue(ctx, rejected_promise[i].u.qjs.message);
-                njs_arr_remove(console->rejected_promises,
-                               &rejected_promise[i]);
+            for (i = 0; i < length; i++) {
+                if (JS_VALUE_GET_PTR(rejected_promise[i].u.qjs.promise)
+                    == promise_obj)
+                {
+                    JS_FreeValue(ctx, rejected_promise[i].u.qjs.promise);
+                    JS_FreeValue(ctx, rejected_promise[i].u.qjs.message);
+                    njs_arr_remove(console->rejected_promises,
+                                   &rejected_promise[i]);
 
-                break;
+                    break;
+                }
             }
         }
 

diff --git a/nginx/ngx_js.c b/nginx/ngx_js.c
index 55d8781fd1c7157ea56db22fa2ca837ca7738eec..cfa9ec14dbd1f02edc5d95a8858dc8524d7fcefb 100644 (file)
--- a/nginx/ngx_js.c
+++ b/nginx/ngx_js.c
@@ -2398,21 +2398,24 @@ ngx_qjs_rejection_tracker(JSContext *cx, JSValueConst promise,
 
     ctx = opaque;
 
-    if (is_handled && ctx->rejected_promises != NULL) {
-        rejected_promise = ctx->rejected_promises->start;
-        length = ctx->rejected_promises->items;
-
-        promise_obj = JS_VALUE_GET_PTR(promise);
-
-        for (i = 0; i < length; i++) {
-            if (JS_VALUE_GET_PTR(ngx_qjs_arg(rejected_promise[i].promise))
-                == promise_obj)
-            {
-                JS_FreeValue(cx, ngx_qjs_arg(rejected_promise[i].promise));
-                JS_FreeValue(cx, ngx_qjs_arg(rejected_promise[i].message));
-                njs_arr_remove(ctx->rejected_promises, &rejected_promise[i]);
-
-                break;
+    if (is_handled) {
+        if (ctx->rejected_promises != NULL) {
+            rejected_promise = ctx->rejected_promises->start;
+            length = ctx->rejected_promises->items;
+
+            promise_obj = JS_VALUE_GET_PTR(promise);
+
+            for (i = 0; i < length; i++) {
+                if (JS_VALUE_GET_PTR(ngx_qjs_arg(rejected_promise[i].promise))
+                    == promise_obj)
+                {
+                    JS_FreeValue(cx, ngx_qjs_arg(rejected_promise[i].promise));
+                    JS_FreeValue(cx, ngx_qjs_arg(rejected_promise[i].message));
+                    njs_arr_remove(ctx->rejected_promises,
+                                   &rejected_promise[i]);
+
+                    break;
+                }
             }
         }
 
@@ -4087,19 +4090,22 @@ ngx_js_rejection_tracker(njs_vm_t *vm, njs_external_ptr_t unused,
 
     ctx = ngx_external_ctx(vm, njs_vm_external_ptr(vm));
 
-    if (is_handled && ctx->rejected_promises != NULL) {
-        rejected_promise = ctx->rejected_promises->start;
-        length = ctx->rejected_promises->items;
+    if (is_handled) {
+        if (ctx->rejected_promises != NULL) {
+            rejected_promise = ctx->rejected_promises->start;
+            length = ctx->rejected_promises->items;
 
-        promise_obj = njs_value_ptr(promise);
+            promise_obj = njs_value_ptr(promise);
 
-        for (i = 0; i < length; i++) {
-            if (njs_value_ptr(njs_value_arg(&rejected_promise[i].promise))
-                == promise_obj)
-            {
-                njs_arr_remove(ctx->rejected_promises, &rejected_promise[i]);
+            for (i = 0; i < length; i++) {
+                if (njs_value_ptr(njs_value_arg(&rejected_promise[i].promise))
+                    == promise_obj)
+                {
+                    njs_arr_remove(ctx->rejected_promises,
+                                   &rejected_promise[i]);
 
-                break;
+                    break;
+                }
             }
         }
 

diff --git a/test/shell_test.exp b/test/shell_test.exp
index 9667b0bcfe37eb52f288e5ca1e73b58a98eb2d8f..f394070b68ced6ce945db27d2d85971126d3e194 100644 (file)
--- a/test/shell_test.exp
+++ b/test/shell_test.exp
@@ -19,6 +19,10 @@ proc njs_test {body {opts ""}} {
         set pair [lindex $body $i]
         send [lindex $pair 0]
         expect [lindex $pair 1]
+        if {[llength $pair] > 2
+            && [regexp [lindex $pair 2] $expect_out(buffer)]} {
+            return -code error "unexpected output: $expect_out(buffer)"
+        }
     }
 
     send "\n"
@@ -397,6 +401,13 @@ njs_test {
      "42\r\n"}
 }
 
+njs_test {
+    {"var rejected = Promise.reject('handled later')\r\n"
+     "unhandled promise rejection: handled later\r\n*>> "}
+    {"rejected.catch(() => {})\r\n"
+     ">> " "unhandled promise rejection"}
+}
+
 # CLI OPTIONS
 
 # help