Unladen error_already_set

include/pybind11/detail/common.h

@@ -323,8 +323,10 @@
     }
 
 #define PYBIND11_CATCH_INIT_EXCEPTIONS                                                            \
-    catch (pybind11::error_already_set & e) {                                                     \
-        pybind11::raise_from(e, PyExc_ImportError, "initialization failed");                      \
+    catch (pybind11::error_already_set &) {                                                       \
+        PyErr_SetString(                                                                          \
+            PyExc_ImportError,                                                                    \
+            ("initialization failed: " + pybind11::get_error_string_and_clear_error()).c_str());  \
         return nullptr;                                                                           \
     }                                                                                             \
     catch (const std::exception &e) {                                                             \

include/pybind11/detail/internals.h

@@ -332,9 +332,8 @@ inline void translate_exception(std::exception_ptr p) {
     }
     try {
         std::rethrow_exception(p);
-    } catch (error_already_set &e) {
-        handle_nested_exception(e, p);
-        e.restore();
+    } catch (error_already_set &) {
+        // The Python error reporting has already been handled.
         return;
     } catch (const builtin_exception &e) {
         // Could not use template since it's an abstract class.
@@ -391,8 +390,7 @@ inline void translate_local_exception(std::exception_ptr p) {
         if (p) {
             std::rethrow_exception(p);
         }
-    } catch (error_already_set &e) {
-        e.restore();
+    } catch (error_already_set &) {
         return;
     } catch (const builtin_exception &e) {
         e.set_error();

include/pybind11/detail/type_caster_base.h

@@ -478,6 +478,8 @@ PYBIND11_NOINLINE std::string error_string() {
 
     error_scope scope; // Preserve error state
 
+    PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
+
     std::string errorString;
     if (scope.type) {
         errorString += handle(scope.type).attr("__name__").cast<std::string>();
@@ -487,8 +489,6 @@ PYBIND11_NOINLINE std::string error_string() {
         errorString += (std::string) str(scope.value);
     }
 
-    PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);
-
     if (scope.trace != nullptr) {
         PyException_SetTraceback(scope.value, scope.trace);
     }

include/pybind11/embed.h

@@ -143,6 +143,7 @@ inline void initialize_interpreter(bool init_signal_handlers = true,
                                    const char *const *argv = nullptr,
                                    bool add_program_dir_to_path = true) {
     if (Py_IsInitialized() != 0) {
+        PyErr_Clear();
         pybind11_fail("The interpreter is already running");
     }
 

include/pybind11/pybind11.h

@@ -972,8 +972,8 @@ class cpp_function : public function {
                     }
                 }
             }
-        } catch (error_already_set &e) {
-            e.restore();
+        } catch (error_already_set &) {
+            // The Python error reporting has already been handled.
             return nullptr;
 #ifdef __GLIBCXX__
         } catch (abi::__forced_unwind &) {
@@ -1074,6 +1074,7 @@ class cpp_function : public function {
                 try {
                     msg += pybind11::repr(args_[ti]);
                 } catch (const error_already_set &) {
+                    PyErr_Clear();
                     msg += "<repr raised Error>";
                 }
             }
@@ -1095,6 +1096,7 @@ class cpp_function : public function {
                         try {
                             msg += pybind11::repr(kwarg.second);
                         } catch (const error_already_set &) {
+                            PyErr_Clear();
                             msg += "<repr raised Error>";
                         }
                     }
@@ -1173,9 +1175,16 @@ class module_ : public object {
             py::module_ m3 = m2.def_submodule("subsub", "A submodule of 'example.sub'");
     \endrst */
     module_ def_submodule(const char *name, const char *doc = nullptr) {
-        std::string full_name
-            = std::string(PyModule_GetName(m_ptr)) + std::string(".") + std::string(name);
-        auto result = reinterpret_borrow<module_>(PyImport_AddModule(full_name.c_str()));
+        const char *this_name = PyModule_GetName(m_ptr);
+        if (this_name == nullptr) {
+            throw error_already_set();
+        }
+        std::string full_name = std::string(this_name) + '.' + name;
+        handle submodule = PyImport_AddModule(full_name.c_str());
+        if (!submodule) {
+            throw error_already_set();
+        }
+        auto result = reinterpret_borrow<module_>(submodule);
         if (doc && options::show_user_defined_docstrings()) {
             result.attr("__doc__") = pybind11::str(doc);
         }
@@ -2587,6 +2596,7 @@ PYBIND11_NOINLINE void print(const tuple &args, const dict &kwargs) {
         try {
             file = module_::import("sys").attr("stdout");
         } catch (const error_already_set &) {
+            PyErr_Clear();
             /* If print() is called from code that is executed as
                part of garbage collection during interpreter shutdown,
                importing 'sys' can fail. Give up rather than crashing the
@@ -2611,16 +2621,6 @@ void print(Args &&...args) {
     detail::print(c.args(), c.kwargs());
 }
 
-error_already_set::~error_already_set() {
-    if (m_type) {
-        gil_scoped_acquire gil;
-        error_scope scope;
-        m_type.release().dec_ref();
-        m_value.release().dec_ref();
-        m_trace.release().dec_ref();
-    }
-}
-
 PYBIND11_NAMESPACE_BEGIN(detail)
 inline function
 get_type_override(const void *this_ptr, const type_info *this_type, const char *name) {

include/pybind11/pytypes.h

@@ -369,38 +369,28 @@ PYBIND11_NAMESPACE_END(detail)
 //     warning C4275: An exported class was derived from a class that wasn't exported.
 //     Can be ignored when derived from a STL class.
 #endif
-/// Fetch and hold an error which was already set in Python.  An instance of this is typically
-/// thrown to propagate python-side errors back through C++ which can either be caught manually or
-/// else falls back to the function dispatcher (which then raises the captured error back to
-/// python).
-class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error {
+class PYBIND11_EXPORT_EXCEPTION error_already_set {
 public:
-    /// Constructs a new exception from the current Python error indicator, if any.  The current
-    /// Python error indicator will be cleared.
-    error_already_set() : std::runtime_error(detail::error_string()) {
-        PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
-    }
+    virtual ~error_already_set() = default;
 
+    // Compatibility with older compilers.
+    error_already_set() = default;
     error_already_set(const error_already_set &) = default;
     error_already_set(error_already_set &&) = default;
 
-    inline ~error_already_set() override;
-
-    /// Give the currently-held error back to Python, if any.  If there is currently a Python error
-    /// already set it is cleared first.  After this call, the current object no longer stores the
-    /// error variables (but the `.what()` string is still available).
-    void restore() {
-        PyErr_Restore(m_type.release().ptr(), m_value.release().ptr(), m_trace.release().ptr());
-    }
-
     /// If it is impossible to raise the currently-held error, such as in a destructor, we can
     /// write it out using Python's unraisable hook (`sys.unraisablehook`). The error context
     /// should be some object whose `repr()` helps identify the location of the error. Python
     /// already knows the type and value of the error, so there is no need to repeat that. After
     /// this call, the current object no longer stores the error variables, and neither does
     /// Python.
     void discard_as_unraisable(object err_context) {
-        restore();
+#if PY_VERSION_HEX < 0x03080000
+        PyObject *exc = nullptr, *val = nullptr, *tb = nullptr;
+        PyErr_Fetch(&exc, &val, &tb);
+        PyErr_NormalizeException(&exc, &val, &tb);
+        PyErr_Restore(exc, val, tb);
+#endif
         PyErr_WriteUnraisable(err_context.ptr());
     }
     /// An alternate version of `discard_as_unraisable()`, where a string provides information on
@@ -409,28 +399,27 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error {
         discard_as_unraisable(reinterpret_steal<object>(PYBIND11_FROM_STRING(err_context)));
     }
 
-    // Does nothing; provided for backwards compatibility.
-    PYBIND11_DEPRECATED("Use of error_already_set.clear() is deprecated")
-    void clear() {}
-
     /// Check if the currently trapped error type matches the given Python exception class (or a
     /// subclass thereof).  May also be passed a tuple to search for any exception class matches in
     /// the given tuple.
     bool matches(handle exc) const {
-        return (PyErr_GivenExceptionMatches(m_type.ptr(), exc.ptr()) != 0);
+        PyObject *exc_type = PyErr_Occurred();
+        if (exc_type == nullptr) {
+            return false;
+        }
+        return (PyErr_GivenExceptionMatches(exc_type, exc.ptr()) != 0);
     }
-
-    const object &type() const { return m_type; }
-    const object &value() const { return m_value; }
-    const object &trace() const { return m_trace; }
-
-private:
-    object m_type, m_value, m_trace;
 };
 #if defined(_MSC_VER)
 #    pragma warning(pop)
 #endif
 
+inline std::string get_error_string_and_clear_error() {
+    std::string result = detail::error_string();
+    PyErr_Clear();
+    return result;
+}
+
 /// Replaces the current Python error indicator with the chosen error, performing a
 /// 'raise from' to indicate that the chosen error was caused by the original error.
 inline void raise_from(PyObject *type, const char *message) {
@@ -459,15 +448,6 @@ inline void raise_from(PyObject *type, const char *message) {
     PyErr_Restore(exc, val2, tb);
 }
 
-/// Sets the current Python error indicator with the chosen error, performing a 'raise from'
-/// from the error contained in error_already_set to indicate that the chosen error was
-/// caused by the original error. After this function is called error_already_set will
-/// no longer contain an error.
-inline void raise_from(error_already_set &err, PyObject *type, const char *message) {
-    err.restore();
-    raise_from(type, message);
-}
-
 /** \defgroup python_builtins const_name
     Unless stated otherwise, the following C++ functions behave the same
     as their Python counterparts.

tests/test_embed/test_interpreter.cpp

@@ -125,17 +125,29 @@ TEST_CASE("Override cache") {
 
 TEST_CASE("Import error handling") {
     REQUIRE_NOTHROW(py::module_::import("widget_module"));
-    REQUIRE_THROWS_WITH(py::module_::import("throw_exception"), "ImportError: C++ Error");
-    REQUIRE_THROWS_WITH(py::module_::import("throw_error_already_set"),
-                        Catch::Contains("ImportError: initialization failed"));
+    try {
+        py::module_::import("throw_exception");
+    } catch (const py::error_already_set &) {
+        REQUIRE(py::get_error_string_and_clear_error() == "ImportError: C++ Error");
+    }
+    try {
+        py::module_::import("throw_error_already_set");
+    } catch (const py::error_already_set &) {
+        REQUIRE_THAT(py::get_error_string_and_clear_error(),
+                     Catch::Contains("ImportError: initialization failed"));
+    }
 
     auto locals = py::dict("is_keyerror"_a = false, "message"_a = "not set");
     py::exec(R"(
         try:
             import throw_error_already_set
         except ImportError as e:
-            is_keyerror = type(e.__cause__) == KeyError
-            message = str(e.__cause__)
+            # TODO: Undo temporary workaround for PYBIND11_CATCH_INIT_EXCEPTIONS issue
+            #       that exists already on master (see PR #3965).
+            # is_keyerror = type(e.__cause__) == KeyError
+            # message = str(e.__cause__)
+            is_keyerror = "KeyError" in str(e)
+            message = str(e).split()[-1]
     )",
              py::globals(),
              locals);

tests/test_eval.cpp

@@ -73,6 +73,7 @@ TEST_SUBMODULE(eval_, m) {
         try {
             py::eval("nonsense code ...");
         } catch (py::error_already_set &) {
+            PyErr_Clear();
             return true;
         }
         return false;

tests/test_exceptions.cpp

@@ -190,6 +190,7 @@ TEST_SUBMODULE(exceptions, m) {
             if (!ex.matches(PyExc_KeyError)) {
                 throw;
             }
+            PyErr_Clear();
             return true;
         }
         return false;
@@ -203,6 +204,7 @@ TEST_SUBMODULE(exceptions, m) {
             if (!ex.matches(PyExc_Exception)) {
                 throw;
             }
+            PyErr_Clear();
             return true;
         }
         return false;
@@ -215,6 +217,7 @@ TEST_SUBMODULE(exceptions, m) {
             if (!ex.matches(PyExc_ImportError)) {
                 throw;
             }
+            PyErr_Clear();
             return true;
         }
         return false;
@@ -223,21 +226,9 @@ TEST_SUBMODULE(exceptions, m) {
     m.def("throw_already_set", [](bool err) {
         if (err) {
             PyErr_SetString(PyExc_ValueError, "foo");
-        }
-        try {
             throw py::error_already_set();
-        } catch (const std::runtime_error &e) {
-            if ((err && e.what() != std::string("ValueError: foo"))
-                || (!err && e.what() != std::string("Unknown internal error occurred"))) {
-                PyErr_Clear();
-                throw std::runtime_error("error message mismatch");
-            }
         }
-        PyErr_Clear();
-        if (err) {
-            PyErr_SetString(PyExc_ValueError, "foo");
-        }
-        throw py::error_already_set();
+        return py::get_error_string_and_clear_error();
     });
 
     m.def("python_call_in_destructor", [](const py::dict &d) {
@@ -247,6 +238,7 @@ TEST_SUBMODULE(exceptions, m) {
             PyErr_SetString(PyExc_ValueError, "foo");
             throw py::error_already_set();
         } catch (const py::error_already_set &) {
+            PyErr_Clear();
             retval = true;
         }
         return retval;
@@ -264,7 +256,7 @@ TEST_SUBMODULE(exceptions, m) {
                   f(*args);
               } catch (py::error_already_set &ex) {
                   if (ex.matches(exc_type)) {
-                      py::print(ex.what());
+                      py::print(py::get_error_string_and_clear_error());
                   } else {
                       throw;
                   }
@@ -286,8 +278,8 @@ TEST_SUBMODULE(exceptions, m) {
         try {
             PyErr_SetString(PyExc_ValueError, "inner");
             throw py::error_already_set();
-        } catch (py::error_already_set &e) {
-            py::raise_from(e, PyExc_ValueError, "outer");
+        } catch (py::error_already_set &) {
+            py::raise_from(PyExc_ValueError, "outer");
             throw py::error_already_set();
         }
     });

tests/test_exceptions.py

@@ -14,9 +14,7 @@ def test_std_exception(msg):
 
 
 def test_error_already_set(msg):
-    with pytest.raises(RuntimeError) as excinfo:
-        m.throw_already_set(False)
-    assert msg(excinfo.value) == "Unknown internal error occurred"
+    assert m.throw_already_set(False) == "Unknown internal error occurred"
 
     with pytest.raises(ValueError) as excinfo:
         m.throw_already_set(True)

tests/test_numpy_array.cpp

@@ -162,7 +162,8 @@ static int data_i = 42;
 TEST_SUBMODULE(numpy_array, sm) {
     try {
         py::module_::import("numpy");
-    } catch (...) {
+    } catch (const py::error_already_set &) {
+        PyErr_Clear();
         return;
     }
 

tests/test_numpy_dtypes.cpp

@@ -301,7 +301,8 @@ struct B {};
 TEST_SUBMODULE(numpy_dtypes, m) {
     try {
         py::module_::import("numpy");
-    } catch (...) {
+    } catch (py::error_already_set &) {
+        PyErr_Clear();
         return;
     }
 

tests/test_numpy_vectorize.cpp

@@ -22,7 +22,8 @@ double my_func(int x, float y, double z) {
 TEST_SUBMODULE(numpy_vectorize, m) {
     try {
         py::module_::import("numpy");
-    } catch (...) {
+    } catch (py::error_already_set &) {
+        PyErr_Clear();
         return;
     }