Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move PyErr_NormalizeException() up a few lines #3971

Merged
merged 5 commits into from
May 26, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -486,9 +488,6 @@ PYBIND11_NOINLINE std::string error_string() {
if (scope.value) {
errorString += (std::string) str(scope.value);
}

PyErr_NormalizeException(&scope.type, &scope.value, &scope.trace);

if (scope.trace != nullptr) {
Skylion007 marked this conversation as resolved.
Show resolved Hide resolved
PyException_SetTraceback(scope.value, scope.trace);
}
Expand Down
8 changes: 8 additions & 0 deletions tests/test_exceptions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(what, py_err_set_after_what);
Skylion007 marked this conversation as resolved.
Show resolved Hide resolved
});
}
49 changes: 49 additions & 0 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -270,3 +270,52 @@ 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


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"