[gjs: 4/5] gerror: Handle any value when converting thrown value to GError
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs: 4/5] gerror: Handle any value when converting thrown value to GError
- Date: Wed, 21 Apr 2021 04:57:27 +0000 (UTC)
commit 2df5407c156480102f24f921a64ac2e5d8f498fb
Author: Philip Chimento <philip chimento gmail com>
Date: Sat Apr 17 21:45:03 2021 -0700
gerror: Handle any value when converting thrown value to GError
In JS you can throw not only Error objects, but any value. If throwing a
value from a signal handler or vfunc with a GError** parameter, then that
value should be converted to a GError whether it is an Error object or
not.
Rename gjs_gerror_make_from_error() to gjs_gerror_make_from_thrown_value()
and refactor it to get the pending exception (thrown value) from the
context, and convert that into a GError.
If the object does not have a 'name' and 'message' property then it's
converted into a GJS_JS_ERROR_ERROR with an appropriate debug message; if
it does, then it's converted into the appropriate type of GJS_JS_ERROR.
gi/function.cpp | 36 ++++++++++----------------
gi/gerror.cpp | 45 +++++++++++++++++++++++++--------
gi/gerror.h | 3 +--
installed-tests/js/testGIMarshalling.js | 42 ++++++++++++++++++++++++++++++
4 files changed, 92 insertions(+), 34 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 5810adea..b0a27a7c 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -378,10 +378,6 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
JS::RootedValue rval(context);
g_callable_info_load_return_type(m_info, &ret_type);
- GIArgument* error_argument = nullptr;
-
- if (g_callable_info_can_throw_gerror(m_info))
- error_argument = args[n_args + c_args_offset];
if (!callback_closure_inner(context, this_object, &rval, args, &ret_type,
n_args, c_args_offset, result)) {
@@ -409,25 +405,21 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
set_return_ffi_arg_from_giargument(&ret_type, result, &argument);
}
- // If an exception has been thrown, log it. Unless the callback has a
- // GError** argument, then try to make a GError from it if it's an
- // object (and otherwise fall back to logging)
- if (error_argument) {
- JS::RootedValue v_exception(context);
- JS_GetPendingException(context, &v_exception);
- if (v_exception.isObject()) {
- JS_ClearPendingException(context); // don't log
- JS::RootedObject exc_object(context, &v_exception.toObject());
- GError* local_error =
- gjs_gerror_make_from_error(context, exc_object);
-
- // the GError ** pointer is the last argument, and is not
- // included in the n_args
- auto* gerror = gjs_arg_get<GError**>(error_argument);
- g_propagate_error(gerror, local_error);
- }
+ // If the callback has a GError** argument, then make a GError from the
+ // value that was thrown. Otherwise, log it as "uncaught" (critical
+ // instead of warning)
+
+ if (!g_callable_info_can_throw_gerror(m_info)) {
+ gjs_log_exception_uncaught(context);
+ return;
}
- gjs_log_exception_uncaught(context);
+
+ // 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 = gjs_arg_get<GError**>(error_argument);
+ GError* local_error = gjs_gerror_make_from_thrown_value(context);
+ g_propagate_error(gerror, local_error);
}
}
diff --git a/gi/gerror.cpp b/gi/gerror.cpp
index ddbae7db..08745a59 100644
--- a/gi/gerror.cpp
+++ b/gi/gerror.cpp
@@ -6,6 +6,8 @@
#include <stdint.h>
+#include <string>
+
#include <girepository.h>
#include <glib-object.h>
@@ -443,14 +445,20 @@ static GError* gerror_from_error_impl(JSContext* cx, JS::HandleObject obj) {
if (!JS_GetPropertyById(cx, obj, atoms.name(), &v_name))
return nullptr;
- JS::UniqueChars name = gjs_string_to_utf8(cx, v_name);
- if (!name)
- return nullptr;
-
JS::RootedValue v_message(cx);
if (!JS_GetPropertyById(cx, obj, atoms.message(), &v_message))
return nullptr;
+ if (!v_name.isString() || !v_message.isString()) {
+ return g_error_new_literal(
+ GJS_JS_ERROR, GJS_JS_ERROR_ERROR,
+ "Object thrown with unexpected name or message property");
+ }
+
+ JS::UniqueChars name = gjs_string_to_utf8(cx, v_name);
+ if (!name)
+ return nullptr;
+
JS::UniqueChars message = gjs_string_to_utf8(cx, v_message);
if (!message)
return nullptr;
@@ -467,15 +475,32 @@ static GError* gerror_from_error_impl(JSContext* cx, JS::HandleObject obj) {
}
/*
- * gjs_gerror_make_from_error:
+ * gjs_gerror_make_from_thrown_value:
*
- * Attempts to convert a JavaScript exception into a #GError. This function is
- * infallible and will always return a #GError with some message, even if the
- * exception object couldn't be converted.
+ * Attempts to convert a JavaScript thrown value (pending on @cx) into a
+ * #GError. This function is infallible and will always return a #GError with
+ * some message, even if the exception value couldn't be converted.
+ *
+ * Clears the pending exception on @cx.
*
* Returns: (transfer full): a new #GError
*/
-GError* gjs_gerror_make_from_error(JSContext* cx, JS::HandleObject obj) {
+GError* gjs_gerror_make_from_thrown_value(JSContext* cx) {
+ g_assert(JS_IsExceptionPending(cx) &&
+ "Should be called when an exception is pending");
+
+ JS::RootedValue exc(cx);
+ JS_GetPendingException(cx, &exc);
+ JS_ClearPendingException(cx); // don't log
+
+ if (!exc.isObject()) {
+ return g_error_new(GJS_JS_ERROR, GJS_JS_ERROR_ERROR,
+ "Non-exception %s value %s thrown",
+ JS::InformalValueTypeName(exc),
+ gjs_debug_value(exc).c_str());
+ }
+
+ JS::RootedObject obj(cx, &exc.toObject());
GError* retval = gerror_from_error_impl(cx, obj);
if (retval)
return retval;
@@ -484,7 +509,7 @@ GError* gjs_gerror_make_from_error(JSContext* cx, JS::HandleObject obj) {
// the exception into one
gjs_log_exception(cx); // log the inner exception
return g_error_new_literal(GJS_JS_ERROR, GJS_JS_ERROR_INTERNAL_ERROR,
- "Failed to convert JS exception into GError");
+ "Failed to convert JS thrown value into GError");
}
/*
diff --git a/gi/gerror.h b/gi/gerror.h
index f64112c0..8841a994 100644
--- a/gi/gerror.h
+++ b/gi/gerror.h
@@ -158,8 +158,7 @@ class ErrorInstance : public GIWrapperInstance<ErrorBase, ErrorPrototype,
};
GJS_JSAPI_RETURN_CONVENTION
-GError *gjs_gerror_make_from_error(JSContext *cx,
- JS::HandleObject obj);
+GError* gjs_gerror_make_from_thrown_value(JSContext* cx);
GJS_JSAPI_RETURN_CONVENTION
bool gjs_define_error_properties(JSContext* cx, JS::HandleObject obj);
diff --git a/installed-tests/js/testGIMarshalling.js b/installed-tests/js/testGIMarshalling.js
index 0be8fbd2..c8dec15c 100644
--- a/installed-tests/js/testGIMarshalling.js
+++ b/installed-tests/js/testGIMarshalling.js
@@ -1193,6 +1193,31 @@ let VFuncTester = GObject.registerClass(class VFuncTester extends GIMarshallingT
code: GLib.SpawnError.TOO_BIG,
message: 'This test is Too Big to Fail',
});
+ case 4:
+ throw null; // eslint-disable-line no-throw-literal
+ case 5:
+ throw undefined; // eslint-disable-line no-throw-literal
+ case 6:
+ throw 42; // eslint-disable-line no-throw-literal
+ case 7:
+ throw true; // eslint-disable-line no-throw-literal
+ case 8:
+ throw 'a string'; // eslint-disable-line no-throw-literal
+ case 9:
+ throw 42n; // eslint-disable-line no-throw-literal
+ case 10:
+ throw Symbol('a symbol');
+ case 11:
+ throw {plain: 'object'}; // eslint-disable-line no-throw-literal
+ case 12:
+ // eslint-disable-next-line no-throw-literal
+ throw {name: 'TypeError', message: 'an error message'};
+ case 13:
+ // eslint-disable-next-line no-throw-literal
+ throw {name: 1, message: 'an error message'};
+ case 14:
+ // eslint-disable-next-line no-throw-literal
+ throw {name: 'TypeError', message: false};
}
}
@@ -1373,6 +1398,23 @@ describe('Virtual function', function () {
}
});
+ it('marshals an error out parameter with a primitive value', function () {
+ expect(() => tester.vfunc_meth_with_error(4)).toThrowError(/null/);
+ expect(() => tester.vfunc_meth_with_error(5)).toThrowError(/undefined/);
+ expect(() => tester.vfunc_meth_with_error(6)).toThrowError(/42/);
+ expect(() => tester.vfunc_meth_with_error(7)).toThrowError(/true/);
+ expect(() => tester.vfunc_meth_with_error(8)).toThrowError(/"a string"/);
+ expect(() => tester.vfunc_meth_with_error(9)).toThrow(); // TODO(ptomato): toThrowError(/42n/)
+ expect(() => tester.vfunc_meth_with_error(10)).toThrowError(/Symbol\("a symbol"\)/);
+ });
+
+ it('marshals an error out parameter with a plain object', function () {
+ expect(() => tester.vfunc_meth_with_error(11)).toThrowError(/Object/);
+ expect(() => tester.vfunc_meth_with_error(12)).toThrowError(TypeError, /an error message/);
+ expect(() => tester.vfunc_meth_with_error(13)).toThrowError(/Object/);
+ expect(() => tester.vfunc_meth_with_error(14)).toThrowError(Error, /Object/);
+ });
+
it('marshals an enum return value', function () {
expect(tester.vfunc_return_enum()).toEqual(GIMarshallingTests.Enum.VALUE2);
});
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]