python: Fix and improve PythonException

For this, PythonException::occurred() is introduced to check if the error indicator is set in Python. PythonObject::to<std::string>() is added to allow formatting of the exception. PythonInterpreter::call() was modified to handle exceptions raised during execution of the called function.
taminob · Mar 27, 2024 · 3de88f8 · 3de88f8
1 parent 2e7b0d2
commit 3de88f8
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 21 deletions.
diff --git a/include/python/python_exception.h b/include/python/python_exception.h
@@ -9,6 +9,7 @@ class PythonException {
 public:
     PythonException() = default;
     [[nodiscard]] static std::optional<PythonException> latest();
+    [[nodiscard]] static bool occurred();
 
     [[nodiscard]] std::string toString() const;
 

diff --git a/include/python/python_interpreter.h b/include/python/python_interpreter.h
@@ -24,7 +24,7 @@ class PythonInterpreter {
     PyThreadState* state() { return state_.get(); }
     PyObject* mainModule() { return main_module_.get(); }
 
-    CallResult<PyObject*> internalCall(const std::string& function_name, PyObject* args);
+    CallResult<PythonObject> internalCall(const std::string& function_name, PyObject* args);
 
 private:
     std::unique_ptr<PyObject, std::function<void(PyObject*)>> main_module_;
@@ -37,11 +37,11 @@ CallResult<ReturnValue> PythonInterpreter::call(const std::string& function_name
     PythonGuard python_guard { state() };
     PythonTuple args_tuple { std::forward<Args>(args)... };
 
-    return internalCall(function_name, args_tuple.pyObject()).andThen([](PyObject* result) {
+    return internalCall(function_name, args_tuple.pyObject()).andThen([](const PythonObject& result) {
         if constexpr (std::is_void_v<ReturnValue>) {
             return;
         } else {
-            return PythonObject { result }.as<ReturnValue>();
+            return result.as<ReturnValue>();
         }
     });
 }

diff --git a/include/python/python_object.h b/include/python/python_object.h
@@ -30,6 +30,9 @@ class PythonObject {
     template <typename T>
     std::optional<T> as();
 
+    template <typename T>
+    std::optional<T> to();
+
 private:
     PyObject* object() { return object_.get(); }
 
@@ -40,6 +43,8 @@ class PythonObject {
     std::optional<bool> asBool();
     std::optional<std::string> asString();
 
+    std::optional<std::string> toString();
+
 private:
     std::unique_ptr<PyObject, void (*)(PyObject*)> object_;
 };
@@ -62,6 +67,17 @@ std::optional<T> PythonObject::as()
     }
     return std::nullopt;
 }
+
+template <typename T>
+std::optional<T> PythonObject::to()
+{
+    if constexpr (std::is_same_v<T, std::string>) {
+        return toString();
+    } else {
+        static_assert(sizeof(T), "Cannot interpret PythonObject as given type!");
+    }
+    return std::nullopt;
+}
 } // namespace ppplugin
 
 #endif // PPPLUGIN_PYTHON_OBJECT_H
diff --git a/src/python_exception.cpp b/src/python_exception.cpp
@@ -11,19 +11,20 @@
 namespace ppplugin {
 std::optional<PythonException> PythonException::latest()
 {
-    // TODO: acquire GIL
+    // TODO: make sure GIL is acquired
 #if PY_VERSION_HEX >= 0x030c0000 // Python 3.12 or newer
     PythonObject exception { PyErr_GetRaisedException() };
-    PythonObject type { PyException_GetTraceback(exception.pyObject()) };
+    PythonObject type { PyObject_Type(exception.pyObject()) };
     PythonObject args_tuple { PyException_GetArgs(exception.pyObject()) };
-    PythonObject value { (PyTuple_Size(args_tuple.pyObject()) > 0) ? PyTuple_GetItem(args_tuple.pyObject(), 0)
+    PythonObject value { (PyTuple_Size(args_tuple.pyObject()) > 0) ? args_tuple
                                                                    : nullptr };
     PythonObject traceback { PyException_GetTraceback(exception.pyObject()) };
 #else
-    PyObject* py_type = nullptr;
-    PyObject* py_value = nullptr;
-    PyObject* py_traceback = nullptr;
+    PyObject* py_type {};
+    PyObject* py_value = {};
+    PyObject* py_traceback {};
     PyErr_Fetch(&py_type, &py_value, &py_traceback);
+    PyErr_NormalizeException(&py_type, &py_value, &py_traceback);
     PythonObject type { py_type };
     PythonObject value { py_value };
     PythonObject traceback { py_traceback };
@@ -35,22 +36,40 @@ std::optional<PythonException> PythonException::latest()
 
     PythonException result;
     if (type) {
-        result.type_ = type.as<std::string>();
+        result.type_ = type.to<std::string>();
     }
     if (value) {
-        result.value_ = value.as<std::string>();
+        result.value_ = value.to<std::string>();
     }
     if (traceback) {
-        PythonObject traceback_module { PyImport_AddModule("traceback") };
+        PythonObject traceback_module { PyImport_ImportModule("traceback") };
+        assert(traceback_module);
         PythonObject format_traceback { PyObject_GetAttrString(traceback_module.pyObject(), "format_tb") };
+        assert(format_traceback);
 
         auto output = PythonObject { PyObject_CallOneArg(format_traceback.pyObject(), traceback.pyObject()) };
-        result.traceback_ = output.as<std::string>();
+        if (PyList_Check(output.pyObject())) {
+            auto output_size = PyList_Size(output.pyObject());
+            std::string formatted_traceback;
+            for (int i = 0; i < output_size; ++i) {
+                formatted_traceback += PythonObject::wrap(PyList_GetItem(output.pyObject(), i)).to<std::string>().value_or("");
+            }
+            if (!formatted_traceback.empty()) {
+                result.traceback_ = formatted_traceback;
+            }
+        } else {
+            result.traceback_ = output.to<std::string>();
+        }
     }
     return result;
 }
 
-[[nodiscard]] std::string PythonException::toString() const
+bool PythonException::occurred()
+{
+    return PyErr_Occurred() != nullptr;
+}
+
+std::string PythonException::toString() const
 {
     auto result = format("'{}': '{}'", type_.value_or("<unknown>"), value_.value_or("<?>"));
     if (traceback_) {

diff --git a/src/python_interpreter.cpp b/src/python_interpreter.cpp
@@ -112,14 +112,14 @@ std::optional<LoadError> PythonInterpreter::load(const std::string& file_name)
     return LoadError::unknown;
 }
 
-CallResult<PyObject*> PythonInterpreter::internalCall(const std::string& function_name, PyObject* args)
+CallResult<PythonObject> PythonInterpreter::internalCall(const std::string& function_name, PyObject* args)
 {
     // TODO: find uniform way to manage guards (here: already locked in PythonPlugin::call)
     // PythonGuard python_guard { state() };
     auto* function = PyObject_GetAttrString(mainModule(), function_name.c_str());
     if ((function == nullptr) || (PyCallable_Check(function) == 0)) {
         Py_XDECREF(function);
-        if (PyErr_Occurred() != nullptr) {
+        if (PythonException::occurred()) {
             if (auto exception = PythonException::latest()) {
                 return CallError {
                     CallError::Code::symbolNotFound,
@@ -132,12 +132,19 @@ CallResult<PyObject*> PythonInterpreter::internalCall(const std::string& functio
     }
     PyObject* kwargs = nullptr;
     // TODO: checkout PyEval_SetTrace (?) or PyThreadState_SetAsyncExc to interrupt thread
-    auto* result = PyObject_Call(function, args, kwargs);
+    PythonObject result { PyObject_Call(function, args, kwargs) };
     // TODO: use PyObject_CallOneArg, PyObject_CallNoArgs, PyObject_CallObject?;
     //       see: https://docs.python.org/3/c-api/call.html
     //       or consider vectorcall: https://peps.python.org/pep-0590/
     Py_XDECREF(kwargs);
     Py_DECREF(function);
+    if (PythonException::occurred()) {
+        if (auto exception = PythonException::latest()) {
+            return CallError { CallError::Code::unknown,
+                exception->toString() };
+        }
+        return CallError { CallError::Code::unknown };
+    }
     return result;
 }
 

diff --git a/src/python_object.cpp b/src/python_object.cpp
@@ -65,14 +65,25 @@ std::optional<bool> PythonObject::asBool()
 std::optional<std::string> PythonObject::asString()
 {
     if (PyUnicode_Check(object()) != 0) {
-        auto* utf8_object = PyUnicode_AsUTF8String(object());
-        if (utf8_object == nullptr) {
+        PythonObject utf8_object { PyUnicode_AsUTF8String(object()) };
+        if (!utf8_object) {
             return std::nullopt;
         }
-        std::string result { PyBytes_AsString(utf8_object) };
-        Py_DECREF(utf8_object);
+        std::string result { PyBytes_AsString(utf8_object.pyObject()) };
         return result;
     }
+    if (PyBytes_Check(object()) != 0) {
+        auto* result = PyBytes_AsString(object());
+        if (result == nullptr) {
+            return std::nullopt;
+        }
+        return std::string { result };
+    }
     return std::nullopt;
 }
+
+std::optional<std::string> PythonObject::toString()
+{
+    return PythonObject { PyObject_Str(object()) }.asString();
+}
 } // namespace ppplugin