From c838af3e2a06304aa07a3eb446896838ce238817 Mon Sep 17 00:00:00 2001 From: Aaron Jomy Date: Tue, 21 Apr 2026 11:17:00 +0200 Subject: [PATCH] [cppyy] Preserve C++ docstring when uninformative PyExceptions/no traceback This follows up on https://github.com/root-project/root/pull/18163 which broke test `test_fragile:test10_documentation` on the compiler-research forks since we lose a C++ side error. This patch resolves the issue by preserving C++ errors when that information can be actually useful, either when the PyException is uninformative, or there is no Python traceback. We completely preserve the intended behaviour of the original commit: ```py import cppyy def inner_func(x): raise ValueError("hello") def func(x): return inner_func(x) cpp_wrapper = cppyy.gbl.std.function["double(double)"](func) cpp_wrapper(3) ``` resulting in something like: ``` Traceback (most recent call last): File "/home/ajomy/cppyy-interop-dev/CppInterOp/cppyy/test/test_err.py", line 11, in cpp_wrapper(3) File "/home/ajomy/cppyy-interop-dev/CppInterOp/cppyy/test/test_err.py", line 7, in func return inner_func(x) ^^^^^^^^^^^^^ File "/home/ajomy/cppyy-interop-dev/CppInterOp/cppyy/test/test_err.py", line 4, in inner_func raise ValueError("hello") ValueError: hello ``` instead of also appending `ValueError: double std::function::operator()(double __args) => ValueError: hello` to the err message But also retaining the C++ docstring context in cases as seen in `test_fragile:test10_documentation` where we expect the docstring when calling the following overload: ```cpp class D { public: virtual int check() { return (int)'D'; } virtual int check(int, int) { return (int)'D'; } void overload() {} void overload(no_such_class*) {} void overload(char, int i = 0) {} // Reflex requires a named arg void overload(int, no_such_class* p = 0) {} }; ``` as `d.overload(None)`: ``` assert "void fragile::D::overload(char, int i = 0)".replace(" ", "") in err_msg ``` --- .../pyroot/cppyy/CPyCppyy/src/CPPMethod.cxx | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/bindings/pyroot/cppyy/CPyCppyy/src/CPPMethod.cxx b/bindings/pyroot/cppyy/CPyCppyy/src/CPPMethod.cxx index df1dbef0567a4..3ea6e6a8734b0 100644 --- a/bindings/pyroot/cppyy/CPyCppyy/src/CPPMethod.cxx +++ b/bindings/pyroot/cppyy/CPyCppyy/src/CPPMethod.cxx @@ -282,8 +282,10 @@ void CPyCppyy::CPPMethod::SetPyError_(PyObject* msg) // C++ method to give some context. // 2. A C++ exception has occurred: // Augment the exception message with the docstring of this method -// 3. A Python exception has occurred: +// 3. A Python exception has occurred with a traceback: // Do nothing, Python exceptions are already informative enough +// 4. If the Python exception has no traceback hinting to an internally set error stack, +// extract its message and wrap it with C++ method docstring context. #if PY_VERSION_HEX >= 0x030c0000 PyObject *evalue = PyErr_Occurred() ? PyErr_GetRaisedException() : nullptr; @@ -299,16 +301,32 @@ void CPyCppyy::CPPMethod::SetPyError_(PyObject* msg) #endif const bool isCppExc = evalue && PyType_IsSubtype((PyTypeObject*)etype, &CPPExcInstance_Type); - // If the error is not a CPPExcInstance, the error from Python itself is - // already complete and messing with it would only make it less informative. + std::string details; + + // If the error is not a CPPExcInstance and has a traceback, the error from + // Python itself is already complete and messing with it would only make it + // less informative. // Just restore and return. if (evalue && !isCppExc) { #if PY_VERSION_HEX >= 0x030c0000 - PyErr_SetRaisedException(evalue); + PyObject* tb = PyException_GetTraceback(evalue); + if (tb) { + Py_DECREF(tb); + PyErr_SetRaisedException(evalue); + return; + } #else - PyErr_Restore(etype, evalue, etrace); + if (etrace) { + PyErr_Restore(etype, evalue, etrace); + return; + } #endif - return; + // no traceback, extract its message and fall through + PyObject* descr = PyObject_Str(evalue); + if (descr) { + details = CPyCppyy_PyText_AsString(descr); + Py_DECREF(descr); + } } PyObject* doc = GetDocString(); @@ -319,9 +337,15 @@ void CPyCppyy::CPPMethod::SetPyError_(PyObject* msg) const char* cname = pyname ? CPyCppyy_PyText_AsString(pyname) : "Exception"; if (!isCppExc) { - // this is the case where no Python error has occurred yet, and we set a new - // error with context info - PyErr_Format(errtype, "%s =>\n %s: %s", cdoc, cname, cmsg ? cmsg : ""); + // this is the case where no Python error has occured yet, or an internal + // one without traceback set a new error with context + if (details.empty()) { + PyErr_Format(errtype, "%s =>\n %s: %s", cdoc, cname, cmsg ? cmsg : ""); + } else if (cmsg) { + PyErr_Format(errtype, "%s =>\n %s: %s (%s)", cdoc, cname, cmsg, details.c_str()); + } else { + PyErr_Format(errtype, "%s =>\n %s: %s", cdoc, cname, details.c_str()); + } } else { // augment the top message with context information PyObject *&topMessage = ((CPPExcInstance*)evalue)->fTopMessage;