From a63e82315ba11717be9b75b290333d9559783e07 Mon Sep 17 00:00:00 2001 From: Matpi Date: Fri, 8 Apr 2022 12:57:12 +0200 Subject: [PATCH] Fix #63 #66, supersede #67: Wrap Python exceptions into custom InternalPythonError object extending InternalError. --- module.c | 121 ++++++++++++++++++++++++++++++++++++++++++-- quickjs/__init__.py | 1 + test_quickjs.py | 67 +++++++++++++++++++++++- 3 files changed, 185 insertions(+), 4 deletions(-) diff --git a/module.c b/module.c index 86de17c..86dd086 100644 --- a/module.c +++ b/module.c @@ -38,8 +38,67 @@ typedef struct { JSValue object; } ObjectData; +static JSClassID js_internal_python_error_class_id; + +typedef struct { + PyObject *error; + ContextData *context; +} JSInternalPythonErrorData; + +static void js_internal_python_error_finalizer(JSRuntime *rt, JSValue value) +{ + JSInternalPythonErrorData *d = JS_GetOpaque(value, js_internal_python_error_class_id); + if (d) { + // NOTE: This may be called from quickjs_exception_to_python, but also from + // e.g. JS_Eval, so we need to ensure that we are in the correct state. + if (d->context->thread_state) { + PyEval_RestoreThread(d->context->thread_state); + } + Py_DECREF(d->error); + if (d->context->thread_state) { + d->context->thread_state = PyEval_SaveThread(); + } + js_free(d->context->context, d); + }; +} + +static JSClassDef js_internal_python_error_class = { + "InternalPythonError", + .finalizer = js_internal_python_error_finalizer, +}; + +static JSValue js_internal_python_error_ctor(JSContext *ctx, JSValueConst new_target, + int argc, JSValueConst *argv) +{ + JSValue proto; + if (JS_IsUndefined(new_target)) { + proto = JS_GetClassProto(ctx, js_internal_python_error_class_id); + } else { + proto = JS_GetPropertyStr(ctx, new_target, "prototype"); + } + if (JS_IsException(proto)) { + return proto; + } + JSValue obj = JS_NewObjectProtoClass(ctx, proto, js_internal_python_error_class_id); + JS_FreeValue(ctx, proto); + if (JS_IsException(obj)) { + return obj; + } + if (!JS_IsUndefined(argv[0])) { + JSValue msg = JS_ToString(ctx, argv[0]); + if (JS_IsException(msg)) { + JS_FreeValue(ctx, obj); + return msg; + } + JS_DefinePropertyValueStr(ctx, obj, "message", msg, + JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE); + } + return obj; +} + // The exception raised by this module. static PyObject *JSException = NULL; +static PyObject *JSPythonException = NULL; static PyObject *StackOverflow = NULL; // Converts a JSValue to a Python object. // @@ -253,7 +312,8 @@ static PyObject *object_call(ObjectData *self, PyObject *args, PyObject *kwds) { return quickjs_to_python(self->context, value); } -// Converts the current Javascript exception to a Python exception via a C string. +// Converts the current Javascript exception to a Python exception via a C string +// or via the internal Python exception if it exists. static void quickjs_exception_to_python(JSContext *context) { JSValue exception = JS_GetException(context); const char *cstring = JS_ToCString(context, exception); @@ -267,7 +327,19 @@ static void quickjs_exception_to_python(JSContext *context) { } if (cstring != NULL) { const char *safe_stack_cstring = stack_cstring ? stack_cstring : ""; - if (strstr(cstring, "stack overflow") != NULL) { + JSInternalPythonErrorData *error_data = JS_GetOpaque(exception, + js_internal_python_error_class_id); + if (error_data) { + PyErr_Format(JSPythonException, "%s\n%s", cstring, safe_stack_cstring); + PyObject *type; + PyObject *value; + PyObject *traceback; + PyErr_Fetch(&type, &value, &traceback); + PyErr_NormalizeException(&type, &value, &traceback); + Py_INCREF(error_data->error); + PyException_SetCause(value, error_data->error); + PyErr_Restore(type, value, traceback); + } else if (strstr(cstring, "stack overflow") != NULL) { PyErr_Format(StackOverflow, "%s\n%s", cstring, safe_stack_cstring); } else { PyErr_Format(JSException, "%s\n%s", cstring, safe_stack_cstring); @@ -371,6 +443,23 @@ static PyObject *context_new(PyTypeObject *type, PyObject *args, PyObject *kwds) // _quickjs.Context can be used concurrently. self->runtime = JS_NewRuntime(); self->context = JS_NewContext(self->runtime); + JS_NewClass(self->runtime, js_internal_python_error_class_id, + &js_internal_python_error_class); + JSValue global = JS_GetGlobalObject(self->context); + JSValue ie_cls = JS_GetPropertyStr(self->context, global, "InternalError"); + JSValue ie_proto = JS_GetPropertyStr(self->context, ie_cls, "prototype"); + JS_FreeValue(self->context, ie_cls); + JSValue proto = JS_NewObjectProto(self->context, ie_proto); + JS_FreeValue(self->context, ie_proto); + JS_SetClassProto(self->context, js_internal_python_error_class_id, proto); + JSValue ctor = JS_NewCFunction2(self->context, js_internal_python_error_ctor, + "InternalPythonError", 1, JS_CFUNC_constructor_or_func, 0); + JS_SetConstructor(self->context, ctor, proto); + JS_FreeValue(self->context, proto); + JS_DefinePropertyValueStr(self->context, global, "InternalPythonError", ctor, + JS_PROP_WRITABLE | JS_PROP_CONFIGURABLE); + JS_FreeValue(self->context, ctor); + JS_FreeValue(self->context, global); self->has_time_limit = 0; self->time_limit = 0; self->thread_state = NULL; @@ -634,8 +723,27 @@ static JSValue js_c_function( PyObject *result = PyObject_CallObject(node->obj, args); Py_DECREF(args); if (!result) { + // NOTE: First throw and catch a standard error just to get a proper stack. + JS_ThrowInternalError(ctx, ""); + JSValue error_with_stack = JS_GetException(ctx); + JSValue error = JS_NewObjectClass(ctx, js_internal_python_error_class_id); + JS_SetPropertyStr(ctx, error, "message", JS_NewString(ctx, "Python call failed")); + JS_SetPropertyStr(ctx, error, "stack", JS_GetPropertyStr(ctx, error_with_stack, "stack")); + JS_FreeValue(ctx, error_with_stack); + PyObject *type; + JSInternalPythonErrorData *error_data = js_malloc(ctx, sizeof(JSInternalPythonErrorData)); + error_data->context = context; + PyObject *traceback; + PyErr_Fetch(&type, &error_data->error, &traceback); + PyErr_NormalizeException(&type, &error_data->error, &traceback); + if (traceback) { + PyException_SetTraceback(error_data->error, traceback); + Py_DECREF(traceback); + } + Py_DECREF(type); + JS_SetOpaque(error, error_data); end_call_python(context); - return JS_ThrowInternalError(ctx, "Python call failed."); + return JS_Throw(ctx, error); } JSValue js_result = JS_NULL; if (python_to_quickjs_possible(context, result)) { @@ -763,10 +871,16 @@ PyMODINIT_FUNC PyInit__quickjs(void) { return NULL; } + JS_NewClassID(&js_internal_python_error_class_id); + JSException = PyErr_NewException("_quickjs.JSException", NULL, NULL); if (JSException == NULL) { return NULL; } + JSPythonException = PyErr_NewException("_quickjs.JSPythonException", JSException, NULL); + if (JSPythonException == NULL) { + return NULL; + } StackOverflow = PyErr_NewException("_quickjs.StackOverflow", JSException, NULL); if (StackOverflow == NULL) { return NULL; @@ -777,6 +891,7 @@ PyMODINIT_FUNC PyInit__quickjs(void) { Py_INCREF(&Object); PyModule_AddObject(module, "Object", (PyObject *)&Object); PyModule_AddObject(module, "JSException", JSException); + PyModule_AddObject(module, "JSPythonException", JSPythonException); PyModule_AddObject(module, "StackOverflow", StackOverflow); return module; } diff --git a/quickjs/__init__.py b/quickjs/__init__.py index 7216fa6..2ddd89e 100755 --- a/quickjs/__init__.py +++ b/quickjs/__init__.py @@ -13,6 +13,7 @@ def test(): Context = _quickjs.Context Object = _quickjs.Object JSException = _quickjs.JSException +JSPythonException = _quickjs.JSPythonException StackOverflow = _quickjs.StackOverflow diff --git a/test_quickjs.py b/test_quickjs.py index 75b2a50..a816bee 100644 --- a/test_quickjs.py +++ b/test_quickjs.py @@ -241,7 +241,6 @@ def test_time_limit_disallowed(self): def test_conversion_failure_does_not_raise_system_error(self): # https://github.com/PetterS/quickjs/issues/38 - def test_list(): return [1, 2, 3] @@ -251,6 +250,72 @@ def test_list(): # instead of a JS exception. self.context.eval("test_list()") + def test_python_exception_is_exposed(self): + # https://github.com/PetterS/quickjs/issues/63 + def python_raise(): + 1/0 + + self.context.add_callable("python_raise", python_raise) + with self.assertRaises(quickjs.JSPythonException): + self.context.eval("python_raise()") + try: + self.context.eval("python_raise()") + except quickjs.JSPythonException as e: + self.assertIsInstance(e.__cause__, ZeroDivisionError) + + def test_python_exception_with_object_return_does_not_raise_system_error(self): + # https://github.com/PetterS/quickjs/issues/66 + def python_raise(): + raise Exception + + self.context.add_callable("python_raise", python_raise) + # When called, `a` should return an object (a promise), + # even though a Python error is generated in the background. + self.context.eval("async function a() {await python_raise();}") + # With incorrect error handling, this raised a SystemError in dev builds, + # and segfaulted in prod builds. + self.assertEqual(self.context.eval("typeof a();"), "object") + + def test_python_exception_with_nested_call(self): + # https://github.com/PetterS/quickjs/pull/67 + def python_raise(): + raise Exception + + def python_nested(): + self.context.eval("python_raise()") + + self.context.add_callable("python_raise", python_raise) + self.context.add_callable("python_nested", python_nested) + with self.assertRaisesRegex(quickjs.JSPythonException, "Python call failed"): + self.context.eval("python_nested()") + + def test_python_exception_instanceof_internalerror(self): + def python_raise(): + raise Exception + + self.context.add_callable("python_raise", python_raise) + self.assertTrue(self.context.eval(""" + (function() { + try{ + python_raise(); + } + catch (e) { + return e instanceof InternalPythonError + && e instanceof InternalError + && e.message === "Python call failed" + } + })(); + """)) + self.assertTrue(self.context.eval(""" + (new InternalPythonError) instanceof InternalPythonError + && (new InternalPythonError) instanceof InternalError + && !((new InternalError) instanceof InternalPythonError) + && (new InternalPythonError("x")).message === "x" + && InternalPythonError() instanceof InternalPythonError + && InternalPythonError() instanceof InternalError + && InternalPythonError("x").message === "x"; + """)) + class Object(unittest.TestCase): def setUp(self):