From 51d693c58454a2c525094a7c74ebac86859353fd Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Thu, 16 Mar 2023 10:16:01 +0000 Subject: [PATCH] gh-102594: PyErr_SetObject adds note to exception raised on normalization error (#102675) --- Include/cpython/pyerrors.h | 4 ++ Lib/test/test_capi/test_exceptions.py | 20 ++++++++++ ...-03-14-00-11-46.gh-issue-102594.BjU-m2.rst | 1 + Modules/_testcapi/exceptions.c | 21 ++++++++++ Objects/exceptions.c | 15 +++++++ Python/errors.c | 40 ++++++++++++++++--- 6 files changed, 96 insertions(+), 5 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h index 0d9cc9922f7368..d0300f6ee56a25 100644 --- a/Include/cpython/pyerrors.h +++ b/Include/cpython/pyerrors.h @@ -112,6 +112,10 @@ PyAPI_FUNC(PyObject *) _PyErr_FormatFromCause( /* In exceptions.c */ +PyAPI_FUNC(int) _PyException_AddNote( + PyObject *exc, + PyObject *note); + /* Helper that attempts to replace the current exception with one of the * same type but with a prefix added to the exception text. The resulting * exception description looks like: diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py index 55f131699a2567..b1c1a61e20685e 100644 --- a/Lib/test/test_capi/test_exceptions.py +++ b/Lib/test/test_capi/test_exceptions.py @@ -169,5 +169,25 @@ class Broken(Exception, metaclass=Meta): with self.assertRaises(ZeroDivisionError) as e: _testcapi.exc_set_object(Broken, Broken()) + def test_set_object_and_fetch(self): + class Broken(Exception): + def __init__(self, *arg): + raise ValueError("Broken __init__") + + exc = _testcapi.exc_set_object_fetch(Broken, 'abcd') + self.assertIsInstance(exc, ValueError) + self.assertEqual(exc.__notes__[0], + "Normalization failed: type=Broken args='abcd'") + + class BadArg: + def __repr__(self): + raise TypeError('Broken arg type') + + exc = _testcapi.exc_set_object_fetch(Broken, BadArg()) + self.assertIsInstance(exc, ValueError) + self.assertEqual(exc.__notes__[0], + 'Normalization failed: type=Broken args=') + + if __name__ == "__main__": unittest.main() diff --git a/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst b/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst new file mode 100644 index 00000000000000..0b95b5ec98e811 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2023-03-14-00-11-46.gh-issue-102594.BjU-m2.rst @@ -0,0 +1 @@ +Add note to exception raised in ``PyErr_SetObject`` when normalization fails. diff --git a/Modules/_testcapi/exceptions.c b/Modules/_testcapi/exceptions.c index a0575213987ffc..c64b823663c32f 100644 --- a/Modules/_testcapi/exceptions.c +++ b/Modules/_testcapi/exceptions.c @@ -92,6 +92,26 @@ exc_set_object(PyObject *self, PyObject *args) return NULL; } +static PyObject * +exc_set_object_fetch(PyObject *self, PyObject *args) +{ + PyObject *exc; + PyObject *obj; + PyObject *type; + PyObject *value; + PyObject *tb; + + if (!PyArg_ParseTuple(args, "OO:exc_set_object", &exc, &obj)) { + return NULL; + } + + PyErr_SetObject(exc, obj); + PyErr_Fetch(&type, &value, &tb); + Py_XDECREF(type); + Py_XDECREF(tb); + return value; +} + static PyObject * raise_exception(PyObject *self, PyObject *args) { @@ -262,6 +282,7 @@ static PyMethodDef test_methods[] = { {"make_exception_with_doc", _PyCFunction_CAST(make_exception_with_doc), METH_VARARGS | METH_KEYWORDS}, {"exc_set_object", exc_set_object, METH_VARARGS}, + {"exc_set_object_fetch", exc_set_object_fetch, METH_VARARGS}, {"raise_exception", raise_exception, METH_VARARGS}, {"raise_memoryerror", raise_memoryerror, METH_NOARGS}, {"set_exc_info", test_set_exc_info, METH_VARARGS}, diff --git a/Objects/exceptions.c b/Objects/exceptions.c index c6fb6a3f19b2d0..d69f7400ca6042 100644 --- a/Objects/exceptions.c +++ b/Objects/exceptions.c @@ -3749,6 +3749,21 @@ _PyExc_Fini(PyInterpreterState *interp) _PyExc_FiniTypes(interp); } +int +_PyException_AddNote(PyObject *exc, PyObject *note) +{ + if (!PyExceptionInstance_Check(exc)) { + PyErr_Format(PyExc_TypeError, + "exc must be an exception, not '%s'", + Py_TYPE(exc)->tp_name); + return -1; + } + PyObject *r = BaseException_add_note(exc, note); + int res = r == NULL ? -1 : 0; + Py_XDECREF(r); + return res; +} + /* Helper to do the equivalent of "raise X from Y" in C, but always using * the current exception rather than passing one in. * diff --git a/Python/errors.c b/Python/errors.c index bbf6d397ce8097..bdcbac317eb9ee 100644 --- a/Python/errors.c +++ b/Python/errors.c @@ -135,6 +135,28 @@ _PyErr_GetTopmostException(PyThreadState *tstate) return exc_info; } +static PyObject * +get_normalization_failure_note(PyThreadState *tstate, PyObject *exception, PyObject *value) +{ + PyObject *args = PyObject_Repr(value); + if (args == NULL) { + _PyErr_Clear(tstate); + args = PyUnicode_FromFormat(""); + } + PyObject *note; + const char *tpname = ((PyTypeObject*)exception)->tp_name; + if (args == NULL) { + _PyErr_Clear(tstate); + note = PyUnicode_FromFormat("Normalization failed: type=%s", tpname); + } + else { + note = PyUnicode_FromFormat("Normalization failed: type=%s args=%S", + tpname, args); + Py_DECREF(args); + } + return note; +} + void _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) { @@ -160,19 +182,27 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value) Py_XINCREF(value); if (!is_subclass) { /* We must normalize the value right now */ - PyObject *fixed_value; /* Issue #23571: functions must not be called with an exception set */ _PyErr_Clear(tstate); - fixed_value = _PyErr_CreateException(exception, value); - Py_XDECREF(value); + PyObject *fixed_value = _PyErr_CreateException(exception, value); if (fixed_value == NULL) { + PyObject *exc = _PyErr_GetRaisedException(tstate); + assert(PyExceptionInstance_Check(exc)); + + PyObject *note = get_normalization_failure_note(tstate, exception, value); + Py_XDECREF(value); + if (note != NULL) { + /* ignore errors in _PyException_AddNote - they will be overwritten below */ + _PyException_AddNote(exc, note); + Py_DECREF(note); + } + _PyErr_SetRaisedException(tstate, exc); return; } - - value = fixed_value; + Py_XSETREF(value, fixed_value); } exc_value = _PyErr_GetTopmostException(tstate)->exc_value;