Repository: OpenAssetIO/OpenAssetIO (language: python)

Issue:
Support chaining exceptions in Python exceptions translator
## What

Support chaining exceptions (`raise from`) during C++->Python exception translation

## Why

If a C++ exception occurs in a pybind11-bound C++ function, then we attempt to translate the C++ exception to a Python exception. As part of this we execute Python statements, in-particular we import `openassetio._openassetio.errors`. Attempting to do this import whilst the Python error indicator is set causes a fatal error in Python.

This can happen if e.g. in the call stack of a pybind11-bound function, some C++ code uses the CPython API, which triggers a Python exception, but then the C++ code goes on to throw its own exception too.

pybind11 handles this case in the default exception translator by checking if `PyErr_Occurred()` then doing the CPython equivalent of `raise from`. The OpenAssetIO exception translator should do similar.

<!---
## Acceptance Criteria

An exhaustive list of falsifiable outcomes that define fully whether
this task is complete.

Remember to consider:

- Release note updates.
- Documentation updates.
- Additional tests or test updates.
- Impact on dependent repos.
--->



Discussion / hints:
Also see `std::throw_with_nested`, the C++ version of `raise from`, which pybind11 handles.

Fix patch:
diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md
index dff11bc83..61ebde983 100644
--- a/RELEASE_NOTES.md
+++ b/RELEASE_NOTES.md
@@ -15,6 +15,13 @@ managers._
   object workflows.
   [#1170](https://github.com/OpenAssetIO/OpenAssetIO/issues/1170)
 
+## Bug fixes
+
+- Added "raise from" behaviour in C++->Python exception translation, in
+  case a Python exception is already active when the C++ exception
+  occurs.
+  [#1419](https://github.com/OpenAssetIO/OpenAssetIO/issues/1419)
+
 v1.0.0-rc.1.0
 ---------------
 
diff --git a/src/openassetio-python/cmodule/src/errors/exceptionsBinding.cpp b/src/openassetio-python/cmodule/src/errors/exceptionsBinding.cpp
index 335e0ae15..94efb7ce1 100644
--- a/src/openassetio-python/cmodule/src/errors/exceptionsBinding.cpp
+++ b/src/openassetio-python/cmodule/src/errors/exceptionsBinding.cpp
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
-// Copyright 2013-2023 The Foundry Visionmongers Ltd
+// Copyright 2023-2024 The Foundry Visionmongers Ltd
+#include <cassert>
 #include <cstddef>
 #include <exception>
 #include <string_view>
@@ -34,15 +35,44 @@ using openassetio::errors::UnhandledException;
  *
  * @tparam Exception C++ exception type.
  * @param exception C++ exception instance.
- * @param pyModule Python module containing Python exception class.
  * @param pyClassName Python exception class name to look up in @p
  * pyModule.
  */
 template <class Exception>
-void setPyException(const Exception &exception, const py::module_ &pyModule,
+void setPyException(const Exception &exception,
                     // TODO(DF): False positive in clang-tidy 12 :(
                     // NOLINTNEXTLINE(readability-avoid-const-params-in-decls)
                     const std::string_view pyClassName) {
+  // Python "raise from" logic. See pybind11::raise_from. We need this
+  // to handle the case of a C++ exception being translated when the
+  // Python error indicator is already set for a different exception.
+  // This can't happen easily - most likely a C++ function triggered a
+  // CPython error state before throwing its own C++ exception.
+  auto fromExcVal = []() -> PyObject * {
+    if (!PyErr_Occurred()) {
+      return nullptr;
+    }
+    PyObject *excVal = nullptr;
+    PyObject *excType = nullptr;
+    PyObject *excTraceback = nullptr;
+    // Fetch (and clear) the original "from" exception. Note that if we
+    // don't clear it, the `import` below will cause a fatal Python
+    // error.
+    PyErr_Fetch(&excType, &excVal, &excTraceback);
+    PyErr_NormalizeException(&excType, &excVal, &excTraceback);
+    if (excTraceback != nullptr) {
+      PyException_SetTraceback(excVal, excTraceback);
+      Py_DECREF(excTraceback);
+    }
+    Py_DECREF(excType);
+    return excVal;
+  }();
+
+  assert(!PyErr_Occurred());
+
+  // Find the Python exception type corresponding to the given C++
+  // exception, and construct an instance.
+  const py::module_ pyModule = py::module_::import(kErrorsModuleName.data());
   const py::object pyClass = pyModule.attr(pyClassName.data());
   const py::object pyInstance = [&] {
     if constexpr (std::is_same_v<Exception, BatchElementException>) {
@@ -51,7 +81,23 @@ void setPyException(const Exception &exception, const py::module_ &pyModule,
       return pyClass(exception.what());
     }
   }();
+
+  // Set the error indicator to our converted C++->Python exception.
   PyErr_SetObject(pyClass.ptr(), pyInstance.ptr());
+
+  // Set the __cause__ of our new exception if raising from a previous
+  // exception.
+  if (fromExcVal != nullptr) {
+    PyObject *excVal = nullptr;
+    PyObject *excType = nullptr;
+    PyObject *excTraceback = nullptr;
+    PyErr_Fetch(&excType, &excVal, &excTraceback);
+    PyErr_NormalizeException(&excType, &excVal, &excTraceback);
+    Py_INCREF(fromExcVal);  // Bump to own two references, stolen by SetCause and SetContext.
+    PyException_SetCause(excVal, fromExcVal);
+    PyException_SetContext(excVal, fromExcVal);
+    PyErr_Restore(excType, excVal, excTraceback);
+  }
 }
 
 /**
@@ -73,21 +119,20 @@ void setPyException(const Exception &exception, const py::module_ &pyModule,
  *
  * @tparam I Index of exception type to catch in
  * CppExceptionsAndPyClassNames list.
- * @param pyModule Python module containing Python exception classes.
  * @param pexc C++ exception to rethrow.
  */
 template <std::size_t I>
-void tryCatch(const py::module_ &pyModule, std::exception_ptr pexc) {
+void tryCatch(std::exception_ptr pexc) {
   try {
     if constexpr (I == 0) {
       std::rethrow_exception(std::move(pexc));
     } else {
-      tryCatch<I - 1>(pyModule, std::move(pexc));
+      tryCatch<I - 1>(std::move(pexc));
     }
   } catch (const HybridException<CppExceptionsAndPyClassNames::Exceptions<I>> &cppExc) {
     throw cppExc.originalPyExc;
   } catch (const CppExceptionsAndPyClassNames::Exceptions<I> &cppExc) {
-    setPyException(cppExc, pyModule, CppExceptionsAndPyClassNames::kClassNames[I]);
+    setPyException(cppExc, CppExceptionsAndPyClassNames::kClassNames[I]);
   }
 }
 
@@ -226,7 +271,7 @@ void registerPyExceptionClasses(const py::module &mod,
  */
 void registerExceptions(const py::module &mod) {
   // Ensure module name matches what we expect, since it must be
-  // imported by name in `register_exception_translator`, below.
+  // imported by name in `setPyException`.
   assert(mod.attr("__name__").cast<std::string>() == kErrorsModuleName);
   // Register new Python exception types. Note that this is not
   // sufficient to cause C++ exceptions to be translated. See
@@ -238,17 +283,15 @@ void registerExceptions(const py::module &mod) {
   //
   // Note that capturing lambdas are not allowed here, so we must
   // `import` the module containing the exceptions in the body of the
-  // function.
+  // function (see setPyException).
   py::register_exception_translator([](std::exception_ptr pexc) {
     if (!pexc) {
       return;
     }
-    const py::module_ pyModule = py::module_::import(kErrorsModuleName.data());
-
     // Handle the different possible C++ exceptions, creating the
     // corresponding Python exception and setting it as the active
     // exception in this thread.
-    tryCatch<CppExceptionsAndPyClassNames::kSize - 1>(pyModule, std::move(pexc));
+    tryCatch<CppExceptionsAndPyClassNames::kSize - 1>(std::move(pexc));
   });
 }
 


Test patch:
diff --git a/src/openassetio-python/tests/cmodule/resources/_testutils/errorsTest.cpp b/src/openassetio-python/tests/cmodule/resources/_testutils/errorsTest.cpp
index 8c8534a38..f2ef32e3e 100644
--- a/src/openassetio-python/tests/cmodule/resources/_testutils/errorsTest.cpp
+++ b/src/openassetio-python/tests/cmodule/resources/_testutils/errorsTest.cpp
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: Apache-2.0
-// Copyright 2023 The Foundry Visionmongers Ltd
+// Copyright 2023-2024 The Foundry Visionmongers Ltd
 /**
  * Bindings used for testing errors behaviour.
  * Specifically, the conversion from cpp -> python.
@@ -8,6 +8,7 @@
  */
 
 #include <pybind11/pybind11.h>
+#include <pyerrors.h>
 
 #include <overrideMacros.hpp>
 
@@ -366,6 +367,10 @@ struct PyExceptionThrower : ExceptionThrower {
  * between hybrid pybind11 and OpenAssetIO exceptions, which have a
  * common STL base class so must be composed with care (i.e. avoid
  * multiple inheritance).
+ *
+ * `setErrorIndicatorAndThrowCppException` "raises" an exception through
+ * raw CPython, then throws a C++ exception, which should result in
+ * equivalent of "raise from".
  */
 void registerExceptionThrower(py::module_& mod) {
   mod.def("throwException", [](const std::string& exceptionName, const std::string& msgData) {
@@ -423,6 +428,11 @@ void registerExceptionThrower(py::module_& mod) {
       },
       py::arg("exceptionThrower"), py::call_guard<py::gil_scoped_release>{});
 
+  mod.def("setErrorIndicatorAndThrowCppException", [] {
+    PyErr_SetString(PyExc_TypeError, "CPython exception");
+    throw openassetio::errors::OpenAssetIOException{"Cpp exception"};
+  });
+
   py::class_<ExceptionThrower, PyExceptionThrower>{mod, "ExceptionThrower"}
       .def(py::init<>())
       .def("throwFromOverride", &ExceptionThrower::throwFromOverride)
diff --git a/src/openassetio-python/tests/cmodule/test_errors.py b/src/openassetio-python/tests/cmodule/test_errors.py
index a976db279..13d3a687e 100644
--- a/src/openassetio-python/tests/cmodule/test_errors.py
+++ b/src/openassetio-python/tests/cmodule/test_errors.py
@@ -1,5 +1,5 @@
 #
-#   Copyright 2023 The Foundry Visionmongers Ltd
+#   Copyright 2023-2024 The Foundry Visionmongers Ltd
 #
 #   Licensed under the Apache License, Version 2.0 (the "License");
 #   you may not use this file except in compliance with the License.
@@ -129,6 +129,16 @@ class OpenAssetIOException(Exception):
             )
 
 
+class Test_raise_from:
+    def test_when_CPython_error_indicator_and_cpp_exception_thrown_then_exceptions_combined(self):
+
+        with pytest.raises(errors.OpenAssetIOException, match="Cpp exception") as err:
+            _openassetio._testutils.setErrorIndicatorAndThrowCppException()
+
+        assert isinstance(err.value.__cause__, TypeError)
+        assert str(err.value.__cause__) == "CPython exception"
+
+
 def make_exception(exception_type):
     if exception_type == errors.BatchElementException:
         error = errors.BatchElementError(