diff --git a/source/loaders/py_loader/source/py_loader_func.c b/source/loaders/py_loader/source/py_loader_func.c index 62bd1e782a..ec0b644a6c 100644 --- a/source/loaders/py_loader/source/py_loader_func.c +++ b/source/loaders/py_loader/source/py_loader_func.c @@ -114,6 +114,12 @@ static PyObject *py_loader_impl_func_call(PyObject *self, PyObject *args, PyObje PyObject *py_ret = py_loader_impl_value_to_capi(wrapped->impl, value_type_id(ret), ret); py_loader_thread_release(); value_type_destroy(ret); + + if (py_ret == NULL && !PyErr_Occurred()) + { + PyErr_SetString(PyExc_RuntimeError, "Failed to convert return value from foreign function call"); + } + return py_ret; } diff --git a/source/loaders/py_loader/source/py_loader_impl.c b/source/loaders/py_loader/source/py_loader_impl.c index 3c9800280d..b2a68440ba 100644 --- a/source/loaders/py_loader/source/py_loader_impl.c +++ b/source/loaders/py_loader/source/py_loader_impl.c @@ -400,7 +400,17 @@ value py_object_interface_method_invoke(object obj, object_impl impl, method m, for (size_t i = 0; i < argc; i++) { - PyTuple_SET_ITEM(args_tuple, i, py_loader_impl_value_to_capi(obj_impl->impl, value_type_id(args[i]), args[i])); + PyObject *arg = py_loader_impl_value_to_capi(obj_impl->impl, value_type_id(args[i]), args[i]); + + if (arg == NULL) + { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "Failed to convert argument %zu for method call", i); + Py_DecRef(args_tuple); + goto release; + } + + PyTuple_SET_ITEM(args_tuple, i, arg); } PyObject *python_object = PyObject_CallMethod(obj_impl->obj, method_name(m), "O", args_tuple, NULL); @@ -531,7 +541,19 @@ object py_class_interface_constructor(klass cls, class_impl impl, const char *na for (size_t i = 0; i < argc; i++) { - PyTuple_SET_ITEM(args_tuple, i, py_loader_impl_value_to_capi(py_cls->impl, value_type_id(args[i]), args[i])); + PyObject *arg = py_loader_impl_value_to_capi(py_cls->impl, value_type_id(args[i]), args[i]); + + if (arg == NULL) + { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "Failed to convert argument %zu for class construction", i); + Py_DecRef(args_tuple); + py_loader_thread_release(); + object_destroy(obj); + return NULL; + } + + PyTuple_SET_ITEM(args_tuple, i, arg); } /* Calling the class will create an instance (object) */ @@ -637,7 +659,18 @@ value py_class_interface_static_invoke(klass cls, class_impl impl, method m, cla for (size_t i = 0; i < argc; i++) { - PyTuple_SET_ITEM(args_tuple, i, py_loader_impl_value_to_capi(cls_impl->impl, value_type_id(args[i]), args[i])); + PyObject *arg = py_loader_impl_value_to_capi(cls_impl->impl, value_type_id(args[i]), args[i]); + + if (arg == NULL) + { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "Failed to convert argument %zu for static method call", i); + Py_DecRef(args_tuple); + Py_DecRef(method); + goto cleanup; + } + + PyTuple_SET_ITEM(args_tuple, i, arg); } PyObject *python_object = PyObject_Call(method, args_tuple, NULL); @@ -1302,9 +1335,21 @@ PyObject *py_loader_impl_value_to_capi(loader_impl impl, type_id id, value v) { PyObject *item = py_loader_impl_value_to_capi(impl, value_type_id((value)array_value[iterator]), (value)array_value[iterator]); + if (item == NULL) + { + /* Conversion failed (e.g. nested throwable) - abort to avoid storing NULL in the list */ + if (!PyErr_Occurred()) + { + PyErr_Format(PyExc_RuntimeError, "Failed to convert array element %zd", iterator); + } + Py_DecRef(list); + return NULL; + } + if (PyList_SetItem(list, iterator, item) != 0) { - /* TODO: Report error */ + Py_DecRef(list); + return NULL; } } @@ -1323,12 +1368,40 @@ PyObject *py_loader_impl_value_to_capi(loader_impl impl, type_id id, value v) value *pair_value = value_to_array(map_value[iterator]); PyObject *key = py_loader_impl_value_to_capi(impl, value_type_id((value)pair_value[0]), (value)pair_value[0]); + + if (key == NULL) + { + if (!PyErr_Occurred()) + { + PyErr_Format(PyExc_RuntimeError, "Failed to convert map key at index %zd", iterator); + } + Py_DecRef(dict); + return NULL; + } + PyObject *item = py_loader_impl_value_to_capi(impl, value_type_id((value)pair_value[1]), (value)pair_value[1]); + if (item == NULL) + { + if (!PyErr_Occurred()) + { + PyErr_Format(PyExc_RuntimeError, "Failed to convert map value at index %zd", iterator); + } + Py_DecRef(key); + Py_DecRef(dict); + return NULL; + } + if (PyDict_SetItem(dict, key, item) != 0) { - /* TODO: Report error */ + Py_DecRef(key); + Py_DecRef(item); + Py_DecRef(dict); + return NULL; } + + Py_DecRef(key); + Py_DecRef(item); } return dict; @@ -1393,6 +1466,34 @@ PyObject *py_loader_impl_value_to_capi(loader_impl impl, type_id id, value v) return obj_impl->obj; } + else if (id == TYPE_THROWABLE) + { + throwable th = value_to_throwable(v); + value inner = throwable_value(th); + + if (inner != NULL && value_type_id(inner) == TYPE_EXCEPTION) + { + exception ex = value_to_exception(inner); + const char *message = exception_message(ex); + const char *label = exception_label(ex); + const char *stacktrace = exception_stacktrace(ex); + + if (stacktrace != NULL && stacktrace[0] != '\0') + { + PyErr_Format(PyExc_RuntimeError, "%s: %s\n%s", label ? label : "Error", message ? message : "Unknown error", stacktrace); + } + else + { + PyErr_Format(PyExc_RuntimeError, "%s: %s", label ? label : "Error", message ? message : "Unknown error"); + } + } + else + { + PyErr_SetString(PyExc_RuntimeError, "An error was thrown from a foreign function call"); + } + + return NULL; + } else { log_write("metacall", LOG_LEVEL_ERROR, "Unrecognized value type: %d", id); @@ -1528,10 +1629,18 @@ function_return function_py_interface_invoke(function func, function_impl impl, type_id id = t == NULL ? value_type_id((value)args[args_count]) : type_index(t); values[args_count] = py_loader_impl_value_to_capi(py_func->impl, id, args[args_count]); - if (values[args_count] != NULL) + if (values[args_count] == NULL) { - PyTuple_SET_ITEM(tuple_args, args_count, values[args_count]); + if (!PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "Failed to convert argument %zu for Python function call", args_count); + Py_DecRef(tuple_args); + Py_LeaveRecursiveCall(); + if (is_var_args) + free(values); + goto finalize; } + + PyTuple_SET_ITEM(tuple_args, args_count, values[args_count]); } PyObject *result = PyObject_CallObject(py_func->func, tuple_args); @@ -1606,10 +1715,17 @@ function_return function_py_interface_await(function func, function_impl impl, f values[args_count] = py_loader_impl_value_to_capi(py_func->impl, id, args[args_count]); - if (values[args_count] != NULL) + if (values[args_count] == NULL) { - PyTuple_SetItem(tuple_args, args_count, values[args_count]); + if (!PyErr_Occurred()) + PyErr_Format(PyExc_TypeError, "Failed to convert argument %zu for Python async function call", args_count); + Py_DecRef(tuple_args); + if (args_size > signature_args_size) + free(values); + goto error; } + + PyTuple_SetItem(tuple_args, args_count, values[args_count]); } PyObject *coroutine = PyObject_CallObject(py_func->func, tuple_args); @@ -3725,7 +3841,7 @@ static int py_loader_impl_validate_object(loader_impl impl, PyObject *obj, objec log_write("metacall", LOG_LEVEL_DEBUG, "Discover object member %s, type %s", PyUnicode_AsUTF8(dict_key), type_id_name(py_loader_impl_capi_to_value_type(dict_val))); - + } } diff --git a/source/loaders/py_loader/source/py_loader_port.c b/source/loaders/py_loader/source/py_loader_port.c index 98fc9b014c..4fc9c9d085 100644 --- a/source/loaders/py_loader/source/py_loader_port.c +++ b/source/loaders/py_loader/source/py_loader_port.c @@ -539,7 +539,10 @@ static PyObject *py_loader_port_invoke(PyObject *self, PyObject *var_args) if (result == NULL) { - result = Py_ReturnNone(); + if (!PyErr_Occurred()) + { + PyErr_SetString(PyExc_RuntimeError, "A foreign function call returned an error"); + } goto clear; } } @@ -865,8 +868,11 @@ static PyObject *py_loader_port_value_dereference(PyObject *self, PyObject *args if (result == NULL) { - PyErr_SetString(PyExc_ValueErrorPtr(), "Failed to convert the MetaCall value to Python object."); - return Py_ReturnNone(); + if (!PyErr_Occurred()) + { + PyErr_SetString(PyExc_ValueErrorPtr(), "Failed to convert the MetaCall value to Python object."); + } + return NULL; } return result; diff --git a/source/tests/CMakeLists.txt b/source/tests/CMakeLists.txt index 0b346cb66d..bb1aed4738 100644 --- a/source/tests/CMakeLists.txt +++ b/source/tests/CMakeLists.txt @@ -141,6 +141,7 @@ add_subdirectory(metacall_node_python_async_after_destroy_test) add_subdirectory(metacall_node_python_await_test) # add_subdirectory(metacall_node_python_await_extended_test) # TODO: https://github.com/metacall/core/issues/519 add_subdirectory(metacall_node_python_exception_test) +add_subdirectory(metacall_node_python_throwable_test) add_subdirectory(metacall_node_clear_mem_test) add_subdirectory(metacall_node_async_resources_test) add_subdirectory(metacall_node_await_chain_test) diff --git a/source/tests/metacall_node_python_throwable_test/CMakeLists.txt b/source/tests/metacall_node_python_throwable_test/CMakeLists.txt new file mode 100644 index 0000000000..68345b3918 --- /dev/null +++ b/source/tests/metacall_node_python_throwable_test/CMakeLists.txt @@ -0,0 +1,160 @@ +# Check if this loader is enabled +if(NOT OPTION_BUILD_LOADERS OR NOT OPTION_BUILD_LOADERS_NODE OR NOT OPTION_BUILD_LOADERS_PY OR NOT OPTION_BUILD_PORTS OR NOT OPTION_BUILD_PORTS_PY) + return() +endif() + +# +# Executable name and options +# + +# Target name +set(target metacall-node-python-throwable-test) +message(STATUS "Test ${target}") + +# +# Compiler warnings +# + +include(Warnings) + +# +# Compiler security +# + +include(SecurityFlags) + +# +# Sources +# + +set(include_path "${CMAKE_CURRENT_SOURCE_DIR}/include/${target}") +set(source_path "${CMAKE_CURRENT_SOURCE_DIR}/source") + +set(sources + ${source_path}/main.cpp + ${source_path}/metacall_node_python_throwable_test.cpp +) + +# Group source files +set(header_group "Header Files (API)") +set(source_group "Source Files") +source_group_by_path(${include_path} "\\\\.h$|\\\\.hpp$" + ${header_group} ${headers}) +source_group_by_path(${source_path} "\\\\.cpp$|\\\\.c$|\\\\.h$|\\\\.hpp$" + ${source_group} ${sources}) + +# +# Create executable +# + +# Build executable +add_executable(${target} + ${sources} +) + +# Create namespaced alias +add_executable(${META_PROJECT_NAME}::${target} ALIAS ${target}) + +# +# Project options +# + +set_target_properties(${target} + PROPERTIES + ${DEFAULT_PROJECT_OPTIONS} + FOLDER "${IDE_FOLDER}" +) + +# +# Include directories +# + +target_include_directories(${target} + PRIVATE + ${DEFAULT_INCLUDE_DIRECTORIES} + ${PROJECT_BINARY_DIR}/source/include +) + +# +# Libraries +# + +target_link_libraries(${target} + PRIVATE + ${DEFAULT_LIBRARIES} + + GTest + + ${META_PROJECT_NAME}::metacall +) + +# +# Compile definitions +# + +target_compile_definitions(${target} + PRIVATE + ${DEFAULT_COMPILE_DEFINITIONS} + + # Python Port path (for sys.path.insert so Python can import metacall) + METACALL_PYTHON_PORT_PATH="${CMAKE_SOURCE_DIR}/source/ports/py_port" +) + +# +# Compile options +# + +target_compile_options(${target} + PRIVATE + ${DEFAULT_COMPILE_OPTIONS} +) + +# +# Compile features +# + +target_compile_features(${target} + PRIVATE + cxx_std_17 +) + +# +# Linker options +# + +target_link_options(${target} + PRIVATE + ${DEFAULT_LINKER_OPTIONS} +) + +# +# Define test +# + +add_test(NAME ${target} + COMMAND $ +) + +# +# Define dependencies +# + +add_dependencies(${target} + node_loader + py_loader +) + +# +# Define test properties +# + +set_property(TEST ${target} + PROPERTY LABELS ${target} +) + +include(TestEnvironmentVariables) + +test_environment_variables(${target} + "" + ${TESTS_ENVIRONMENT_VARIABLES} +) diff --git a/source/tests/metacall_node_python_throwable_test/source/main.cpp b/source/tests/metacall_node_python_throwable_test/source/main.cpp new file mode 100644 index 0000000000..fb41f44af8 --- /dev/null +++ b/source/tests/metacall_node_python_throwable_test/source/main.cpp @@ -0,0 +1,28 @@ +/* + * MetaCall Library by Parra Studios + * A library for providing a foreign function interface calls. + * + * Copyright (C) 2016 - 2026 Vicente Eduardo Ferrer Garcia + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include + +int main(int argc, char *argv[]) +{ + ::testing::InitGoogleTest(&argc, argv); + + return RUN_ALL_TESTS(); +} diff --git a/source/tests/metacall_node_python_throwable_test/source/metacall_node_python_throwable_test.cpp b/source/tests/metacall_node_python_throwable_test/source/metacall_node_python_throwable_test.cpp new file mode 100644 index 0000000000..f1bd54ff42 --- /dev/null +++ b/source/tests/metacall_node_python_throwable_test/source/metacall_node_python_throwable_test.cpp @@ -0,0 +1,201 @@ +/* + * MetaCall Library by Parra Studios + * A library for providing a foreign function interface calls. + * + * Copyright (C) 2016 - 2026 Vicente Eduardo Ferrer Garcia + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + */ + +#include + +#include + +#include +#include +#include + +class metacall_node_python_throwable_test : public testing::Test +{ +public: +}; + +TEST_F(metacall_node_python_throwable_test, DefaultConstructor) +{ + metacall_print_info(); + + ASSERT_EQ((int)0, (int)metacall_initialize()); + +/* NodeJS & Python */ +#if defined(OPTION_BUILD_LOADERS_NODE) && defined(OPTION_BUILD_LOADERS_PY) + { + /* + * Load a Node script that exports: + * - flip: higher-order function (swaps arg order) - replicates derpyramda.js flip + * used in the original failing Python test (test.py) + * - throwing_node_fn: function that directly throws a JS Error, used to test + * that py_loader_port_invoke (the metacall() Python port) correctly propagates + * the thrown TYPE_THROWABLE as a Python exception rather than returning Py_None + * with a pending exception (which causes a fatal error in Python 3.14+). + */ + static const char node_buffer[] = + "function flip(fn) {\n" + " return function(x, y) {\n" + " return fn(y, x);\n" + " };\n" + "}\n" + "function throwing_node_fn() {\n" + " throw new Error('test error from node');\n" + "}\n" + "module.exports = { flip, throwing_node_fn };\n"; + + ASSERT_EQ((int)0, (int)metacall_load_from_memory("node", node_buffer, sizeof(node_buffer), NULL)); + + /* + * Load a Python script that defines four test functions exercising the + * cross-language throwable propagation fixes: + * + * test_flip_returns_correct_value - the original crash scenario (test.py): + * flip(lambda)(5.0, 4.0) must return -1.0. Before the fix this SEGFAULT'd + * on Python 3.14+ because the return value arrived as TYPE_THROWABLE and + * py_loader_impl_value_to_capi returned NULL without setting a Python + * exception, which Python 3.14 treats as a fatal error. + * + * test_flip_with_throwing_callback - validates py_loader_func.c: + * When the Python callback passed to flip raises, the error must propagate + * back as a Python exception (not a SEGFAULT / silent None). + * + * test_direct_node_throw_raises_exception - validates py_loader_port_invoke: + * A Node function that directly throws must surface as a Python RuntimeError + * when called via metacall(). Before the fix py_loader_port_invoke returned + * Py_None with a pending exception - a Python 3.14 fatal contract violation. + * + * test_node_error_has_stacktrace - validates the stacktrace field is included + * in the RuntimeError message. node_loader populates exception_stacktrace + * with Error.stack; the new py_loader_impl_value_to_capi handler appends it. + */ + static const char py_buffer[] = + "import sys\n" + "sys.path.insert(0, '" METACALL_PYTHON_PORT_PATH "')\n" + "from metacall import metacall, metacall_load_from_memory\n" + "\n" + "def test_flip_returns_correct_value():\n" + " flip_fn = metacall('flip', lambda x, y: x - y)\n" + " result = flip_fn(5.0, 4.0)\n" + " return result\n" + "\n" + "def test_flip_with_throwing_callback():\n" + " def throwing_cb(x, y):\n" + " raise ValueError('intentional error from callback')\n" + " try:\n" + " flip_fn = metacall('flip', throwing_cb)\n" + " result = flip_fn(5.0, 4.0)\n" + " return False\n" + " except Exception:\n" + " return True\n" + "\n" + "def test_direct_node_throw_raises_exception():\n" + " try:\n" + " metacall('throwing_node_fn')\n" + " return False\n" + " except Exception:\n" + " return True\n" + "\n" + "def test_node_error_has_stacktrace():\n" + " try:\n" + " metacall('throwing_node_fn')\n" + " return ''\n" + " except Exception as e:\n" + " return str(e)\n"; + + ASSERT_EQ((int)0, (int)metacall_load_from_memory("py", py_buffer, sizeof(py_buffer), NULL)); + + /* Test 1: flip(lambda x, y: x - y)(5.0, 4.0) should return -1.0 + * This is the exact scenario from the original failing test.py. + * Before the fix, this would SEGFAULT on Python 3.14+ because the + * return value arrived as TYPE_THROWABLE and py_loader_impl_value_to_capi + * returned NULL without setting a Python exception. */ + { + void *ret = metacall("test_flip_returns_correct_value"); + + ASSERT_NE((void *)NULL, (void *)ret); + + EXPECT_EQ((enum metacall_value_id)METACALL_DOUBLE, (enum metacall_value_id)metacall_value_id(ret)); + + EXPECT_EQ((double)-1.0, (double)metacall_value_to_double(ret)); + + metacall_value_destroy(ret); + } + + /* Test 2: flip with a throwing callback should not SEGFAULT and should + * propagate the exception back to the caller cleanly. + * The Python function returns True if an exception was raised and caught, + * False if the call unexpectedly succeeded. */ + { + void *ret = metacall("test_flip_with_throwing_callback"); + + ASSERT_NE((void *)NULL, (void *)ret); + + EXPECT_EQ((enum metacall_value_id)METACALL_BOOL, (enum metacall_value_id)metacall_value_id(ret)); + + /* The Python function returns True when the exception was caught */ + EXPECT_NE((int)0, (int)metacall_value_to_bool(ret)); + + metacall_value_destroy(ret); + } + + /* Test 3: a Node function that directly throws must surface as a Python + * exception when called via metacall() from the Python port. + * Validates the py_loader_port_invoke fix: before the fix, py_loader_port_invoke + * returned Py_None with a pending PyErr - a fatal contract violation in Python + * 3.14+ ("a function returned a result with an exception set"). + * After the fix, NULL is returned and Python sees a clean RuntimeError. */ + { + void *ret = metacall("test_direct_node_throw_raises_exception"); + + ASSERT_NE((void *)NULL, (void *)ret); + + EXPECT_EQ((enum metacall_value_id)METACALL_BOOL, (enum metacall_value_id)metacall_value_id(ret)); + + EXPECT_NE((int)0, (int)metacall_value_to_bool(ret)); + + metacall_value_destroy(ret); + } + + /* Test 4: the JS Error.stack (stacktrace) must appear in the RuntimeError + * message raised in Python. node_loader populates exception_stacktrace with + * Error.stack; the updated TYPE_THROWABLE handler appends it after the message. + * The Python function returns the exception string; we verify it contains the + * expected error message text. */ + { + void *ret = metacall("test_node_error_has_stacktrace"); + + ASSERT_NE((void *)NULL, (void *)ret); + + EXPECT_EQ((enum metacall_value_id)METACALL_STRING, (enum metacall_value_id)metacall_value_id(ret)); + + const char *msg = metacall_value_to_string(ret); + + EXPECT_NE((void *)NULL, (void *)msg); + + /* The error message must contain the text thrown from JS */ + EXPECT_NE((void *)NULL, (void *)strstr(msg, "test error from node")); + + metacall_value_destroy(ret); + } + } +#endif /* OPTION_BUILD_LOADERS_NODE && OPTION_BUILD_LOADERS_PY */ + + metacall_destroy(); +}