Feature/local exception translator (#2650)

* Create a module_internals struct

Since we now have two things that are going to be module local, it felt
correct to add a struct to manage them.

* Add local exception translators

These are added via the  register_local_exception_translator function
and are then applied before the global translators

* Add unit tests to show the local exception translator works

* Fix a bug in the unit test with the string value of KeyError

* Fix a formatting issue

* Rename registered_local_types_cpp()

Rename it to get_registered_local_types_cpp() to disambiguate from the
new member of module_internals

* Add additional comments to new local exception code path

* Add a register_local_exception function

* Add additional unit tests for register_local_exception

* Use get_local_internals like get_internals

* Update documentation for new local exception feature

* Add back a missing space

* Clean-up some issues in the docs

* Remove the code duplication when translating exceptions

Separated out the exception processing into a standalone function in the
details namespace.

Clean-up some comments as per PR notes as well

* Remove the code duplication in register_exception

* Cleanup some formatting things caught by clang-format

* Remove the templates from exception translators

But I added a using declaration to alias the type.

* Remove the extra local from local_internals variable names

* Add an extra explanatory comment to local_internals

* Fix a typo in the code

docs/advanced/exceptions.rst

@@ -75,9 +75,10 @@ Registering custom translators
 
 If the default exception conversion policy described above is insufficient,
 pybind11 also provides support for registering custom exception translators.
-To register a simple exception conversion that translates a C++ exception into
+Similar to pybind11 classes, exception translators can be local to the module
-a new Python exception using the C++ exception's ``what()`` method, a helper
+they are defined in or global to the entire python session.  To register a simple
-function is available:
+exception conversion that translates a C++ exception into a new Python exception
+using the C++ exception's ``what()`` method, a helper function is available:
 
 .. code-block:: cpp
 
@@ -87,12 +88,20 @@ This call creates a Python exception class with the name ``PyExp`` in the given
 module and automatically converts any encountered exceptions of type ``CppExp``
 into Python exceptions of type ``PyExp``.
 
+A matching function is available for registering a local exception translator:
+
+.. code-block:: cpp
+
+    py::register_local_exception<CppExp>(module, "PyExp");
+
+
 It is possible to specify base class for the exception using the third
 parameter, a `handle`:
 
 .. code-block:: cpp
 
     py::register_exception<CppExp>(module, "PyExp", PyExc_RuntimeError);
+    py::register_local_exception<CppExp>(module, "PyExp", PyExc_RuntimeError);
 
 Then `PyExp` can be caught both as `PyExp` and `RuntimeError`.
 
@@ -100,16 +109,18 @@ The class objects of the built-in Python exceptions are listed in the Python
 documentation on `Standard Exceptions <https://docs.python.org/3/c-api/exceptions.html#standard-exceptions>`_.
 The default base class is `PyExc_Exception`.
 
-When more advanced exception translation is needed, the function
+When more advanced exception translation is needed, the functions
-``py::register_exception_translator(translator)`` can be used to register
+``py::register_exception_translator(translator)`` and
+``py::register_local_exception_translator(translator)`` can be used to register
 functions that can translate arbitrary exception types (and which may include
-additional logic to do so).  The function takes a stateless callable (e.g.  a
+additional logic to do so).  The functions takes a stateless callable (e.g. a
 function pointer or a lambda function without captured variables) with the call
 signature ``void(std::exception_ptr)``.
 
 When a C++ exception is thrown, the registered exception translators are tried
 in reverse order of registration (i.e. the last registered translator gets the
-first shot at handling the exception).
+first shot at handling the exception). All local translators will be tried
+before a global translator is tried.
 
 Inside the translator, ``std::rethrow_exception`` should be used within
 a try block to re-throw the exception.  One or more catch clauses to catch
@@ -168,6 +179,53 @@ section.
     with ``-fvisibility=hidden``. Therefore exceptions that are used across ABI boundaries need to be explicitly exported, as exercised in ``tests/test_exceptions.h``.
     See also: "Problems with C++ exceptions" under `GCC Wiki <https://gcc.gnu.org/wiki/Visibility>`_.
 
+
+Local vs Global Exception Translators
+=====================================
+
+When a global exception translator is registered, it will be applied across all
+modules in the reverse order of registration. This can create behavior where the
+order of module import influences how exceptions are translated.
+
+If module1 has the following translator:
+
+.. code-block:: cpp
+
+      py::register_exception_translator([](std::exception_ptr p) {
+        try {
+            if (p) std::rethrow_exception(p);
+        } catch (const std::invalid_argument &e) {
+            PyErr_SetString("module1 handled this")
+        }
+      }
+
+and module2 has the following similar translator:
+
+.. code-block:: cpp
+
+      py::register_exception_translator([](std::exception_ptr p) {
+        try {
+            if (p) std::rethrow_exception(p);
+        } catch (const std::invalid_argument &e) {
+            PyErr_SetString("module2 handled this")
+        }
+      }
+
+then which translator handles the invalid_argument will be determined by the
+order that module1 and module2 are imported. Since exception translators are
+applied in the reverse order of registration, which ever module was imported
+last will "win" and that translator will be applied.
+
+If there are multiple pybind11 modules that share exception types (either
+standard built-in or custom) loaded into a single python instance and
+consistent error handling behavior is needed, then local translators should be
+used.
+
+Changing the previous example to use ``register_local_exception_translator``
+would mean that when invalid_argument is thrown in the module2 code, the
+module2 translator will always handle it, while in module1, the module1
+translator will do the same.
+
 .. _handling_python_exceptions_cpp:
 
 Handling exceptions from Python in C++

include/pybind11/detail/class.h

@@ -209,7 +209,7 @@ extern "C" inline void pybind11_meta_dealloc(PyObject *obj) {
         internals.direct_conversions.erase(tindex);
 
         if (tinfo->module_local)
-            registered_local_types_cpp().erase(tindex);
+            get_local_internals().registered_types_cpp.erase(tindex);
         else
             internals.registered_types_cpp.erase(tindex);
         internals.registered_types_py.erase(tinfo->type);

include/pybind11/detail/internals.h

@@ -12,7 +12,11 @@
 #include "../pytypes.h"
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
+
+using ExceptionTranslator = void (*)(std::exception_ptr);
+
 PYBIND11_NAMESPACE_BEGIN(detail)
+
 // Forward declarations
 inline PyTypeObject *make_static_property_type();
 inline PyTypeObject *make_default_metaclass();
@@ -100,7 +104,7 @@ struct internals {
     std::unordered_set<std::pair<const PyObject *, const char *>, override_hash> inactive_override_cache;
     type_map<std::vector<bool (*)(PyObject *, void *&)>> direct_conversions;
     std::unordered_map<const PyObject *, std::vector<PyObject *>> patients;
-    std::forward_list<void (*) (std::exception_ptr)> registered_exception_translators;
+    std::forward_list<ExceptionTranslator> registered_exception_translators;
     std::unordered_map<std::string, void *> shared_data; // Custom data to be shared across extensions
     std::vector<PyObject *> loader_patient_stack; // Used by `loader_life_support`
     std::forward_list<std::string> static_strings; // Stores the std::strings backing detail::c_str()
@@ -313,12 +317,25 @@ PYBIND11_NOINLINE inline internals &get_internals() {
     return **internals_pp;
 }
 
-/// Works like `internals.registered_types_cpp`, but for module-local registered types:
+
-inline type_map<type_info *> &registered_local_types_cpp() {
+// the internals struct (above) is shared between all the modules. local_internals are only
-    static type_map<type_info *> locals{};
+// 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;
+};
+
+/// Works like `get_internals`, but for things which are locally registered.
+inline local_internals &get_local_internals() {
+  static local_internals locals;
   return locals;
 }
 
+
 /// Constructs a std::string with the given arguments, stores it in `internals`, and returns its
 /// `c_str()`.  Such strings objects have a long storage duration -- the internal strings are only
 /// cleared when the program exits or after interpreter shutdown (when embedding), and so are

include/pybind11/detail/type_caster_base.h

@@ -160,7 +160,7 @@ PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) {
 }
 
 inline detail::type_info *get_local_type_info(const std::type_index &tp) {
-    auto &locals = registered_local_types_cpp();
+    auto &locals = get_local_internals().registered_types_cpp;
     auto it = locals.find(tp);
     if (it != locals.end())
         return it->second;

include/pybind11/pybind11.h

@@ -56,6 +56,29 @@
 
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 
+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;
+}
+
+PYBIND11_NAMESPACE_END(detail)
+
+
 /// Wraps an arbitrary C++ function/method/lambda function/.. into a callable Python object
 class cpp_function : public function {
 public:
@@ -550,6 +573,7 @@ protected:
         }
     }
 
+
     /// Main dispatch logic for calls to functions bound using pybind11
     static PyObject *dispatcher(PyObject *self, PyObject *args_in, PyObject *kwargs_in) {
         using namespace detail;
@@ -830,8 +854,12 @@ protected:
 #endif
         } catch (...) {
             /* When an exception is caught, give each registered exception
-               translator a chance to translate it to a Python exception
+               translator a chance to translate it to a Python exception. First
-               in reverse order of registration.
+               all module-local translators will be tried in reverse order of
+               registration. If none of the module-locale translators handle
+               the exception (or there are no module-locale translators) then
+               the global translators will be tried, also in reverse order of
+               registration.
 
                A translator may choose to do one of the following:
 
@@ -840,17 +868,15 @@ protected:
                 - do nothing and let the exception fall through to the next translator, or
                 - delegate translation to the next translator by throwing a new type of exception. */
 
-            auto last_exception = std::current_exception();
+            auto &local_exception_translators = get_local_internals().registered_exception_translators;
-            auto &registered_exception_translators = get_internals().registered_exception_translators;
+            if (detail::apply_exception_translators(local_exception_translators)) {
-            for (auto& translator : registered_exception_translators) {
-                try {
-                    translator(last_exception);
-                } catch (...) {
-                    last_exception = std::current_exception();
-                    continue;
-                }
                 return nullptr;
             }
+            auto &exception_translators = get_internals().registered_exception_translators;
+            if (detail::apply_exception_translators(exception_translators)) {
+                return nullptr;
+            }
+
             PyErr_SetString(PyExc_SystemError, "Exception escaped from default exception translator!");
             return nullptr;
         }
@@ -951,6 +977,7 @@ protected:
     }
 };
 
+
 /// Wrapper for Python extension modules
 class module_ : public object {
 public:
@@ -1124,7 +1151,7 @@ protected:
         auto tindex = std::type_index(*rec.type);
         tinfo->direct_conversions = &internals.direct_conversions[tindex];
         if (rec.module_local)
-            registered_local_types_cpp()[tindex] = tinfo;
+            get_local_internals().registered_types_cpp[tindex] = tinfo;
         else
             internals.registered_types_cpp[tindex] = tinfo;
         internals.registered_types_py[(PyTypeObject *) m_ptr] = { tinfo };
@@ -1310,7 +1337,7 @@ public:
         generic_type::initialize(record);
 
         if (has_alias) {
-            auto &instances = record.module_local ? registered_local_types_cpp() : get_internals().registered_types_cpp;
+            auto &instances = record.module_local ? get_local_internals().registered_types_cpp : get_internals().registered_types_cpp;
             instances[std::type_index(typeid(type_alias))] = instances[std::type_index(typeid(type))];
         }
     }
@@ -2010,12 +2037,24 @@ template <typename InputType, typename OutputType> void implicitly_convertible()
         pybind11_fail("implicitly_convertible: Unable to find type " + type_id<OutputType>());
 }
 
-template <typename ExceptionTranslator>
+
-void register_exception_translator(ExceptionTranslator&& translator) {
+inline void register_exception_translator(ExceptionTranslator &&translator) {
     detail::get_internals().registered_exception_translators.push_front(
         std::forward<ExceptionTranslator>(translator));
 }
 
+
+/**
+  * Add a new module-local exception translator. Locally registered functions
+  * will be tried before any globally registered exception translators, which
+  * will only be invoked if the module-local handlers do not deal with
+  * the exception.
+  */
+inline void register_local_exception_translator(ExceptionTranslator &&translator) {
+    detail::get_local_internals().registered_exception_translators.push_front(
+        std::forward<ExceptionTranslator>(translator));
+}
+
 /**
  * Wrapper to generate a new Python exception type.
  *
@@ -2049,22 +2088,20 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 // directly in register_exception, but that makes clang <3.5 segfault - issue #1349).
 template <typename CppException>
 exception<CppException> &get_exception_object() { static exception<CppException> ex; return ex; }
-PYBIND11_NAMESPACE_END(detail)
 
-/**
+// Helper function for register_exception and register_local_exception
- * Registers a Python exception in `m` of the given `name` and installs an exception translator to
- * translate the C++ exception to the created Python exception using the exceptions what() method.
- * This is intended for simple exception translations; for more complex translation, register the
- * exception object and translator directly.
- */
 template <typename CppException>
-exception<CppException> &register_exception(handle scope,
+exception<CppException> &register_exception_impl(handle scope,
                                                 const char *name,
-                                            handle base = PyExc_Exception) {
+                                                handle base,
+                                                bool isLocal) {
     auto &ex = detail::get_exception_object<CppException>();
     if (!ex) ex = exception<CppException>(scope, name, base);
 
-    register_exception_translator([](std::exception_ptr p) {
+    auto register_func = isLocal ? &register_local_exception_translator
+                                 : &register_exception_translator;
+
+    register_func([](std::exception_ptr p) {
         if (!p) return;
         try {
             std::rethrow_exception(p);
@@ -2075,6 +2112,36 @@ exception<CppException> &register_exception(handle scope,
     return ex;
 }
 
+PYBIND11_NAMESPACE_END(detail)
+
+/**
+ * Registers a Python exception in `m` of the given `name` and installs a translator to
+ * translate the C++ exception to the created Python exception using the what() method.
+ * This is intended for simple exception translations; for more complex translation, register the
+ * exception object and translator directly.
+ */
+template <typename CppException>
+exception<CppException> &register_exception(handle scope,
+                                            const char *name,
+                                            handle base = PyExc_Exception) {
+    return detail::register_exception_impl<CppException>(scope, name, base, false /* isLocal */);
+}
+
+/**
+ * Registers a Python exception in `m` of the given `name` and installs a translator to
+ * translate the C++ exception to the created Python exception using the what() method.
+ * This translator will only be used for exceptions that are thrown in this module and will be
+ * tried before global exception translators, including those registered with register_exception.
+ * This is intended for simple exception translations; for more complex translation, register the
+ * exception object and translator directly.
+ */
+template <typename CppException>
+exception<CppException> &register_local_exception(handle scope,
+                                                  const char *name,
+                                                  handle base = PyExc_Exception) {
+    return detail::register_exception_impl<CppException>(scope, name, base, true /* isLocal */);
+}
+
 PYBIND11_NAMESPACE_BEGIN(detail)
 PYBIND11_NOINLINE inline void print(const tuple &args, const dict &kwargs) {
     auto strings = tuple(args.size());

tests/local_bindings.h

@@ -35,6 +35,25 @@ using NonLocalVec2 = std::vector<NonLocal2>;
 using NonLocalMap = std::unordered_map<std::string, NonLocalType>;
 using NonLocalMap2 = std::unordered_map<std::string, uint8_t>;
 
+
+// Exception that will be caught via the module local translator.
+class LocalException : public std::exception {
+public:
+    explicit LocalException(const char * m) : message{m} {}
+    const char * what() const noexcept override {return message.c_str();}
+private:
+    std::string message = "";
+};
+
+// Exception that will be registered with register_local_exception_translator
+class LocalSimpleException : public std::exception {
+public:
+    explicit LocalSimpleException(const char * m) : message{m} {}
+    const char * what() const noexcept override {return message.c_str();}
+private:
+    std::string message = "";
+};
+
 PYBIND11_MAKE_OPAQUE(LocalVec);
 PYBIND11_MAKE_OPAQUE(LocalVec2);
 PYBIND11_MAKE_OPAQUE(LocalMap);

tests/pybind11_cross_module_tests.cpp

@@ -29,11 +29,14 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
     bind_local<ExternalType2>(m, "ExternalType2", py::module_local());
 
     // test_exceptions.py
+    py::register_local_exception<LocalSimpleException>(m, "LocalSimpleException");
     m.def("raise_runtime_error", []() { PyErr_SetString(PyExc_RuntimeError, "My runtime error"); throw py::error_already_set(); });
     m.def("raise_value_error", []() { PyErr_SetString(PyExc_ValueError, "My value error"); throw py::error_already_set(); });
     m.def("throw_pybind_value_error", []() { throw py::value_error("pybind11 value error"); });
     m.def("throw_pybind_type_error", []() { throw py::type_error("pybind11 type error"); });
     m.def("throw_stop_iteration", []() { throw py::stop_iteration(); });
+    m.def("throw_local_error", []() { throw LocalException("just local"); });
+    m.def("throw_local_simple_error", []() { throw LocalSimpleException("external mod"); });
     py::register_exception_translator([](std::exception_ptr p) {
       try {
           if (p) std::rethrow_exception(p);
@@ -42,6 +45,17 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
       }
     });
 
+    // translate the local exception into a key error but only in this module
+    py::register_local_exception_translator([](std::exception_ptr p) {
+      try {
+          if (p) {
+            std::rethrow_exception(p);
+          }
+      } catch (const LocalException &e) {
+        PyErr_SetString(PyExc_KeyError, e.what());
+      }
+    });
+
     // test_local_bindings.py
     // Local to both:
     bind_local<LocalType, 1>(m, "LocalType", py::module_local())
@@ -94,7 +108,7 @@ PYBIND11_MODULE(pybind11_cross_module_tests, m) {
     m.def("get_mixed_lg", [](int i) { return MixedLocalGlobal(i); });
 
     // test_internal_locals_differ
-    m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); });
+    m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::get_local_internals().registered_types_cpp; });
 
     // test_stl_caster_vs_stl_bind
     py::bind_vector<std::vector<int>>(m, "VectorInt");

tests/test_exceptions.cpp

@@ -6,9 +6,10 @@
     All rights reserved. Use of this source code is governed by a
     BSD-style license that can be found in the LICENSE file.
 */
-
 #include "test_exceptions.h"
 
+#include "local_bindings.h"
+
 #include "pybind11_tests.h"
 #include <utility>
 
@@ -68,6 +69,17 @@ class MyException5_1 : public MyException5 {
     using MyException5::MyException5;
 };
 
+
+// Exception that will be caught via the module local translator.
+class MyException6 : public std::exception {
+public:
+    explicit MyException6(const char * m) : message{m} {}
+    const char * what() const noexcept override {return message.c_str();}
+private:
+    std::string message = "";
+};
+
+
 struct PythonCallInDestructor {
     PythonCallInDestructor(const py::dict &d) : d(d) {}
     ~PythonCallInDestructor() { d["good"] = true; }
@@ -138,14 +150,29 @@ TEST_SUBMODULE(exceptions, m) {
     // A slightly more complicated one that declares MyException5_1 as a subclass of MyException5
     py::register_exception<MyException5_1>(m, "MyException5_1", ex5.ptr());
 
+    //py::register_local_exception<LocalSimpleException>(m, "LocalSimpleException")
+
+    py::register_local_exception_translator([](std::exception_ptr p) {
+      try {
+          if (p) {
+            std::rethrow_exception(p);
+          }
+      } catch (const MyException6 &e) {
+        PyErr_SetString(PyExc_RuntimeError, e.what());
+      }
+    });
+
     m.def("throws1", []() { throw MyException("this error should go to a custom type"); });
     m.def("throws2", []() { throw MyException2("this error should go to a standard Python exception"); });
     m.def("throws3", []() { throw MyException3("this error cannot be translated"); });
     m.def("throws4", []() { throw MyException4("this error is rethrown"); });
     m.def("throws5", []() { throw MyException5("this is a helper-defined translated exception"); });
     m.def("throws5_1", []() { throw MyException5_1("MyException5 subclass"); });
+    m.def("throws6", []() { throw MyException6("MyException6 only handled in this module"); });
     m.def("throws_logic_error", []() { throw std::logic_error("this error should fall through to the standard handler"); });
     m.def("throws_overflow_error", []() { throw std::overflow_error(""); });
+    m.def("throws_local_error", []() { throw LocalException("never caught"); });
+    m.def("throws_local_simple_error", []() { throw LocalSimpleException("this mod"); });
     m.def("exception_matches", []() {
         py::dict foo;
         try {

tests/test_exceptions.py

@@ -25,7 +25,7 @@ def test_error_already_set(msg):
     assert msg(excinfo.value) == "foo"
 
 
-def test_cross_module_exceptions():
+def test_cross_module_exceptions(msg):
     with pytest.raises(RuntimeError) as excinfo:
         cm.raise_runtime_error()
     assert str(excinfo.value) == "My runtime error"
@@ -45,6 +45,15 @@ def test_cross_module_exceptions():
     with pytest.raises(StopIteration) as excinfo:
         cm.throw_stop_iteration()
 
+    with pytest.raises(cm.LocalSimpleException) as excinfo:
+        cm.throw_local_simple_error()
+    assert msg(excinfo.value) == "external mod"
+
+    with pytest.raises(KeyError) as excinfo:
+        cm.throw_local_error()
+    # KeyError is a repr of the key, so it has an extra set of quotes
+    assert str(excinfo.value) == "'just local'"
+
 
 # TODO: FIXME
 @pytest.mark.xfail(
@@ -221,3 +230,21 @@ def test_invalid_repr():
 
     with pytest.raises(TypeError):
         m.simple_bool_passthrough(MyRepr())
+
+
+def test_local_translator(msg):
+    """Tests that a local translator works and that the local translator from
+    the cross module is not applied"""
+    with pytest.raises(RuntimeError) as excinfo:
+        m.throws6()
+    assert msg(excinfo.value) == "MyException6 only handled in this module"
+
+    with pytest.raises(RuntimeError) as excinfo:
+        m.throws_local_error()
+    assert not isinstance(excinfo.value, KeyError)
+    assert msg(excinfo.value) == "never caught"
+
+    with pytest.raises(Exception) as excinfo:
+        m.throws_local_simple_error()
+    assert not isinstance(excinfo.value, cm.LocalSimpleException)
+    assert msg(excinfo.value) == "this mod"

tests/test_local_bindings.cpp

@@ -78,7 +78,7 @@ TEST_SUBMODULE(local_bindings, m) {
     m.def("get_mixed_lg", [](int i) { return MixedLocalGlobal(i); });
 
     // test_internal_locals_differ
-    m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::registered_local_types_cpp(); });
+    m.def("local_cpp_types_addr", []() { return (uintptr_t) &py::detail::get_local_internals().registered_types_cpp; });
 
     // test_stl_caster_vs_stl_bind
     m.def("load_vector_via_caster", [](std::vector<int> v) {