Skip to content

Commit ce9bbc0

Browse files
authored
Python 3.11+: Add __notes__ to error_already_set::what() output. (#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 6081628. * Add general comment explaining why the `error_fetch_and_normalize` code is so unusual.
1 parent 19816f0 commit ce9bbc0

File tree

2 files changed

+90
-16
lines changed

2 files changed

+90
-16
lines changed

include/pybind11/pytypes.h

+62-8
Original file line numberDiff line numberDiff line change
@@ -471,13 +471,24 @@ inline const char *obj_class_name(PyObject *obj) {
471471

472472
std::string error_string();
473473

474+
// The code in this struct is very unusual, to minimize the chances of
475+
// masking bugs (elsewhere) by errors during the error handling (here).
476+
// This is meant to be a lifeline for troubleshooting long-running processes
477+
// that crash under conditions that are virtually impossible to reproduce.
478+
// Low-level implementation alternatives are preferred to higher-level ones
479+
// that might raise cascading exceptions. Last-ditch-kind-of attempts are made
480+
// to report as much of the original error as possible, even if there are
481+
// secondary issues obtaining some of the details.
474482
struct error_fetch_and_normalize {
475-
// Immediate normalization is long-established behavior (starting with
476-
// https://github.com/pybind/pybind11/commit/135ba8deafb8bf64a15b24d1513899eb600e2011
477-
// from Sep 2016) and safest. Normalization could be deferred, but this could mask
478-
// errors elsewhere, the performance gain is very minor in typical situations
479-
// (usually the dominant bottleneck is EH unwinding), and the implementation here
480-
// would be more complex.
483+
// This comment only applies to Python <= 3.11:
484+
// Immediate normalization is long-established behavior (starting with
485+
// https://github.com/pybind/pybind11/commit/135ba8deafb8bf64a15b24d1513899eb600e2011
486+
// from Sep 2016) and safest. Normalization could be deferred, but this could mask
487+
// errors elsewhere, the performance gain is very minor in typical situations
488+
// (usually the dominant bottleneck is EH unwinding), and the implementation here
489+
// would be more complex.
490+
// Starting with Python 3.12, PyErr_Fetch() normalizes exceptions immediately.
491+
// Any errors during normalization are tracked under __notes__.
481492
explicit error_fetch_and_normalize(const char *called) {
482493
PyErr_Fetch(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
483494
if (!m_type) {
@@ -492,6 +503,14 @@ struct error_fetch_and_normalize {
492503
"of the original active exception type.");
493504
}
494505
m_lazy_error_string = exc_type_name_orig;
506+
#if PY_VERSION_HEX >= 0x030C0000
507+
// The presence of __notes__ is likely due to exception normalization
508+
// errors, although that is not necessarily true, therefore insert a
509+
// hint only:
510+
if (PyObject_HasAttrString(m_value.ptr(), "__notes__")) {
511+
m_lazy_error_string += "[WITH __notes__]";
512+
}
513+
#else
495514
// PyErr_NormalizeException() may change the exception type if there are cascading
496515
// failures. This can potentially be extremely confusing.
497516
PyErr_NormalizeException(&m_type.ptr(), &m_value.ptr(), &m_trace.ptr());
@@ -506,12 +525,12 @@ struct error_fetch_and_normalize {
506525
+ " failed to obtain the name "
507526
"of the normalized active exception type.");
508527
}
509-
#if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00
528+
# if defined(PYPY_VERSION_NUM) && PYPY_VERSION_NUM < 0x07030a00
510529
// This behavior runs the risk of masking errors in the error handling, but avoids a
511530
// conflict with PyPy, which relies on the normalization here to change OSError to
512531
// FileNotFoundError (https://github.com/pybind/pybind11/issues/4075).
513532
m_lazy_error_string = exc_type_name_norm;
514-
#else
533+
# else
515534
if (exc_type_name_norm != m_lazy_error_string) {
516535
std::string msg = std::string(called)
517536
+ ": MISMATCH of original and normalized "
@@ -523,6 +542,7 @@ struct error_fetch_and_normalize {
523542
msg += ": " + format_value_and_trace();
524543
pybind11_fail(msg);
525544
}
545+
# endif
526546
#endif
527547
}
528548

@@ -558,6 +578,40 @@ struct error_fetch_and_normalize {
558578
}
559579
}
560580
}
581+
#if PY_VERSION_HEX >= 0x030B0000
582+
auto notes
583+
= reinterpret_steal<object>(PyObject_GetAttrString(m_value.ptr(), "__notes__"));
584+
if (!notes) {
585+
PyErr_Clear(); // No notes is good news.
586+
} else {
587+
auto len_notes = PyList_Size(notes.ptr());
588+
if (len_notes < 0) {
589+
result += "\nFAILURE obtaining len(__notes__): " + detail::error_string();
590+
} else {
591+
result += "\n__notes__ (len=" + std::to_string(len_notes) + "):";
592+
for (ssize_t i = 0; i < len_notes; i++) {
593+
PyObject *note = PyList_GET_ITEM(notes.ptr(), i);
594+
auto note_bytes = reinterpret_steal<object>(
595+
PyUnicode_AsEncodedString(note, "utf-8", "backslashreplace"));
596+
if (!note_bytes) {
597+
result += "\nFAILURE obtaining __notes__[" + std::to_string(i)
598+
+ "]: " + detail::error_string();
599+
} else {
600+
char *buffer = nullptr;
601+
Py_ssize_t length = 0;
602+
if (PyBytes_AsStringAndSize(note_bytes.ptr(), &buffer, &length)
603+
== -1) {
604+
result += "\nFAILURE formatting __notes__[" + std::to_string(i)
605+
+ "]: " + detail::error_string();
606+
} else {
607+
result += '\n';
608+
result += std::string(buffer, static_cast<std::size_t>(length));
609+
}
610+
}
611+
}
612+
}
613+
}
614+
#endif
561615
} else {
562616
result = "<MESSAGE UNAVAILABLE>";
563617
}

tests/test_exceptions.py

+28-8
Original file line numberDiff line numberDiff line change
@@ -317,13 +317,7 @@ def test_error_already_set_what_with_happy_exceptions(
317317
assert what == expected_what
318318

319319

320-
@pytest.mark.skipif(
321-
# Intentionally very specific:
322-
"sys.version_info == (3, 12, 0, 'alpha', 7)",
323-
reason="WIP: https://github.com/python/cpython/issues/102594",
324-
)
325-
@pytest.mark.skipif("env.PYPY", reason="PyErr_NormalizeException Segmentation fault")
326-
def test_flaky_exception_failure_point_init():
320+
def _test_flaky_exception_failure_point_init_before_py_3_12():
327321
with pytest.raises(RuntimeError) as excinfo:
328322
m.error_already_set_what(FlakyException, ("failure_point_init",))
329323
lines = str(excinfo.value).splitlines()
@@ -337,7 +331,33 @@ def test_flaky_exception_failure_point_init():
337331
# Checking the first two lines of the traceback as formatted in error_string():
338332
assert "test_exceptions.py(" in lines[3]
339333
assert lines[3].endswith("): __init__")
340-
assert lines[4].endswith("): test_flaky_exception_failure_point_init")
334+
assert lines[4].endswith(
335+
"): _test_flaky_exception_failure_point_init_before_py_3_12"
336+
)
337+
338+
339+
def _test_flaky_exception_failure_point_init_py_3_12():
340+
# Behavior change in Python 3.12: https://github.com/python/cpython/issues/102594
341+
what, py_err_set_after_what = m.error_already_set_what(
342+
FlakyException, ("failure_point_init",)
343+
)
344+
assert not py_err_set_after_what
345+
lines = what.splitlines()
346+
assert lines[0].endswith("ValueError[WITH __notes__]: triggered_failure_point_init")
347+
assert lines[1] == "__notes__ (len=1):"
348+
assert "Normalization failed:" in lines[2]
349+
assert "FlakyException" in lines[2]
350+
351+
352+
@pytest.mark.skipif(
353+
"env.PYPY and sys.version_info[:2] < (3, 12)",
354+
reason="PyErr_NormalizeException Segmentation fault",
355+
)
356+
def test_flaky_exception_failure_point_init():
357+
if sys.version_info[:2] < (3, 12):
358+
_test_flaky_exception_failure_point_init_before_py_3_12()
359+
else:
360+
_test_flaky_exception_failure_point_init_py_3_12()
341361

342362

343363
def test_flaky_exception_failure_point_str():

0 commit comments

Comments
 (0)