[gjs/wip/ptomato/mozjs52] promise: Report unhandled rejections

[Philip Chimento <pchimento src gnome org>]

commit 0d72190a417594464e08b35cfe361bba96684012
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Jul 8 20:45:07 2017 -0700

    promise: Report unhandled rejections
    
    A known gotcha with promises is forgetting to attach a .catch() handler,
    in which case a failed promise is swallowed silently. This will log a
    warning when that happens.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=784713

 gjs/context-private.h                      |    7 +++++
 gjs/context.cpp                            |   37 ++++++++++++++++++++++++++++
 gjs/engine.cpp                             |   30 ++++++++++++++++++++++
 installed-tests/scripts/testCommandLine.sh |   18 +++++++++++++-
 4 files changed, 91 insertions(+), 1 deletions(-)
---
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 824c47e..700a2f7 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -53,6 +53,13 @@ bool _gjs_context_enqueue_job(GjsContext      *gjs_context,
 
 bool _gjs_context_run_jobs(GjsContext *gjs_context);
 
+void _gjs_context_register_unhandled_promise_rejection(GjsContext *gjs_context,
+                                                       uint64_t    promise_id,
+                                                       const char *stack);
+
+void _gjs_context_unregister_unhandled_promise_rejection(GjsContext *gjs_context,
+                                                         uint64_t    promise_id);
+
 G_END_DECLS
 
 #endif  /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index bfe1e7c..d0fdbd4 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -24,6 +24,7 @@
 #include <config.h>
 
 #include <array>
+#include <unordered_map>
 
 #include <gio/gio.h>
 
@@ -89,6 +90,8 @@ struct _GjsContext {
 
     JS::PersistentRooted<JobQueue> *job_queue;
     bool draining_job_queue;
+
+    std::unordered_map<uint64_t, GjsAutoChar> unhandled_rejection_stacks;
 };
 
 /* Keep this consistent with GjsConstString */
@@ -189,6 +192,21 @@ gjs_context_tracer(JSTracer *trc, void *data)
 }
 
 static void
+warn_about_unhandled_promise_rejections(GjsContext *gjs_context)
+{
+    for (auto& kv : gjs_context->unhandled_rejection_stacks) {
+        const char *stack = kv.second;
+        g_warning("Unhandled promise rejection. To suppress this warning, add "
+                  "an error handler to your promise chain with .catch() or a "
+                  "try-catch block around your await expression. %s%s",
+                  stack ? "Stack trace of the failed promise:\n" :
+                    "Unfortunately there is no stack trace of the failed promise.",
+                  stack ? stack : "");
+    }
+    gjs_context->unhandled_rejection_stacks.clear();
+}
+
+static void
 gjs_context_dispose(GObject *object)
 {
     GjsContext *js_context;
@@ -204,6 +222,8 @@ gjs_context_dispose(GObject *object)
         gjs_debug(GJS_DEBUG_CONTEXT,
                   "Destroying JS context");
 
+        warn_about_unhandled_promise_rejections(js_context);
+
         JS_BeginRequest(js_context->context);
 
         /* Do a full GC here before tearing down, since once we do
@@ -552,6 +572,23 @@ _gjs_context_run_jobs(GjsContext *gjs_context)
     return retval;
 }
 
+void
+_gjs_context_register_unhandled_promise_rejection(GjsContext *gjs_context,
+                                                  uint64_t    id,
+                                                  const char *stack)
+{
+    gjs_context->unhandled_rejection_stacks[id] = stack;
+}
+
+void
+_gjs_context_unregister_unhandled_promise_rejection(GjsContext *gjs_context,
+                                                    uint64_t    id)
+{
+    size_t erased = gjs_context->unhandled_rejection_stacks.erase(id);
+    g_assert(((void)"Handler attached to rejected promise that wasn't "
+              "previously marked as unhandled", erased == 1));
+}
+
 /**
  * gjs_context_maybe_gc:
  * @context: a #GjsContext
diff --git a/gjs/engine.cpp b/gjs/engine.cpp
index ea9ab3e..40d155a 100644
--- a/gjs/engine.cpp
+++ b/gjs/engine.cpp
@@ -220,6 +220,34 @@ on_enqueue_promise_job(JSContext       *cx,
     return _gjs_context_enqueue_job(gjs_context, callback);
 }
 
+static void
+on_promise_unhandled_rejection(JSContext                    *cx,
+                               JS::HandleObject              promise,
+                               PromiseRejectionHandlingState state,
+                               void                         *data)
+{
+    auto gjs_context = static_cast<GjsContext *>(data);
+    uint64_t id = JS::GetPromiseID(promise);
+
+    if (state == PromiseRejectionHandlingState::Handled) {
+        /* This happens when catching an exception from an await expression. */
+        _gjs_context_unregister_unhandled_promise_rejection(gjs_context, id);
+        return;
+    }
+
+    JS::RootedObject allocation_site(cx, JS::GetPromiseAllocationSite(promise));
+    JS::RootedString stack_str(cx);
+    GjsAutoJSChar stack_utf8(cx);
+    JS::AutoSaveExceptionState saved_exc(cx);
+    if (JS::BuildStackString(cx, allocation_site, &stack_str))
+        stack_utf8.reset(cx, JS_EncodeStringToUTF8(cx, stack_str));
+
+    _gjs_context_register_unhandled_promise_rejection(gjs_context, id,
+                                                      stack_utf8);
+
+    saved_exc.restore();
+}
+
 #ifdef G_OS_WIN32
 HMODULE gjs_dll;
 static bool gjs_is_inited = false;
@@ -305,6 +333,8 @@ gjs_create_js_context(GjsContext *js_context)
     JS::SetWarningReporter(cx, gjs_warning_reporter);
     JS::SetGetIncumbentGlobalCallback(cx, gjs_get_import_global);
     JS::SetEnqueuePromiseJobCallback(cx, on_enqueue_promise_job, js_context);
+    JS::SetPromiseRejectionTrackerCallback(cx, on_promise_unhandled_rejection,
+                                           js_context);
 
     /* setExtraWarnings: Be extra strict about code that might hide a bug */
     if (!g_getenv("GJS_DISABLE_EXTRA_WARNINGS")) {
diff --git a/installed-tests/scripts/testCommandLine.sh b/installed-tests/scripts/testCommandLine.sh
index 096b793..74ce17d 100755
--- a/installed-tests/scripts/testCommandLine.sh
+++ b/installed-tests/scripts/testCommandLine.sh
@@ -39,6 +39,17 @@ Promise.resolve().then(() => {
 Promise.resolve().then(() => print('Should not be printed'));
 EOF
 
+# this JS script should not cause an unhandled promise rejection
+cat <<EOF >awaitcatch.js
+async function foo() { throw new Error('foo'); }
+async function bar() {
+    try {
+        await foo();
+    } catch (e) {}
+}
+bar();
+EOF
+
 total=0
 
 report () {
@@ -147,6 +158,11 @@ report "interpreter should run queued promise jobs before finishing"
 test -n "${output##*Should not be printed*}"
 report "interpreter should stop running jobs when one calls System.exit()"
 
-rm -f exit.js help.js promise.js
+"$gjs" -c "Promise.resolve().then(() => { throw new Error(); });" 2>&1 | grep -q 'Gjs-WARNING.*Unhandled 
promise rejection.*[sS]tack trace'
+report "unhandled promise rejection should be reported"
+test -z $("$gjs" awaitcatch.js)
+report "catching an await expression should not cause unhandled rejection"
+
+rm -f exit.js help.js promise.js awaitcatch.js
 
 echo "1..$total"