diff --git a/include/pybind11/detail/common.h b/include/pybind11/detail/common.h index 9355ecfda0..122fa49122 100644 --- a/include/pybind11/detail/common.h +++ b/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) { \ diff --git a/include/pybind11/detail/internals.h b/include/pybind11/detail/internals.h index 3f83113bd7..a766b5c476 100644 --- a/include/pybind11/detail/internals.h +++ b/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(); diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a7b9771326..c250bc6e6e 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/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(); @@ -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); } diff --git a/include/pybind11/embed.h b/include/pybind11/embed.h index 98cc3e4664..c2a80e3e14 100644 --- a/include/pybind11/embed.h +++ b/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"); } diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index ca94aaeb3a..895384c5ef 100644 --- a/include/pybind11/pybind11.h +++ b/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 += ""; } } @@ -1095,6 +1096,7 @@ class cpp_function : public function { try { msg += pybind11::repr(kwarg.second); } catch (const error_already_set &) { + PyErr_Clear(); msg += ""; } } @@ -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(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(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) { diff --git a/include/pybind11/pytypes.h b/include/pybind11/pytypes.h index 256a2441b1..70829b11eb 100644 --- a/include/pybind11/pytypes.h +++ b/include/pybind11/pytypes.h @@ -369,30 +369,15 @@ 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 @@ -400,7 +385,12 @@ class PYBIND11_EXPORT_EXCEPTION error_already_set : public std::runtime_error { /// 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(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. diff --git a/tests/test_embed/test_interpreter.cpp b/tests/test_embed/test_interpreter.cpp index 1c45457a05..f8b36a138c 100644 --- a/tests/test_embed/test_interpreter.cpp +++ b/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); diff --git a/tests/test_eval.cpp b/tests/test_eval.cpp index cd2903f0ab..9de080ab40 100644 --- a/tests/test_eval.cpp +++ b/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; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 3e9a3d771e..b94a4dd4d0 100644 --- a/tests/test_exceptions.cpp +++ b/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(); } }); diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0be61804a6..93d81a8f06 100644 --- a/tests/test_exceptions.py +++ b/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) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 8bc77a33fa..c43dc54ccb 100644 --- a/tests/test_numpy_array.cpp +++ b/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; } diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index c25bc042b8..29507b3454 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/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; } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index eab08ec0a6..4a9bfee737 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/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; } diff --git a/tests/test_pytypes.cpp b/tests/test_pytypes.cpp index 8d296f655a..b55283baa4 100644 --- a/tests/test_pytypes.cpp +++ b/tests/test_pytypes.cpp @@ -243,11 +243,13 @@ TEST_SUBMODULE(pytypes, m) { try { o.attr("sub").attr("missing").ptr(); } catch (const py::error_already_set &) { + PyErr_Clear(); d["missing_attr_ptr"] = "raised"_s; } try { o.attr("missing").attr("doesn't matter"); } catch (const py::error_already_set &) { + PyErr_Clear(); d["missing_attr_chain"] = "raised"_s; } @@ -269,6 +271,7 @@ TEST_SUBMODULE(pytypes, m) { try { existing_t[0] = 1; } catch (const py::error_already_set &) { + PyErr_Clear(); // --> Python system error // Only new tuples (refcount == 1) are mutable auto new_t = py::tuple(3); diff --git a/tests/test_stl_binders.cpp b/tests/test_stl_binders.cpp index ca9630bd19..c6d05b1642 100644 --- a/tests/test_stl_binders.cpp +++ b/tests/test_stl_binders.cpp @@ -132,7 +132,8 @@ TEST_SUBMODULE(stl_binders, m) { // The rest depends on numpy: try { py::module_::import("numpy"); - } catch (...) { + } catch (py::error_already_set &) { + PyErr_Clear(); return; }