Simplify error_already_set

include/pybind11/common.h

@@ -231,7 +231,6 @@ extern "C" {
         try {                                                                  \
             return pybind11_init();                                            \
         } catch (pybind11::error_already_set &e) {                             \
-            e.clear();                                                         \
             PyErr_SetString(PyExc_ImportError, e.what());                      \
             return nullptr;                                                    \
         } catch (const std::exception &e) {                                    \
@@ -278,7 +277,6 @@ extern "C" {
             pybind11_init_##name(m);                                           \
             return m.ptr();                                                    \
         } catch (pybind11::error_already_set &e) {                             \
-            e.clear();                                                         \
             PyErr_SetString(PyExc_ImportError, e.what());                      \
             return nullptr;                                                    \
         } catch (const std::exception &e) {                                    \
@@ -353,8 +351,6 @@ inline static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : lo
 // Returns the size as a multiple of sizeof(void *), rounded up.
 inline static constexpr size_t size_in_ptrs(size_t s) { return 1 + ((s - 1) >> log2(sizeof(void *))); }
 
-inline std::string error_string();
-
 /**
  * The space to allocate for simple layout instance holders (see below) in multiple of the size of
  * a pointer (e.g.  2 means 16 bytes on 64-bit architectures).  The default is the minimum required
@@ -703,36 +699,6 @@ template<typename T> T& get_or_create_shared_data(const std::string& name) {
     return *ptr;
 }
 
-/// Fetch and hold an error which was already set in Python
-class error_already_set : public std::runtime_error {
-public:
-    error_already_set() : std::runtime_error(detail::error_string()) {
-        PyErr_Fetch(&type, &value, &trace);
-    }
-
-    error_already_set(const error_already_set &) = delete;
-
-    error_already_set(error_already_set &&e)
-        : std::runtime_error(e.what()), type(e.type), value(e.value),
-          trace(e.trace) { e.type = e.value = e.trace = nullptr; }
-
-    inline ~error_already_set(); // implementation in pybind11.h
-
-    error_already_set& operator=(const error_already_set &) = delete;
-
-    /// Give the error back to Python
-    void restore() { PyErr_Restore(type, value, trace); type = value = trace = nullptr; }
-
-    /// Clear the held Python error state (the C++ `what()` message remains intact)
-    void clear() { restore(); PyErr_Clear(); }
-
-    /// Check if the trapped exception matches a given Python exception class
-    bool matches(PyObject *ex) const { return PyErr_GivenExceptionMatches(ex, type); }
-
-private:
-    PyObject *type, *value, *trace;
-};
-
 /// C++ bindings of builtin Python exceptions
 class builtin_exception : public std::runtime_error {
 public:

include/pybind11/embed.h

@@ -51,7 +51,6 @@
             pybind11_init_##name(m);                                          \
             return m.ptr();                                                   \
         } catch (pybind11::error_already_set &e) {                            \
-            e.clear();                                                        \
             PyErr_SetString(PyExc_ImportError, e.what());                     \
             return nullptr;                                                   \
         } catch (const std::exception &e) {                                   \

include/pybind11/pybind11.h

@@ -1754,9 +1754,11 @@ class gil_scoped_release { };
 #endif
 
 error_already_set::~error_already_set() {
-    if (value) {
+    if (type) {
         gil_scoped_acquire gil;
-        clear();
+        type.release().dec_ref();
+        value.release().dec_ref();
+        trace.release().dec_ref();
     }
 }
 

include/pybind11/pytypes.h

@@ -279,6 +279,42 @@ template <typename T> T reinterpret_borrow(handle h) { return {h, object::borrow
 \endrst */
 template <typename T> T reinterpret_steal(handle h) { return {h, object::stolen_t{}}; }
 
+NAMESPACE_BEGIN(detail)
+inline std::string error_string();
+NAMESPACE_END(detail)
+
+/// 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 error_already_set : public std::runtime_error {
+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(&type.ptr(), &value.ptr(), &trace.ptr());
+    }
+
+    inline ~error_already_set();
+
+    /// 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(type.release().ptr(), value.release().ptr(), trace.release().ptr()); }
+
+    // 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 ex) const { return PyErr_GivenExceptionMatches(ex.ptr(), type.ptr()); }
+
+private:
+    object type, value, trace;
+};
+
 /** \defgroup python_builtins _
     Unless stated otherwise, the following C++ functions behave the same
     as their Python counterparts.

tests/test_exceptions.cpp

@@ -58,56 +58,14 @@ class MyException5_1 : public MyException5 {
     using MyException5::MyException5;
 };
 
-void throws1() {
-    throw MyException("this error should go to a custom type");
-}
-
-void throws2() {
-    throw MyException2("this error should go to a standard Python exception");
-}
-
-void throws3() {
-    throw MyException3("this error cannot be translated");
-}
-
-void throws4() {
-    throw MyException4("this error is rethrown");
-}
-
-void throws5() {
-    throw MyException5("this is a helper-defined translated exception");
-}
-
-void throws5_1() {
-    throw MyException5_1("MyException5 subclass");
-}
-
-void throws_logic_error() {
-    throw std::logic_error("this error should fall through to the standard handler");
-}
-
-// Test error_already_set::matches() method
-void exception_matches() {
-    py::dict foo;
-    try {
-        foo["bar"];
-    }
-    catch (py::error_already_set& ex) {
-        if (ex.matches(PyExc_KeyError))
-            ex.clear();
-        else
-            throw;
-    }
-}
-
 struct PythonCallInDestructor {
     PythonCallInDestructor(const py::dict &d) : d(d) {}
     ~PythonCallInDestructor() { d["good"] = true; }
 
     py::dict d;
 };
 
-test_initializer custom_exceptions([](py::module &m) {
+TEST_SUBMODULE(exceptions, m) {
     m.def("throw_std_exception", []() {
         throw std::runtime_error("This exception was intentionally thrown.");
     });
@@ -151,14 +109,20 @@ test_initializer custom_exceptions([](py::module &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());
 
-    m.def("throws1", &throws1);
-    m.def("throws2", &throws2);
-    m.def("throws3", &throws3);
-    m.def("throws4", &throws4);
-    m.def("throws5", &throws5);
-    m.def("throws5_1", &throws5_1);
-    m.def("throws_logic_error", &throws_logic_error);
-    m.def("exception_matches", &exception_matches);
+    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("throws_logic_error", []() { throw std::logic_error("this error should fall through to the standard handler"); });
+    m.def("exception_matches", []() {
+        py::dict foo;
+        try { foo["bar"]; }
+        catch (py::error_already_set& ex) {
+            if (!ex.matches(PyExc_KeyError)) throw;
+        }
+    });
 
     m.def("throw_already_set", [](bool err) {
         if (err)
@@ -189,4 +153,16 @@ test_initializer custom_exceptions([](py::module &m) {
         }
         return false;
     });
-});
+
+    // test_nested_throws
+    m.def("try_catch", [m](py::object exc_type, py::function f, py::args args) {
+        try { f(*args); }
+        catch (py::error_already_set &ex) {
+            if (ex.matches(exc_type))
+                py::print(ex.what());
+            else
+                throw;
+        }
+    });
+
+}

tests/test_exceptions.py

@@ -1,87 +1,122 @@
 import pytest
 
+from pybind11_tests import exceptions as m
 
-def test_std_exception(msg):
-    from pybind11_tests import throw_std_exception
 
+def test_std_exception(msg):
     with pytest.raises(RuntimeError) as excinfo:
-        throw_std_exception()
+        m.throw_std_exception()
     assert msg(excinfo.value) == "This exception was intentionally thrown."
 
 
 def test_error_already_set(msg):
-    from pybind11_tests import throw_already_set
-
     with pytest.raises(RuntimeError) as excinfo:
-        throw_already_set(False)
+        m.throw_already_set(False)
     assert msg(excinfo.value) == "Unknown internal error occurred"
 
     with pytest.raises(ValueError) as excinfo:
-        throw_already_set(True)
+        m.throw_already_set(True)
     assert msg(excinfo.value) == "foo"
 
 
 def test_python_call_in_catch():
-    from pybind11_tests import python_call_in_destructor
-
     d = {}
-    assert python_call_in_destructor(d) is True
+    assert m.python_call_in_destructor(d) is True
     assert d["good"] is True
 
 
 def test_exception_matches():
-    from pybind11_tests import exception_matches
-    exception_matches()
+    m.exception_matches()
 
 
 def test_custom(msg):
-    from pybind11_tests import (MyException, MyException5, MyException5_1,
-                                throws1, throws2, throws3, throws4, throws5, throws5_1,
-                                throws_logic_error)
-
-    # Can we catch a MyException?"
-    with pytest.raises(MyException) as excinfo:
-        throws1()
+    # Can we catch a MyException?
+    with pytest.raises(m.MyException) as excinfo:
+        m.throws1()
     assert msg(excinfo.value) == "this error should go to a custom type"
 
     # Can we translate to standard Python exceptions?
     with pytest.raises(RuntimeError) as excinfo:
-        throws2()
+        m.throws2()
     assert msg(excinfo.value) == "this error should go to a standard Python exception"
 
     # Can we handle unknown exceptions?
     with pytest.raises(RuntimeError) as excinfo:
-        throws3()
+        m.throws3()
     assert msg(excinfo.value) == "Caught an unknown exception!"
 
     # Can we delegate to another handler by rethrowing?
-    with pytest.raises(MyException) as excinfo:
-        throws4()
+    with pytest.raises(m.MyException) as excinfo:
+        m.throws4()
     assert msg(excinfo.value) == "this error is rethrown"
 
-    # "Can we fall-through to the default handler?"
+    # Can we fall-through to the default handler?
     with pytest.raises(RuntimeError) as excinfo:
-        throws_logic_error()
+        m.throws_logic_error()
     assert msg(excinfo.value) == "this error should fall through to the standard handler"
 
     # Can we handle a helper-declared exception?
-    with pytest.raises(MyException5) as excinfo:
-        throws5()
+    with pytest.raises(m.MyException5) as excinfo:
+        m.throws5()
     assert msg(excinfo.value) == "this is a helper-defined translated exception"
 
     # Exception subclassing:
-    with pytest.raises(MyException5) as excinfo:
-        throws5_1()
+    with pytest.raises(m.MyException5) as excinfo:
+        m.throws5_1()
     assert msg(excinfo.value) == "MyException5 subclass"
-    assert isinstance(excinfo.value, MyException5_1)
+    assert isinstance(excinfo.value, m.MyException5_1)
 
-    with pytest.raises(MyException5_1) as excinfo:
-        throws5_1()
+    with pytest.raises(m.MyException5_1) as excinfo:
+        m.throws5_1()
     assert msg(excinfo.value) == "MyException5 subclass"
 
-    with pytest.raises(MyException5) as excinfo:
+    with pytest.raises(m.MyException5) as excinfo:
         try:
-            throws5()
-        except MyException5_1:
+            m.throws5()
+        except m.MyException5_1:
             raise RuntimeError("Exception error: caught child from parent")
     assert msg(excinfo.value) == "this is a helper-defined translated exception"
+
+
+def test_nested_throws(capture):
+    """Tests nested (e.g. C++ -> Python -> C++) exception handling"""
+
+    def throw_myex():
+        raise m.MyException("nested error")
+
+    def throw_myex5():
+        raise m.MyException5("nested error 5")
+
+    # In the comments below, the exception is caught in the first step, thrown in the last step
+
+    # C++ -> Python
+    with capture:
+        m.try_catch(m.MyException5, throw_myex5)
+    assert str(capture).startswith("MyException5: nested error 5")
+
+    # Python -> C++ -> Python
+    with pytest.raises(m.MyException) as excinfo:
+        m.try_catch(m.MyException5, throw_myex)
+    assert str(excinfo.value) == "nested error"
+
+    def pycatch(exctype, f, *args):
+        try:
+            f(*args)
+        except m.MyException as e:
+            print(e)
+
+    # C++ -> Python -> C++ -> Python
+    with capture:
+        m.try_catch(
+            m.MyException5, pycatch, m.MyException, m.try_catch, m.MyException, throw_myex5)
+    assert str(capture).startswith("MyException5: nested error 5")
+
+    # C++ -> Python -> C++
+    with capture:
+        m.try_catch(m.MyException, pycatch, m.MyException5, m.throws4)
+    assert capture == "this error is rethrown"
+
+    # Python -> C++ -> Python -> C++
+    with pytest.raises(m.MyException5) as excinfo:
+        m.try_catch(m.MyException, pycatch, m.MyException, m.throws5)
+    assert str(excinfo.value) == "this is a helper-defined translated exception"