Skip to content

Fix #63 #66, supersede #67: Wrap Python exceptions into custom InternalPythonError #79

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix #63 #66, supersede #67: Wrap Python exceptions into custom InternalPythonError #79

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 118 additions & 3 deletions module.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
//
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}
1 change: 1 addition & 0 deletions quickjs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def test():
Context = _quickjs.Context
Object = _quickjs.Object
JSException = _quickjs.JSException
JSPythonException = _quickjs.JSPythonException
StackOverflow = _quickjs.StackOverflow


Expand Down
67 changes: 66 additions & 1 deletion test_quickjs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand All @@ -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):
Expand Down