Skip to content

gh-108082: Add PyErr_FormatUnraisable() function #111086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions Doc/c-api/exceptions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,26 @@ Printing and clearing
Use :func:`sys.unraisablehook`.


.. c:function:: void PyErr_FormatUnraisable(const char *format, ...)

Similar to :c:func:`PyErr_WriteUnraisable`, but the *format* and subsequent
parameters help format the warning message; they have the same meaning and
values as in :c:func:`PyUnicode_FromFormat`.
``PyErr_WriteUnraisable(obj)`` is roughtly equivalent to
``PyErr_FormatUnraisable("Exception ignored in: %R, obj)``.
If *format* is ``NULL``, only the traceback is printed.
Copy link
Member

@vstinner vstinner Oct 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind to give examples? I'm not sure how to use this API.

I understood that PyErr_WriteUnraisable(obj) can be written as:

PyErr_FormatUnraisable("Exception ignored in: %R", obj);

And without an exception an object, it should be:

PyErr_FormatUnraisable("Exception ignored in:");

Do the format must end with a newline character?

If format is NULL, only the traceback is printed.

What's the usage of this mode? Is the caller expected to log something like "Exception ignored in xxx" manually?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PyErr_FormatUnraisable(NULL) is the same as PyErr_WriteUnraisable(NULL). It just prints the traceback, without "Exception ignored ...".

I simply documented the existing behavior for PyErr_WriteUnraisable(), and for PyErr_FormatUnraisable(NULL) the most natural way is to do the same. What alternative do you propose?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, my remark is not specific to NULL value, but more about the whole function. I don't know which string to pass to the function. I suggest to add an example to the doc. Like:

For example, ``PyErr_WriteUnraisable(obj)`` is similar to ``PyErr_FormatUnraisable("Exception ignored in: %R", obj);``.


.. versionadded:: 3.13


.. c:function:: void PyErr_DisplayException(PyObject *exc)

Print the standard traceback display of ``exc`` to ``sys.stderr``, including
chained exceptions and notes.

.. versionadded:: 3.12


Raising exceptions
==================

Expand Down
4 changes: 4 additions & 0 deletions Doc/whatsnew/3.13.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1107,6 +1107,10 @@ New Features
limited C API.
(Contributed by Victor Stinner in :gh:`85283`.)

* Add :c:func:`PyErr_FormatUnraisable` function: similar to
:c:func:`PyErr_WriteUnraisable`, but allow to customize the warning mesage.
(Contributed by Serhiy Storchaka in :gh:`108082`.)

* Add :c:func:`PyUnicode_AsUTF8` function to the limited C API.
(Contributed by Victor Stinner in :gh:`111089`.)

Expand Down
2 changes: 2 additions & 0 deletions Include/cpython/pyerrors.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,4 +120,6 @@ PyAPI_FUNC(void) _Py_NO_RETURN _Py_FatalErrorFunc(
const char *func,
const char *message);

PyAPI_FUNC(void) PyErr_FormatUnraisable(const char *, ...);

#define Py_FatalError(message) _Py_FatalErrorFunc(__func__, (message))
57 changes: 57 additions & 0 deletions Lib/test/test_capi/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,63 @@ def test_err_writeunraisable(self):
# CRASHES writeunraisable(NULL, hex)
# CRASHES writeunraisable(NULL, NULL)

def test_err_formatunraisable(self):
# Test PyErr_FormatUnraisable()
formatunraisable = _testcapi.err_formatunraisable
firstline = self.test_err_formatunraisable.__code__.co_firstlineno

with support.catch_unraisable_exception() as cm:
formatunraisable(CustomError('oops!'), b'Error in %R', [])
self.assertEqual(cm.unraisable.exc_type, CustomError)
self.assertEqual(str(cm.unraisable.exc_value), 'oops!')
self.assertEqual(cm.unraisable.exc_traceback.tb_lineno,
firstline + 6)
self.assertEqual(cm.unraisable.err_msg, 'Error in []')
self.assertIsNone(cm.unraisable.object)

with support.catch_unraisable_exception() as cm:
formatunraisable(CustomError('oops!'), b'undecodable \xff')
self.assertEqual(cm.unraisable.exc_type, CustomError)
self.assertEqual(str(cm.unraisable.exc_value), 'oops!')
self.assertEqual(cm.unraisable.exc_traceback.tb_lineno,
firstline + 15)
self.assertIsNone(cm.unraisable.err_msg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to log an error in the error handler in this case. Ignoring silently errors doesn't help :-(

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_PyErr_WriteUnraisableMsg() does the same: silently ignores decoding and memory errors.

I minimized changes. Other changes can be made in a separate PR.

self.assertIsNone(cm.unraisable.object)

with support.catch_unraisable_exception() as cm:
formatunraisable(CustomError('oops!'), NULL)
self.assertEqual(cm.unraisable.exc_type, CustomError)
self.assertEqual(str(cm.unraisable.exc_value), 'oops!')
self.assertEqual(cm.unraisable.exc_traceback.tb_lineno,
firstline + 24)
self.assertIsNone(cm.unraisable.err_msg)
self.assertIsNone(cm.unraisable.object)

with (support.swap_attr(sys, 'unraisablehook', None),
support.captured_stderr() as stderr):
formatunraisable(CustomError('oops!'), b'Error in %R', [])
lines = stderr.getvalue().splitlines()
self.assertEqual(lines[0], f'Error in []:')
self.assertEqual(lines[1], 'Traceback (most recent call last):')
self.assertEqual(lines[-1], f'{__name__}.CustomError: oops!')

with (support.swap_attr(sys, 'unraisablehook', None),
support.captured_stderr() as stderr):
formatunraisable(CustomError('oops!'), b'undecodable \xff')
lines = stderr.getvalue().splitlines()
self.assertEqual(lines[0], 'Traceback (most recent call last):')
self.assertEqual(lines[-1], f'{__name__}.CustomError: oops!')

with (support.swap_attr(sys, 'unraisablehook', None),
support.captured_stderr() as stderr):
formatunraisable(CustomError('oops!'), NULL)
lines = stderr.getvalue().splitlines()
self.assertEqual(lines[0], 'Traceback (most recent call last):')
self.assertEqual(lines[-1], f'{__name__}.CustomError: oops!')

# CRASHES formatunraisable(NULL, b'Error in %R', [])
# CRASHES formatunraisable(NULL, NULL)


class Test_PyUnstable_Exc_PrepReraiseStar(ExceptionIsLikeMixin, unittest.TestCase):

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add :c:func:`PyErr_FormatUnraisable` function.
25 changes: 25 additions & 0 deletions Modules/_testcapi/exceptions.c
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,30 @@ err_writeunraisable(PyObject *Py_UNUSED(module), PyObject *args)
Py_RETURN_NONE;
}

static PyObject *
err_formatunraisable(PyObject *Py_UNUSED(module), PyObject *args)
{
PyObject *exc;
const char *fmt;
Py_ssize_t fmtlen;
PyObject *objs[10] = {NULL};

if (!PyArg_ParseTuple(args, "Oz#|OOOOOOOOOO", &exc, &fmt, &fmtlen,
&objs[0], &objs[1], &objs[2], &objs[3], &objs[4],
&objs[5], &objs[6], &objs[7], &objs[8], &objs[9]))
{
return NULL;
}
NULLABLE(exc);
if (exc) {
PyErr_SetRaisedException(Py_NewRef(exc));
}
PyErr_FormatUnraisable(fmt,
objs[0], objs[1], objs[2], objs[3], objs[4],
objs[5], objs[6], objs[7], objs[8], objs[9]);
Py_RETURN_NONE;
}

/*[clinic input]
_testcapi.unstable_exc_prep_reraise_star
orig: object
Expand Down Expand Up @@ -364,6 +388,7 @@ static PyTypeObject PyRecursingInfinitelyError_Type = {
static PyMethodDef test_methods[] = {
{"err_restore", err_restore, METH_VARARGS},
{"err_writeunraisable", err_writeunraisable, METH_VARARGS},
{"err_formatunraisable", err_formatunraisable, METH_VARARGS},
_TESTCAPI_ERR_SET_RAISED_METHODDEF
_TESTCAPI_EXCEPTION_PRINT_METHODDEF
_TESTCAPI_FATAL_ERROR_METHODDEF
Expand Down
48 changes: 40 additions & 8 deletions Python/errors.c
Original file line number Diff line number Diff line change
Expand Up @@ -1569,14 +1569,16 @@ _PyErr_WriteUnraisableDefaultHook(PyObject *args)
for Python to handle it. For example, when a destructor raises an exception
or during garbage collection (gc.collect()).

If err_msg_str is non-NULL, the error message is formatted as:
"Exception ignored %s" % err_msg_str. Otherwise, use "Exception ignored in"
error message.
If format is non-NULL, the error message is formatted using format and
variable arguments as in PyUnicode_FromFormat().
Otherwise, use "Exception ignored in" error message.

An exception must be set when calling this function. */
void
_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)

static void
format_unraisable_v(const char *format, va_list va, PyObject *obj)
{
const char *err_msg_str;
PyThreadState *tstate = _PyThreadState_GET();
_Py_EnsureTstateNotNULL(tstate);

Expand Down Expand Up @@ -1610,8 +1612,8 @@ _PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)
}
}

if (err_msg_str != NULL) {
err_msg = PyUnicode_FromFormat("Exception ignored %s", err_msg_str);
if (format != NULL) {
err_msg = PyUnicode_FromFormatV(format, va);
if (err_msg == NULL) {
PyErr_Clear();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that it's the best behavior. I would prefer to goto error. What do you think?

Maybe PyUnicode_FromString("Exception ignored in sys.unraisablehook") can become a _Py_DECLARE_STR(exc_ignored_msg, "...") + _Py_STR(exc_ignored_msg) static immortal string to avoid error 3 while handling error 2 while handling error 1.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the same as in _PyErr_WriteUnraisableMsg(). Changing the behavior of _PyErr_WriteUnraisableMsg() is out of the scope of this PR.

}
Expand Down Expand Up @@ -1676,11 +1678,41 @@ _PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)
_PyErr_Clear(tstate); /* Just in case */
}

void
PyErr_FormatUnraisable(const char *format, ...)
{
va_list va;

va_start(va, format);
format_unraisable_v(format, va, NULL);
va_end(va);
}

static void
format_unraisable(PyObject *obj, const char *format, ...)
{
va_list va;

va_start(va, format);
format_unraisable_v(format, va, obj);
va_end(va);
}

void
_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)
{
if (err_msg_str) {
format_unraisable(obj, "Exception ignored %s", err_msg_str);
}
else {
format_unraisable(obj, NULL);
}
}

void
PyErr_WriteUnraisable(PyObject *obj)
{
_PyErr_WriteUnraisableMsg(NULL, obj);
format_unraisable(obj, NULL);
}


Expand Down