-
-
Notifications
You must be signed in to change notification settings - Fork 31.9k
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
Changes from all commits
de02658
20a11b3
bceb8c5
bda2510
38b5ba2
b7d68e7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 :-( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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): | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add :c:func:`PyErr_FormatUnraisable` function. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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); | ||
|
||
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 Maybe There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is the same as in |
||
} | ||
|
@@ -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); | ||
} | ||
|
||
|
||
|
There was a problem hiding this comment.
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:
And without
an exceptionan object, it should be:Do the format must end with a newline character?
What's the usage of this mode? Is the caller expected to log something like "Exception ignored in xxx" manually?
There was a problem hiding this comment.
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 asPyErr_WriteUnraisable(NULL)
. It just prints the traceback, without "Exception ignored ...".I simply documented the existing behavior for
PyErr_WriteUnraisable()
, and forPyErr_FormatUnraisable(NULL)
the most natural way is to do the same. What alternative do you propose?There was a problem hiding this comment.
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: