[gjs/wip/ptomato/classes: 2/4] promise: Report unhandled rejections

[Philip Chimento <pchimento src gnome org>]

commit 74b7b2845cfdb9920d87386ef4023ef3d8d68564
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                      |    8 ++++++
 gjs/context.cpp                            |   37 ++++++++++++++++++++++++++++
 gjs/engine.cpp                             |   23 +++++++++++++++++
 installed-tests/scripts/testCommandLine.sh |   18 +++++++++++++-
 4 files changed, 85 insertions(+), 1 deletions(-)
---
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 824c47e..6dbe669 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -27,6 +27,7 @@
 #include <inttypes.h>
 
 #include "context.h"
+#include "jsapi-util.h"
 #include "jsapi-wrapper.h"
 
 G_BEGIN_DECLS
@@ -53,6 +54,13 @@ bool _gjs_context_enqueue_job(GjsContext      *gjs_context,
 
 bool _gjs_context_run_jobs(GjsContext *gjs_context);
 
+void _gjs_context_unregister_unhandled_promise_rejection(GjsContext *gjs_context,
+                                                         uint64_t    promise_id);
+
 G_END_DECLS
 
+void _gjs_context_register_unhandled_promise_rejection(GjsContext   *gjs_context,
+                                                       uint64_t      promise_id,
+                                                       GjsAutoChar&& stack);
+
 #endif  /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 9aecf13..3ea7e86 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>
 
@@ -90,6 +91,8 @@ struct _GjsContext {
     JS::PersistentRooted<JobQueue> *job_queue;
     unsigned idle_drain_handler;
     bool draining_job_queue;
+
+    std::unordered_map<uint64_t, GjsAutoChar> unhandled_rejection_stacks;
 };
 
 /* Keep this consistent with GjsConstString */
@@ -190,6 +193,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;
@@ -205,6 +223,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
@@ -564,6 +584,23 @@ _gjs_context_run_jobs(GjsContext *gjs_context)
     return retval;
 }
 
+void
+_gjs_context_register_unhandled_promise_rejection(GjsContext   *gjs_context,
+                                                  uint64_t      id,
+                                                  GjsAutoChar&& stack)
+{
+    gjs_context->unhandled_rejection_stacks[id] = std::move(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..a6d5f48 100644
--- a/gjs/engine.cpp
+++ b/gjs/engine.cpp
@@ -220,6 +220,27 @@ 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));
+    GjsAutoChar stack = gjs_format_stack_trace(cx, allocation_site);
+    _gjs_context_register_unhandled_promise_rejection(gjs_context, id,
+                                                      std::move(stack));
+}
+
 #ifdef G_OS_WIN32
 HMODULE gjs_dll;
 static bool gjs_is_inited = false;
@@ -305,6 +326,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"