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

Author: jesse-sony

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
-a new Python exception using the C++ exception's ``what()`` method, a helper
-function is available:
+Similar to pybind11 classes, exception translators can be local to the module
+they are defined in or global to the entire python session.  To register a simple
+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,29 +88,39 @@ 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`.
 
 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
-``py::register_exception_translator(translator)`` can be used to register
+When more advanced exception translation is needed, the functions
+``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() {
-    static type_map<type_info *> locals{};
-    return locals;
+
+// 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;
+};
+
+/// 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 @@ class cpp_function : public function {
         }
     }
 
+
     /// 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 @@ class cpp_function : public function {
 #endif
         } catch (...) {
             /* When an exception is caught, give each registered exception
-               translator a chance to translate it to a Python exception
-               in reverse order of registration.
+               translator a chance to translate it to a Python exception. First
+               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 @@ class cpp_function : public function {
                 - 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 &registered_exception_translators = get_internals().registered_exception_translators;
-            for (auto& translator : registered_exception_translators) {
-                try {
-                    translator(last_exception);
-                } catch (...) {
-                    last_exception = std::current_exception();
-                    continue;
-                }
+            auto &local_exception_translators = get_local_internals().registered_exception_translators;
+            if (detail::apply_exception_translators(local_exception_translators)) {
+                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 @@ class cpp_function : public function {
     }
 };
 
+
 /// Wrapper for Python extension modules
 class module_ : public object {
 public:
@@ -1124,7 +1151,7 @@ class generic_type : public object {
         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 @@ class class_ : public detail::generic_type {
         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)
 
-/**
- * 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.
- */
+// Helper function for register_exception and register_local_exception
 template <typename CppException>
-exception<CppException> &register_exception(handle scope,
-                                            const char *name,
-                                            handle base = PyExc_Exception) {
+exception<CppException> &register_exception_impl(handle scope,
+                                                const char *name,
+                                                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());