Skip to content

Commit

Permalink
Python 3.11+: Add __notes__ to error_already_set::what() output. …
Browse files Browse the repository at this point in the history
…(#4678)

* First version adding `__notes__` to `error_already_set::what()` output.

* Fix trivial oversight (missing adjustment in existing test).

* Minor enhancements of new code.

* Re-enable `cmake --target cpptest -j 2`

* Revert "Re-enable `cmake --target cpptest -j 2`"

This reverts commit 60816285e9e7b95b7d33a60805888b80b2bec641.

* Add general comment explaining why the `error_fetch_and_normalize` code is so unusual.
  • Loading branch information
Ralf W. Grosse-Kunstleve authored May 23, 2023
1 parent 19816f0 commit ce9bbc0
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 16 deletions.
70 changes: 62 additions & 8 deletions include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -471,13 +471,24 @@ inline const char *obj_class_name(PyObject *obj) {

std::string error_string();

// The code in this struct is very unusual, to minimize the chances of
// masking bugs (elsewhere) by errors during the error handling (here).
// This is meant to be a lifeline for troubleshooting long-running processes
// that crash under conditions that are virtually impossible to reproduce.
// Low-level implementation alternatives are preferred to higher-level ones
// that might raise cascading exceptions. Last-ditch-kind-of attempts are made
// to report as much of the original error as possible, even if there are
// secondary issues obtaining some of the details.
struct error_fetch_and_normalize {
// Immediate normalization is long-established behavior (starting with
// https://github.com/pybind/pybind11/commit/135ba8deafb8bf64a15b24d1513899eb600e2011
// from Sep 2016) and safest. Normalization could be deferred, but this could mask
// errors elsewhere, the performance gain is very minor in typical situations
// (usually the dominant bottleneck is EH unwinding), and the implementation here
// would be more complex.
// This comment only applies to Python <= 3.11:
// Immediate normalization is long-established behavior (starting with
// https://github.com/pybind/pybind11/commit/135ba8deafb8bf64a15b24d1513899eb600e2011
// from Sep 2016) and safest. Normalization could be deferred, but this could mask
// errors elsewhere, the performance gain is very minor in typical situations
// (usually the dominant bottleneck is EH unwinding), and the implementation here
// would be more complex.
// Starting with Python 3.12, PyErr_Fetch() normalizes exceptions immediately.
// Any errors during normalization are tracked under __notes__.
explicit error_fetch_and_normalize(const char *called) {
PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
if (!m_type) {
Expand All @@ -492,6 +503,14 @@ struct error_fetch_and_normalize {
"of the original active exception type.");
}
m_lazy_error_string = exc_type_name_orig;
#if PY_VERSION_HEX >= 0x030C0000
// The presence of __notes__ is likely due to exception normalization
// errors, although that is not necessarily true, therefore insert a
// hint only:
if (PyObject_HasAttrString(m_value.ptr(), "__notes__")) {
m_lazy_error_string += "[WITH __notes__]";
}
#else
// PyErr_NormalizeException() may change the exception type if there are cascading
// failures. This can potentially be extremely confusing.
PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
Expand All @@ -506,12 +525,12 @@ struct error_fetch_and_normalize {
+ " failed to obtain the name "
"of the normalized active exception type.");
}
#if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00
# if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00
// This behavior runs the risk of masking errors in the error handling, but avoids a
// conflict with PyPy, which relies on the normalization here to change OSError to
// FileNotFoundError (https://github.com/pybind/pybind11/issues/4075).
m_lazy_error_string = exc_type_name_norm;
#else
# else
if (exc_type_name_norm != m_lazy_error_string) {
std::string msg = std::string(called)
+ ": MISMATCH of original and normalized "
Expand All @@ -523,6 +542,7 @@ struct error_fetch_and_normalize {
msg += ": " + format_value_and_trace();
pybind11_fail(msg);
}
# endif
#endif
}

Expand Down Expand Up @@ -558,6 +578,40 @@ struct error_fetch_and_normalize {
}
}
}
#if PY_VERSION_HEX >= 0x030B0000
auto notes
= reinterpret_steal<object>(PyObject_GetAttrString(m_value.ptr(), "__notes__"));
if (!notes) {
PyErr_Clear(); // No notes is good news.
} else {
auto len_notes = PyList_Size(notes.ptr());
if (len_notes < 0) {
result += "\nFAILURE obtaining len(__notes__): " + detail::error_string();
} else {
result += "\n__notes__ (len=" + std::to_string(len_notes) + "):";
for (ssize_t i = 0; i < len_notes; i++) {
PyObject *note = PyList_GET_ITEM(notes.ptr(), i);
auto note_bytes = reinterpret_steal<object>(
PyUnicode_AsEncodedString(note, "utf-8", "backslashreplace"));
if (!note_bytes) {
result += "\nFAILURE obtaining __notes__[" + std::to_string(i)
+ "]: " + detail::error_string();
} else {
char *buffer = nullptr;
Py_ssize_t length = 0;
if (PyBytes_AsStringAndSize(note_bytes.ptr(), &buffer, &length)
== -1) {
result += "\nFAILURE formatting __notes__[" + std::to_string(i)
+ "]: " + detail::error_string();
} else {
result += '\n';
result += std::string(buffer, static_cast<std::size_t>(length));
}
}
}
}
}
#endif
} else {
result = "<MESSAGE UNAVAILABLE>";
}
Expand Down
36 changes: 28 additions & 8 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -317,13 +317,7 @@ def test_error_already_set_what_with_happy_exceptions(
assert what == expected_what


@pytest.mark.skipif(
# Intentionally very specific:
"sys.version_info == (3, 12, 0, 'alpha', 7)",
reason="WIP: https://github.com/python/cpython/issues/102594",
)
@pytest.mark.skipif("env.PYPY", reason="PyErr_NormalizeException Segmentation fault")
def test_flaky_exception_failure_point_init():
def _test_flaky_exception_failure_point_init_before_py_3_12():
with pytest.raises(RuntimeError) as excinfo:
m.error_already_set_what(FlakyException, ("failure_point_init",))
lines = str(excinfo.value).splitlines()
Expand All @@ -337,7 +331,33 @@ def test_flaky_exception_failure_point_init():
# 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")
assert lines[4].endswith(
"): _test_flaky_exception_failure_point_init_before_py_3_12"
)


def _test_flaky_exception_failure_point_init_py_3_12():
# Behavior change in Python 3.12: https://github.com/python/cpython/issues/102594
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()
assert lines[0].endswith("ValueError[WITH __notes__]: triggered_failure_point_init")
assert lines[1] == "__notes__ (len=1):"
assert "Normalization failed:" in lines[2]
assert "FlakyException" in lines[2]


@pytest.mark.skipif(
"env.PYPY and sys.version_info[:2] < (3, 12)",
reason="PyErr_NormalizeException Segmentation fault",
)
def test_flaky_exception_failure_point_init():
if sys.version_info[:2] < (3, 12):
_test_flaky_exception_failure_point_init_before_py_3_12()
else:
_test_flaky_exception_failure_point_init_py_3_12()


def test_flaky_exception_failure_point_str():
Expand Down

0 comments on commit ce9bbc0

Please sign in to comment.