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 60816285e9.

* Add general comment explaining why the `error_fetch_and_normalize` code is so unusual.

include/pybind11/pytypes.h

@@ -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 {
+    // 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) {
@@ -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());
@@ -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 "
@@ -523,6 +542,7 @@ struct error_fetch_and_normalize {
             msg += ": " + format_value_and_trace();
             pybind11_fail(msg);
         }
+#    endif
 #endif
     }
 
@@ -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>";
         }

tests/test_exceptions.py

@@ -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()
@@ -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():