From 2c549eb7aa16bf1297af337e1b022e9ed2a4dc52 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Wed, 25 May 2022 21:44:55 -0700 Subject: [PATCH 1/2] Move `PyErr_NormalizeException()` up a few lines (#3971) * Add error_already_set_what what tests, asserting the status quo. * Move PyErr_NormalizeException() up a few lines. * @pytest.mark.skipif("env.PYPY") from PR #1895 is required even for this much simpler PR * Move PyException_SetTraceback() with PyErr_NormalizeException() as suggested by @skylion007 * Insert a std::move() as suggested by @skylion007 --- include/pybind11/detail/type_caster_base.h | 11 +++-- tests/test_exceptions.cpp | 8 ++++ tests/test_exceptions.py | 50 ++++++++++++++++++++++ 3 files changed, 63 insertions(+), 6 deletions(-) diff --git a/include/pybind11/detail/type_caster_base.h b/include/pybind11/detail/type_caster_base.h index a7b977132..14561759b 100644 --- a/include/pybind11/detail/type_caster_base.h +++ b/include/pybind11/detail/type_caster_base.h @@ -478,6 +478,11 @@ PYBIND11_NOINLINE std::string error_string() { error_scope scope; // Preserve error state + PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace); + if (scope.trace != nullptr) { + PyException_SetTraceback(scope.value, scope.trace); + } + std::string errorString; if (scope.type) { errorString += handle(scope.type).attr("__name__").cast(); @@ -487,12 +492,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); - } - #if !defined(PYPY_VERSION) if (scope.trace) { auto *trace = (PyTracebackObject *) scope.trace; diff --git a/tests/test_exceptions.cpp b/tests/test_exceptions.cpp index 3e9a3d771..d7b31cf74 100644 --- a/tests/test_exceptions.cpp +++ b/tests/test_exceptions.cpp @@ -299,4 +299,12 @@ TEST_SUBMODULE(exceptions, m) { std::throw_with_nested(std::runtime_error("Outer Exception")); } }); + + m.def("error_already_set_what", [](const py::object &exc_type, const py::object &exc_value) { + PyErr_SetObject(exc_type.ptr(), exc_value.ptr()); + std::string what = py::error_already_set().what(); + bool py_err_set_after_what = (PyErr_Occurred() != nullptr); + PyErr_Clear(); + return py::make_tuple(std::move(what), py_err_set_after_what); + }); } diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 0be61804a..dc7594113 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -270,3 +270,53 @@ def test_local_translator(msg): m.throws_local_simple_error() assert not isinstance(excinfo.value, cm.LocalSimpleException) assert msg(excinfo.value) == "this mod" + + +class FlakyException(Exception): + def __init__(self, failure_point): + if failure_point == "failure_point_init": + raise ValueError("triggered_failure_point_init") + self.failure_point = failure_point + + def __str__(self): + if self.failure_point == "failure_point_str": + raise ValueError("triggered_failure_point_str") + return "FlakyException.__str__" + + +@pytest.mark.parametrize( + "exc_type, exc_value, expected_what", + ( + (ValueError, "plain_str", "ValueError: plain_str"), + (ValueError, ("tuple_elem",), "ValueError: tuple_elem"), + (FlakyException, ("happy",), "FlakyException: FlakyException.__str__"), + ), +) +def test_error_already_set_what_with_happy_exceptions( + exc_type, exc_value, expected_what +): + what, py_err_set_after_what = m.error_already_set_what(exc_type, exc_value) + assert not py_err_set_after_what + assert what == expected_what + + +@pytest.mark.skipif("env.PYPY", reason="PyErr_NormalizeException Segmentation fault") +def test_flaky_exception_failure_point_init(): + what, py_err_set_after_what = m.error_already_set_what( + FlakyException, ("failure_point_init",) + ) + assert not py_err_set_after_what + lines = what.splitlines() + # PyErr_NormalizeException replaces the original FlakyException with ValueError: + assert lines[:3] == ["ValueError: triggered_failure_point_init", "", "At:"] + # Checking the first two lines of the traceback as formatted in error_string(): + assert "test_exceptions.py(" in lines[3] + assert lines[3].endswith("): __init__") + assert lines[4].endswith("): test_flaky_exception_failure_point_init") + + +def test_flaky_exception_failure_point_str(): + # The error_already_set ctor fails due to a ValueError in error_string(): + with pytest.raises(ValueError) as excinfo: + m.error_already_set_what(FlakyException, ("failure_point_str",)) + assert str(excinfo.value) == "triggered_failure_point_str" From 8d14e666e3aab5c958169e23b619ae8b17b09ea6 Mon Sep 17 00:00:00 2001 From: "Ralf W. Grosse-Kunstleve" Date: Thu, 26 May 2022 08:07:40 -0700 Subject: [PATCH 2/2] fix: avoid `catch (...)` for expected `import numpy` failures (#3974) * Replace import numpy catch (...) with catch (error_already_set) * Add missing const (not sure how those got lost). --- tests/test_numpy_array.cpp | 2 +- tests/test_numpy_dtypes.cpp | 2 +- tests/test_numpy_vectorize.cpp | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_numpy_array.cpp b/tests/test_numpy_array.cpp index 8bc77a33f..69ddbe1ef 100644 --- a/tests/test_numpy_array.cpp +++ b/tests/test_numpy_array.cpp @@ -162,7 +162,7 @@ static int data_i = 42; TEST_SUBMODULE(numpy_array, sm) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; } diff --git a/tests/test_numpy_dtypes.cpp b/tests/test_numpy_dtypes.cpp index c25bc042b..6654f9ed8 100644 --- a/tests/test_numpy_dtypes.cpp +++ b/tests/test_numpy_dtypes.cpp @@ -301,7 +301,7 @@ struct B {}; TEST_SUBMODULE(numpy_dtypes, m) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; } diff --git a/tests/test_numpy_vectorize.cpp b/tests/test_numpy_vectorize.cpp index eab08ec0a..dcc4c6ac2 100644 --- a/tests/test_numpy_vectorize.cpp +++ b/tests/test_numpy_vectorize.cpp @@ -22,7 +22,7 @@ double my_func(int x, float y, double z) { TEST_SUBMODULE(numpy_vectorize, m) { try { py::module_::import("numpy"); - } catch (...) { + } catch (const py::error_already_set &) { return; }