[gjs/wip/ptomato/develop: 9/11] Capture JS exceptions from invoked virtual functions

[Philip Chimento <pchimento src gnome org>]

commit 0aa6bcdba5e8a3b1d575c7967ceb84795cb27be4
Author: Giovanni Campagna <gcampagna src gnome org>
Date:   Sun Aug 26 00:25:45 2012 +0200

    Capture JS exceptions from invoked virtual functions
    
    If a vfunc is annotated to throw, capture JS exceptions and turn
    them into GErrors instead of logging the error and silently
    returning null.
    This allows "throw new GLib.Error()" to do the expected thing
    inside a vfunc. If the vfunc does not throw, the exception is
    reported as usual.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=682701

 gi/closure.cpp                          |   16 +++++++++-
 gi/closure.h                            |    3 +-
 gi/function.cpp                         |   25 ++++++++++++++--
 gi/value.cpp                            |    2 +-
 installed-tests/js/testGIMarshalling.js |   49 +++++++++++++++++++++++++++++++
 5 files changed, 88 insertions(+), 7 deletions(-)
---
diff --git a/gi/closure.cpp b/gi/closure.cpp
index 9aad756..e5c3841 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -189,7 +189,8 @@ bool
 gjs_closure_invoke(GClosure                   *closure,
                    JS::HandleObject            this_obj,
                    const JS::HandleValueArray& args,
-                   JS::MutableHandleValue      retval)
+                   JS::MutableHandleValue      retval,
+                   bool                        return_exception)
 {
     Closure *c;
     JSContext *context;
@@ -218,9 +219,20 @@ gjs_closure_invoke(GClosure                   *closure,
         gjs_debug_closure("Closure invocation failed (exception should "
                           "have been thrown) closure %p callable %p",
                           closure, c->obj.get());
-        if (!gjs_log_exception(context))
+        /* If an exception has been thrown, log it, unless the caller
+         * explicitly wants to handle it manually (for example to turn it
+         * into a GError), in which case it replaces the return value
+         * (which is not valid anyway) */
+        if (JS_IsExceptionPending(context)) {
+            if (return_exception)
+                JS_GetPendingException(context, retval);
+            else
+                gjs_log_exception(context);
+        } else {
+            retval.setUndefined();
             gjs_debug_closure("Closure invocation failed but no exception was set?"
                               "closure %p", closure);
+        }
         return false;
     }
 
diff --git a/gi/closure.h b/gi/closure.h
index cbb9e51..0367b58 100644
--- a/gi/closure.h
+++ b/gi/closure.h
@@ -39,7 +39,8 @@ GClosure*  gjs_closure_new           (JSContext    *context,
 bool gjs_closure_invoke(GClosure                   *closure,
                         JS::HandleObject            this_obj,
                         const JS::HandleValueArray& args,
-                        JS::MutableHandleValue      retval);
+                        JS::MutableHandleValue      retval,
+                        bool                        return_exception);
 
 JSContext* gjs_closure_get_context   (GClosure     *closure);
 bool       gjs_closure_is_valid      (GClosure     *closure);
diff --git a/gi/function.cpp b/gi/function.cpp
index 8bc5a94..876cc90 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -206,6 +206,7 @@ gjs_callback_closure(ffi_cif *cif,
     JSAutoCompartment ac(context,
                          gjs_closure_get_callable(trampoline->js_function));
 
+    bool can_throw_gerror = g_callable_info_can_throw_gerror(trampoline->info);
     n_args = g_callable_info_get_n_args(trampoline->info);
 
     g_assert(n_args >= 0);
@@ -294,7 +295,8 @@ gjs_callback_closure(ffi_cif *cif,
         }
     }
 
-    if (!gjs_closure_invoke(trampoline->js_function, this_object, jsargs, &rval))
+    if (!gjs_closure_invoke(trampoline->js_function, this_object, jsargs, &rval,
+                            true))
         goto out;
 
     g_callable_info_load_return_type(trampoline->info, &ret_type);
@@ -418,11 +420,28 @@ out:
             exit(1);
         }
 
-        gjs_log_exception (context);
-
         /* Fill in the result with some hopefully neutral value */
         g_callable_info_load_return_type(trampoline->info, &ret_type);
         gjs_g_argument_init_default (context, &ret_type, (GArgument *) result);
+
+        /* If the callback has a GError** argument and invoking the closure
+         * returned an error, try to make a GError from it */
+        if (can_throw_gerror && rval.isObject()) {
+            JS::RootedObject exc_object(context, &rval.toObject());
+            GError *local_error = gjs_gerror_make_from_error(context, exc_object);
+
+            if (local_error) {
+                /* the GError ** pointer is the last argument, and is not
+                 * included in the n_args */
+                GIArgument *error_argument = args[n_args + c_args_offset];
+                auto gerror = static_cast<GError **>(error_argument->v_pointer);
+                g_propagate_error(gerror, local_error);
+                JS_ClearPendingException(context);  /* don't log */
+            }
+        } else if (!rval.isUndefined()) {
+            JS_SetPendingException(context, rval);
+        }
+        gjs_log_exception(context);
     }
 
     if (trampoline->scope == GI_SCOPE_TYPE_ASYNC) {
diff --git a/gi/value.cpp b/gi/value.cpp
index f6d7f69..acd57a9 100644
--- a/gi/value.cpp
+++ b/gi/value.cpp
@@ -269,7 +269,7 @@ closure_marshal(GClosure        *closure,
             g_base_info_unref((GIBaseInfo *)type_info_for[i]);
 
     JS::RootedValue rval(context);
-    gjs_closure_invoke(closure, nullptr, argv, &rval);
+    gjs_closure_invoke(closure, nullptr, argv, &rval, false);
 
     if (return_value != NULL) {
         if (rval.isUndefined()) {
diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js
index 4a7d438..713de01 100644
--- a/installed-tests/js/testGIMarshalling.js
+++ b/installed-tests/js/testGIMarshalling.js
@@ -574,6 +574,28 @@ const VFuncTester = GObject.registerClass(class VFuncTester extends GIMarshallin
     vfunc_vfunc_return_value_and_one_out_parameter() { return [46, 47]; }
     vfunc_vfunc_return_value_and_multiple_out_parameters() { return [48, 49, 50]; }
     vfunc_vfunc_array_out_parameter() { return [50, 51]; }
+    vfunc_vfunc_meth_with_err(x) {
+        switch (x) {
+        case -1:
+            return true;
+        case 0:
+            undefined.throw_type_error();
+            break;
+        case 1:
+            void reference_error;  // eslint-disable-line no-undef
+            break;
+        case 2:
+            throw new Gio.IOErrorEnum({
+                code: Gio.IOErrorEnum.FAILED,
+                message: 'I FAILED, but the test passed!',
+            });
+        case 3:
+            throw new GLib.SpawnError({
+                code: GLib.SpawnError.TOO_BIG,
+                message: 'This test is Too Big to Fail',
+            });
+        }
+    }
 });
 
 describe('Virtual function', function () {
@@ -607,6 +629,33 @@ describe('Virtual function', function () {
     it('marshals an array out parameter', function () {
         expect(tester.vfunc_array_out_parameter()).toEqual([50, 51]);
     });
+
+    it('marshals an error out parameter when no error', function () {
+        expect(tester.vfunc_meth_with_error(-1)).toBeTruthy();
+    });
+
+    it('marshals an error out parameter with a JavaScript exception', function () {
+        expect(() => tester.vfunc_meth_with_error(0)).toThrowError(TypeError);
+        expect(() => tester.vfunc_meth_with_error(1)).toThrowError(ReferenceError);
+    });
+
+    it('marshals an error out parameter with a GError exception', function () {
+        try {
+            tester.vfunc_meth_with_error(2);
+            fail('Exception should be thrown');
+        } catch (e) {
+            expect(e.matches(Gio.IOErrorEnum, Gio.IOErrorEnum.FAILED)).toBeTruthy();
+            expect(e.message).toEqual('I FAILED, but the test passed!');
+        }
+
+        try {
+            tester.vfunc_meth_with_error(3);
+            fail('Exception should be thrown');
+        } catch (e) {
+            expect(e.matches(GLib.SpawnError, GLib.SpawnError.TOO_BIG)).toBeTruthy();
+            expect(e.message).toEqual('This test is Too Big to Fail');
+        }
+    });
 });
 
 describe('Interface', function () {