Ensure that all translators are considered for nested exceptions

Author: cbalioglu

include/pybind11/detail/internals.h

@@ -211,6 +211,54 @@ struct internals {
 #endif
 };
 
+internals &get_internals();
+
+// the internals struct (above) is shared between all the modules. local_internals are only
+// for a single module. Any changes made to internals may require an update to
+// PYBIND11_INTERNALS_VERSION, breaking backwards compatibility. local_internals is, by design,
+// restricted to a single module. Whether a module has local internals or not should not
+// impact any other modules, because the only things accessing the local internals is the
+// module that contains them.
+struct local_internals {
+    type_map<type_info *> registered_types_cpp;
+    std::forward_list<ExceptionTranslator> registered_exception_translators;
+#if defined(WITH_THREAD) && PYBIND11_INTERNALS_VERSION == 4
+
+    // For ABI compatibility, we can't store the loader_life_support TLS key in
+    // the `internals` struct directly.  Instead, we store it in `shared_data` and
+    // cache a copy in `local_internals`.  If we allocated a separate TLS key for
+    // each instance of `local_internals`, we could end up allocating hundreds of
+    // TLS keys if hundreds of different pybind11 modules are loaded (which is a
+    // plausible number).
+    PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
+
+    // Holds the shared TLS key for the loader_life_support stack.
+    struct shared_loader_life_support_data {
+        PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
+        shared_loader_life_support_data() {
+            if (!PYBIND11_TLS_KEY_CREATE(loader_life_support_tls_key)) {
+                pybind11_fail("local_internals: could not successfully initialize the "
+                              "loader_life_support TLS key!");
+            }
+        }
+        // We can't help but leak the TLS key, because Python never unloads extension modules.
+    };
+
+    local_internals() {
+        auto &internals = get_internals();
+        // Get or create the `loader_life_support_stack_key`.
+        auto &ptr = internals.shared_data["_life_support"];
+        if (!ptr) {
+            ptr = new shared_loader_life_support_data;
+        }
+        loader_life_support_tls_key
+            = static_cast<shared_loader_life_support_data *>(ptr)->loader_life_support_tls_key;
+    }
+#endif //  defined(WITH_THREAD) && PYBIND11_INTERNALS_VERSION == 4
+};
+
+local_internals &get_local_internals();
+
 /// Additional type information which does not fit into the PyTypeObject.
 /// Changes to this struct also require bumping `PYBIND11_INTERNALS_VERSION`.
 struct type_info {
@@ -314,16 +362,41 @@ inline internals **&get_internals_pp() {
     return internals_pp;
 }
 
-// forward decl
-inline void translate_exception(std::exception_ptr);
+// Apply all the extensions translators from a list
+// Return true if one of the translators completed without raising an exception
+// itself. Return of false indicates that if there are other translators
+// available, they should be tried.
+inline bool apply_exception_translators(std::forward_list<ExceptionTranslator> &translators,
+                                        std::exception_ptr last_exception) {
+    for (auto &translator : translators) {
+        try {
+            translator(last_exception);
+            return true;
+        } catch (...) {
+            last_exception = std::current_exception();
+        }
+    }
+    return false;
+}
+
+inline bool apply_exception_translators(std::forward_list<ExceptionTranslator> &translators) {
+    return apply_exception_translators(translators, std::current_exception());
+}
 
 template <class T,
           enable_if_t<std::is_same<std::nested_exception, remove_cvref_t<T>>::value, int> = 0>
 bool handle_nested_exception(const T &exc, const std::exception_ptr &p) {
     std::exception_ptr nested = exc.nested_ptr();
     if (nested != nullptr && nested != p) {
-        translate_exception(nested);
-        return true;
+        auto &local_translators = get_local_internals().registered_exception_translators;
+        if (apply_exception_translators(local_translators, nested)) {
+            return true;
+        }
+
+        auto &translators = get_internals().registered_exception_translators;
+        if (apply_exception_translators(translators, nested)) {
+            return true;
+        }
     }
     return false;
 }
@@ -491,50 +564,6 @@ PYBIND11_NOINLINE internals &get_internals() {
     return **internals_pp;
 }
 
-// the internals struct (above) is shared between all the modules. local_internals are only
-// for a single module. Any changes made to internals may require an update to
-// PYBIND11_INTERNALS_VERSION, breaking backwards compatibility. local_internals is, by design,
-// restricted to a single module. Whether a module has local internals or not should not
-// impact any other modules, because the only things accessing the local internals is the
-// module that contains them.
-struct local_internals {
-    type_map<type_info *> registered_types_cpp;
-    std::forward_list<ExceptionTranslator> registered_exception_translators;
-#if defined(WITH_THREAD) && PYBIND11_INTERNALS_VERSION == 4
-
-    // For ABI compatibility, we can't store the loader_life_support TLS key in
-    // the `internals` struct directly.  Instead, we store it in `shared_data` and
-    // cache a copy in `local_internals`.  If we allocated a separate TLS key for
-    // each instance of `local_internals`, we could end up allocating hundreds of
-    // TLS keys if hundreds of different pybind11 modules are loaded (which is a
-    // plausible number).
-    PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
-
-    // Holds the shared TLS key for the loader_life_support stack.
-    struct shared_loader_life_support_data {
-        PYBIND11_TLS_KEY_INIT(loader_life_support_tls_key)
-        shared_loader_life_support_data() {
-            if (!PYBIND11_TLS_KEY_CREATE(loader_life_support_tls_key)) {
-                pybind11_fail("local_internals: could not successfully initialize the "
-                              "loader_life_support TLS key!");
-            }
-        }
-        // We can't help but leak the TLS key, because Python never unloads extension modules.
-    };
-
-    local_internals() {
-        auto &internals = get_internals();
-        // Get or create the `loader_life_support_stack_key`.
-        auto &ptr = internals.shared_data["_life_support"];
-        if (!ptr) {
-            ptr = new shared_loader_life_support_data;
-        }
-        loader_life_support_tls_key
-            = static_cast<shared_loader_life_support_data *>(ptr)->loader_life_support_tls_key;
-    }
-#endif //  defined(WITH_THREAD) && PYBIND11_INTERNALS_VERSION == 4
-};
-
 /// Works like `get_internals`, but for things which are locally registered.
 inline local_internals &get_local_internals() {
     // Current static can be created in the interpreter finalization routine. If the later will be

include/pybind11/pybind11.h

@@ -52,24 +52,6 @@ PYBIND11_WARNING_DISABLE_MSVC(4127)
 
 PYBIND11_NAMESPACE_BEGIN(detail)
 
-// Apply all the extensions translators from a list
-// Return true if one of the translators completed without raising an exception
-// itself. Return of false indicates that if there are other translators
-// available, they should be tried.
-inline bool apply_exception_translators(std::forward_list<ExceptionTranslator> &translators) {
-    auto last_exception = std::current_exception();
-
-    for (auto &translator : translators) {
-        try {
-            translator(last_exception);
-            return true;
-        } catch (...) {
-            last_exception = std::current_exception();
-        }
-    }
-    return false;
-}
-
 #if defined(_MSC_VER)
 #    define PYBIND11_COMPAT_STRDUP _strdup
 #else

tests/test_exceptions.cpp

@@ -297,19 +297,35 @@ TEST_SUBMODULE(exceptions, m) {
         }
     });
 
-    m.def("throw_nested_exception", []() {
+    m.def("throw_stl_nested_exception", []() {
         try {
-            throw std::runtime_error("Inner Exception");
+            throw std::runtime_error("Inner STL Exception");
         } catch (const std::runtime_error &) {
-            std::throw_with_nested(std::runtime_error("Outer Exception"));
+            std::throw_with_nested(std::runtime_error("Outer STL Exception"));
         }
     });
 
-    m.def("throw_custom_nested_exception", []() {
+    m.def("throw_stl_nested_exception_with_custom_exception", []() {
         try {
-            throw std::runtime_error("Inner Exception");
+            throw std::runtime_error("Inner STL Exception");
         } catch (const std::runtime_error &) {
-            std::throw_with_nested(MyException5("Outer Exception"));
+            std::throw_with_nested(MyException5("Outer Custom Exception"));
+        }
+    });
+
+    m.def("throw_custom_nested_exception_with_stl_exception", []() {
+        try {
+            throw MyException5("Inner Custom Exception");
+        } catch (const MyException5 &) {
+            std::throw_with_nested(std::runtime_error("Outer STL Exception"));
+        }
+    });
+
+    m.def("throw_custom_nested_exception_with_custom_exception", []() {
+        try {
+            throw MyException5("Inner Custom Exception");
+        } catch (const MyException5 &) {
+            std::throw_with_nested(MyException5("Outer Custom Exception"));
         }
     });
 

tests/test_exceptions.py

@@ -241,18 +241,32 @@ def pycatch(exctype, f, *args):
     assert str(excinfo.value) == "this is a helper-defined translated exception"
 
 
-def test_throw_nested_exception():
+def test_throw_stl_nested_exception():
     with pytest.raises(RuntimeError) as excinfo:
-        m.throw_nested_exception()
-    assert str(excinfo.value) == "Outer Exception"
-    assert str(excinfo.value.__cause__) == "Inner Exception"
+        m.throw_stl_nested_exception()
+    assert str(excinfo.value) == "Outer STL Exception"
+    assert str(excinfo.value.__cause__) == "Inner STL Exception"
 
 
-def test_throw_custom_nested_exception():
+def test_throw_stl_nested_exception_with_custom_exception():
     with pytest.raises(m.MyException5) as excinfo:
-        m.throw_custom_nested_exception()
-    assert str(excinfo.value) == "Outer Exception"
-    assert str(excinfo.value.__cause__) == "Inner Exception"
+        m.throw_stl_nested_exception_with_custom_exception()
+    assert str(excinfo.value) == "Outer Custom Exception"
+    assert str(excinfo.value.__cause__) == "Inner STL Exception"
+
+
+def test_throw_custom_nested_exception_with_stl_exception():
+    with pytest.raises(RuntimeError) as excinfo:
+        m.throw_custom_nested_exception_with_stl_exception()
+    assert str(excinfo.value) == "Outer STL Exception"
+    assert str(excinfo.value.__cause__) == "Inner Custom Exception"
+
+
+def test_throw_custom_nested_exception_with_custom_exception():
+    with pytest.raises(m.MyException5) as excinfo:
+        m.throw_custom_nested_exception_with_custom_exception()
+    assert str(excinfo.value) == "Outer Custom Exception"
+    assert str(excinfo.value.__cause__) == "Inner Custom Exception"
 
 
 # This can often happen if you wrap a pybind11 class in a Python wrapper