[gjs: 1/3] jsapi-util: Print error cause, if available, when logging error

[Philip Chimento <pchimento src gnome org>]

commit 33d58646d43b84d4c0ffc3681b89d125d5ccdfc6
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Feb 12 11:50:54 2022 -0800

    jsapi-util: Print error cause, if available, when logging error
    
    A new feature in SpiderMonkey 91 is the Error.cause property. If there is
    a cause property on an error that we log with gjs_log_exception(), append
    it to the original error's log message.
    
    Closes: #454

 gjs/atoms.h                          |  1 +
 gjs/jsapi-util.cpp                   | 95 ++++++++++++++++++++++++++++--------
 installed-tests/js/testExceptions.js | 45 +++++++++++++++++
 3 files changed, 120 insertions(+), 21 deletions(-)
---
diff --git a/gjs/atoms.h b/gjs/atoms.h
index 1e0b72fca..955165017 100644
--- a/gjs/atoms.h
+++ b/gjs/atoms.h
@@ -18,6 +18,7 @@ class JSTracer;
 
 // clang-format off
 #define FOR_EACH_ATOM(macro) \
+    macro(cause, "cause") \
     macro(code, "code") \
     macro(column_number, "columnNumber") \
     macro(connect_after, "connect_after") \
diff --git a/gjs/jsapi-util.cpp b/gjs/jsapi-util.cpp
index 6f46ca2f5..4168f3acb 100644
--- a/gjs/jsapi-util.cpp
+++ b/gjs/jsapi-util.cpp
@@ -25,7 +25,9 @@
 #include <js/ErrorReport.h>
 #include <js/Exception.h>
 #include <js/GCAPI.h>     // for JS_MaybeGC, NonIncrementalGC, GCRe...
+#include <js/GCHashTable.h>  // for GCHashSet
 #include <js/GCVector.h>  // for RootedVector
+#include <js/HashTable.h>  // for DefaultHasher
 #include <js/Object.h>    // for GetClass
 #include <js/RootingAPI.h>
 #include <js/String.h>
@@ -34,11 +36,16 @@
 #include <js/ValueArray.h>
 #include <jsapi.h>        // for JS_GetPropertyById, JS_InstanceOf
 #include <jsfriendapi.h>  // for ProtoKeyToClass
+#include <mozilla/HashTable.h>  // for HashSet::AddPtr
 
 #include "gjs/atoms.h"
 #include "gjs/context-private.h"
 #include "gjs/jsapi-util.h"
 
+namespace js {
+class SystemAllocPolicy;
+}
+
 static void
 throw_property_lookup_error(JSContext       *cx,
                             JS::HandleObject obj,
@@ -410,11 +417,72 @@ static std::string format_syntax_error_location(JSContext* cx,
     return out.str();
 }
 
+using CauseSet = JS::GCHashSet<JSObject*, js::DefaultHasher<JSObject*>,
+                               js::SystemAllocPolicy>;
+
+static std::string format_exception_with_cause(
+    JSContext* cx, JS::HandleObject exc_obj,
+    JS::MutableHandle<CauseSet> seen_causes) {
+    std::ostringstream out;
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+
+    JS::UniqueChars utf8_stack;
+    // Check both the internal SavedFrame object and the stack property.
+    // GErrors will not have the former, and internal errors will not
+    // have the latter.
+    JS::RootedObject saved_frame(cx, JS::ExceptionStackOrNull(exc_obj));
+    JS::RootedString str(cx);
+    if (saved_frame) {
+        JS::BuildStackString(cx, nullptr, saved_frame, &str, 0);
+    } else {
+        JS::RootedValue stack(cx);
+        if (JS_GetPropertyById(cx, exc_obj, atoms.stack(), &stack) &&
+            stack.isString())
+            str = stack.toString();
+    }
+    if (str)
+        utf8_stack = JS_EncodeStringToUTF8(cx, str);
+    if (utf8_stack)
+        out << '\n' << utf8_stack.get();
+    JS_ClearPendingException(cx);
+
+    // COMPAT: use JS::GetExceptionCause, mozjs 91.6 and later, on Error objects
+    // in order to avoid side effects
+    JS::RootedValue v_cause(cx);
+    if (!JS_GetPropertyById(cx, exc_obj, atoms.cause(), &v_cause))
+        JS_ClearPendingException(cx);
+    if (v_cause.isUndefined())
+        return out.str();
+
+    JS::RootedObject cause(cx);
+    if (v_cause.isObject()) {
+        cause = &v_cause.toObject();
+        CauseSet::AddPtr entry = seen_causes.lookupForAdd(cause);
+        if (entry)
+            return out.str();  // cause has been printed already, ref cycle
+        if (!seen_causes.add(entry, cause))
+            return out.str();  // out of memory, just stop here
+    }
+
+    out << "Caused by: ";
+    JS::RootedString exc_str(cx, exception_to_string(cx, v_cause));
+    if (exc_str) {
+        JS::UniqueChars utf8_exception = JS_EncodeStringToUTF8(cx, exc_str);
+        if (utf8_exception)
+            out << utf8_exception.get();
+    }
+    JS_ClearPendingException(cx);
+
+    if (v_cause.isObject())
+        out << format_exception_with_cause(cx, cause, seen_causes);
+
+    return out.str();
+}
+
 static std::string format_exception_log_message(JSContext* cx,
                                                 JS::HandleValue exc,
                                                 JS::HandleString message) {
     std::ostringstream out;
-    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
 
     if (message) {
         JS::UniqueChars utf8_message = JS_EncodeStringToUTF8(cx, message);
@@ -440,30 +508,15 @@ static std::string format_exception_log_message(JSContext* cx,
         // We log syntax errors differently, because the stack for those
         // includes only the referencing module, but we want to print out the
         // file name, line number, and column number from the exception.
+        // We assume that syntax errors have no cause property, and are not the
+        // cause of other exceptions, so no recursion.
         out << format_syntax_error_location(cx, exc_obj);
         return out.str();
     }
 
-    JS::UniqueChars utf8_stack;
-    // Check both the internal SavedFrame object and the stack property.
-    // GErrors will not have the former, and internal errors will not
-    // have the latter.
-    JS::RootedObject saved_frame(cx, JS::ExceptionStackOrNull(exc_obj));
-    JS::RootedString str(cx);
-    if (saved_frame) {
-        JS::BuildStackString(cx, nullptr, saved_frame, &str, 0);
-    } else {
-        JS::RootedValue stack(cx);
-        if (JS_GetPropertyById(cx, exc_obj, atoms.stack(), &stack) &&
-            stack.isString())
-            str = stack.toString();
-    }
-    if (str)
-        utf8_stack = JS_EncodeStringToUTF8(cx, str);
-    if (utf8_stack)
-        out << '\n' << utf8_stack.get();
-    JS_ClearPendingException(cx);
-
+    JS::Rooted<CauseSet> seen_causes(cx);
+    seen_causes.putNew(exc_obj);
+    out << format_exception_with_cause(cx, exc_obj, &seen_causes);
     return out.str();
 }
 
diff --git a/installed-tests/js/testExceptions.js b/installed-tests/js/testExceptions.js
index fbc32bc20..17c61da2f 100644
--- a/installed-tests/js/testExceptions.js
+++ b/installed-tests/js/testExceptions.js
@@ -166,6 +166,51 @@ describe('logError', function () {
             logError(e);
         }
     });
+
+    it('logs an error with cause', function marker() {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            'JS ERROR: Error: an error\nmarker@*Caused by: Gio.IOErrorEnum: another error\nmarker2@*');
+        function marker2() {
+            return new Gio.IOErrorEnum({message: 'another error', code: 0});
+        }
+        logError(new Error('an error', {cause: marker2()}));
+    });
+
+    it('logs a GError with cause', function marker() {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            'JS ERROR: Gio.IOErrorEnum: an error\nmarker@*Caused by: Error: another error\nmarker2@*');
+        function marker2() {
+            return new Error('another error');
+        }
+        const e = new Gio.IOErrorEnum({message: 'an error', code: 0});
+        e.cause = marker2();
+        logError(e);
+    });
+
+    it('logs an error with non-object cause', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            'JS ERROR: Error: an error\n*Caused by: 3');
+        logError(new Error('an error', {cause: 3}));
+    });
+
+    it('logs an error with a cause tree', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            'JS ERROR: Error: one\n*Caused by: Error: two\n*Caused by: Error: three\n*');
+        const three = new Error('three');
+        const two = new Error('two', {cause: three});
+        logError(new Error('one', {cause: two}));
+    });
+
+    it('logs an error with cyclical causes', function () {
+        // We cannot assert here with GLib.test_expect_message that the * at the
+        // end of the string doesn't match more causes, but at least the idea is
+        // that it shouldn't go into an infinite loop
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            'JS ERROR: Error: one\n*Caused by: Error: two\n*');
+        const one = new Error('one');
+        one.cause = new Error('two', {cause: one});
+        logError(one);
+    });
 });
 
 describe('Exception from function with too few arguments', function () {