GeezerVison is showing github.com — bpo-36829: Add _PyErr_WriteUnraisableMsg() by vstinner · Pull Request #13488 · python/cpython
bpo-36829: Add _PyErr_WriteUnraisableMsg()
#13488
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
Uh oh!
There was an error while loading. Please reload this page.
Show file tree
Hide file tree
Changes from all commits
Commits
File filter
Filter by extension
.c
(5)
.h
(2)
.py
(1)
.rst
(1)
All 4 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Hide whitespace
Diff view
Hide whitespace
There are no files selected for viewing
Original file line number
Diff line number
Diff line change
Expand Up
@@ -1566,11 +1566,16 @@ always available.
* *exc_type*: Exception type.
* *exc_value*: Exception value, can be ``None``.
* *exc_traceback*: Exception traceback, can be ``None``.
* *err_msg*: Error message, can be ``None``.
* *object*: Object causing the exception, can be ``None``.
:func:`sys.unraisablehook` can be overridden to control how unraisable
exceptions are handled.
The default hook formats *err_msg* and *object* as:
``f'{err_msg}: {object!r}'``; use "Exception ignored in" error message
Comment thread
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 .
``f'{err_msg}: {object!r}' ``; use "Exception ignored in" error message
``f'Exception ignored {err_msg}: {object!r}' ``; use "Exception ignored in" error message
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 .
It really logs f'{err_msg}: {object!r}'. _PyErr_WriteUnraisableMsg("xxx") sets err_msg to "Exception ignored xxx".
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
if *err_msg* is ``None``.
See also :func:`excepthook` which handles uncaught exceptions.
.. versionadded:: 3.8
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -171,6 +171,9 @@ PyAPI_FUNC(PyObject *) _PyUnicodeTranslateError_Create(
const char *reason /* UTF-8 encoded string */
);
PyAPI_FUNC(void) _PyErr_WriteUnraisableMsg(
const char *err_msg,
PyObject *obj);
#ifdef __cplusplus
}
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -878,31 +878,38 @@ def test__enablelegacywindowsfsencoding(self):
@test.support.cpython_only
class UnraisableHookTest(unittest.TestCase):
def write_unraisable_exc(self, exc, obj):
def write_unraisable_exc(self, exc, err_msg, obj):
import _testcapi
import types
err_msg2 = f"Exception ignored {err_msg}"
try:
_testcapi.write_unraisable_exc(exc, obj)
_testcapi.write_unraisable_exc(exc, err_msg, obj)
return types.SimpleNamespace(exc_type=type(exc),
exc_value=exc,
exc_traceback=exc.__traceback__,
err_msg=err_msg2,
object=obj)
finally:
# Explicitly break any reference cycle
exc = None
def test_original_unraisablehook(self):
obj = "an object"
with test.support.captured_output("stderr") as stderr:
with test.support.swap_attr(sys, 'unraisablehook',
sys.__unraisablehook__):
self.write_unraisable_exc(ValueError(42), obj)
err = stderr.getvalue()
self.assertIn(f'Exception ignored in: {obj!r}\n', err)
self.assertIn('Traceback (most recent call last):\n', err)
self.assertIn('ValueError: 42\n', err)
for err_msg in (None, "original hook"):
with self.subTest(err_msg=err_msg):
obj = "an object"
with test.support.captured_output("stderr") as stderr:
with test.support.swap_attr(sys, 'unraisablehook',
sys.__unraisablehook__):
self.write_unraisable_exc(ValueError(42), err_msg, obj)
err = stderr.getvalue()
if err_msg is not None:
self.assertIn(f'Exception ignored {err_msg}: {obj!r}\n', err)
else:
self.assertIn(f'Exception ignored in: {obj!r}\n', err)
self.assertIn('Traceback (most recent call last):\n', err)
self.assertIn('ValueError: 42\n', err)
def test_original_unraisablehook_err(self):
# bpo-22836: PyErr_WriteUnraisable() should give sensible reports
Expand Down
Expand Up
@@ -962,8 +969,9 @@ def hook_func(args):
obj = object()
try:
with test.support.swap_attr(sys, 'unraisablehook', hook_func):
expected = self.write_unraisable_exc(ValueError(42), obj)
for attr in "exc_type exc_value exc_traceback object".split():
expected = self.write_unraisable_exc(ValueError(42),
"custom hook", obj)
for attr in "exc_type exc_value exc_traceback err_msg object".split():
self.assertEqual(getattr(hook_args, attr),
getattr(expected, attr),
(hook_args, expected))
Expand All
@@ -978,10 +986,12 @@ def hook_func(*args):
with test.support.captured_output("stderr") as stderr:
with test.support.swap_attr(sys, 'unraisablehook', hook_func):
self.write_unraisable_exc(ValueError(42), None)
self.write_unraisable_exc(ValueError(42),
"custom hook fail", None)
err = stderr.getvalue()
self.assertIn(f'Exception ignored in: {hook_func!r}\n',
self.assertIn(f'Exception ignored in sys.unraisablehook: '
f'{hook_func!r}\n',
err)
self.assertIn('Traceback (most recent call last):\n', err)
self.assertIn('Exception: hook_func failed\n', err)
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -150,9 +150,9 @@ _DictRemover_call(PyObject *myself, PyObject *args, PyObject *kw)
{
DictRemoverObject *self = (DictRemoverObject *)myself;
if (self->key && self->dict) {
if (-1 == PyDict_DelItem(self->dict, self->key))
/* XXX Error context */
PyErr_WriteUnraisable(Py_None);
if (-1 == PyDict_DelItem(self->dict, self->key)) {
_PyErr_WriteUnraisableMsg("on calling _ctypes.DictRemover", NULL);
}
Py_CLEAR(self->key);
Py_CLEAR(self->dict);
}
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -4985,13 +4985,24 @@ negative_refcount(PyObject *self, PyObject *Py_UNUSED(args))
static PyObject*
test_write_unraisable_exc(PyObject *self, PyObject *args)
{
PyObject *exc, *obj;
if (!PyArg_ParseTuple(args, "OO ", &exc, &obj)) {
PyObject *exc, *err_msg, * obj;
if (!PyArg_ParseTuple(args, "OOO ", &exc, &err_msg , &obj)) {
return NULL;
}
const char *err_msg_utf8;
if (err_msg != Py_None) {
err_msg_utf8 = PyUnicode_AsUTF8(err_msg);
if (err_msg_utf8 == NULL) {
return NULL;
}
}
else {
err_msg_utf8 = NULL;
}
PyErr_SetObject((PyObject *)Py_TYPE(exc), exc);
PyErr_WriteUnraisable( obj);
_PyErr_WriteUnraisableMsg(err_msg_utf8, obj);
Py_RETURN_NONE;
}
Expand Down
Original file line number
Diff line number
Diff line change
Expand Up
@@ -929,9 +929,8 @@ delete_garbage(struct _gc_runtime_state *state,
Py_INCREF(op);
(void) clear(op);
if (PyErr_Occurred()) {
PySys_WriteStderr("Exception ignored in tp_clear of "
"%.50s\n", Py_TYPE(op)->tp_name);
PyErr_WriteUnraisable(NULL);
_PyErr_WriteUnraisableMsg("in tp_clear of",
(PyObject*)Py_TYPE(op));
}
Py_DECREF(op);
}
Expand Down
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Original file line number
Diff line number
Diff line change
Expand Up
@@ -1077,6 +1077,7 @@ static PyStructSequence_Field UnraisableHookArgs_fields[] = {
{"exc_type", "Exception type"},
{"exc_value", "Exception value"},
{"exc_traceback", "Exception traceback"},
{"err_msg", "Error message"},
{"object", "Object causing the exception"},
{0}
};
Expand All
@@ -1085,7 +1086,7 @@ static PyStructSequence_Desc UnraisableHookArgs_desc = {
.name = "UnraisableHookArgs",
.doc = UnraisableHookArgs__doc__,
.fields = UnraisableHookArgs_fields,
.n_in_sequence = 4
.n_in_sequence = 5
};
Expand All
@@ -1104,7 +1105,8 @@ _PyErr_Init(void)
static PyObject *
make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type,
PyObject *exc_value, PyObject *exc_tb, PyObject *obj)
PyObject *exc_value, PyObject *exc_tb,
PyObject *err_msg, PyObject *obj)
{
PyObject *args = PyStructSequence_New(&UnraisableHookArgsType);
if (args == NULL) {
Expand All
@@ -1125,6 +1127,7 @@ make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type,
ADD_ITEM(exc_type);
ADD_ITEM(exc_value);
ADD_ITEM(exc_tb);
ADD_ITEM(err_msg);
ADD_ITEM(obj);
#undef ADD_ITEM
Expand All
@@ -1145,11 +1148,21 @@ make_unraisable_hook_args(PyThreadState *tstate, PyObject *exc_type,
static int
write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type,
PyObject *exc_value, PyObject *exc_tb,
PyObject *obj, PyObject *file)
PyObject *err_msg, PyObject * obj, PyObject *file)
{
if (obj != NULL && obj != Py_None) {
if (PyFile_WriteString("Exception ignored in: ", file) < 0) {
return -1;
if (err_msg != NULL && err_msg != Py_None) {
if (PyFile_WriteObject(err_msg, file, Py_PRINT_RAW) < 0) {
return -1;
}
if (PyFile_WriteString(": ", file) < 0) {
return -1;
}
}
else {
if (PyFile_WriteString("Exception ignored in: ", file) < 0) {
return -1;
}
}
if (PyFile_WriteObject(obj, file, 0) < 0) {
Expand All
@@ -1162,6 +1175,14 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type,
return -1;
}
}
else if (err_msg != NULL && err_msg != Py_None) {
Comment thread
Outdated
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 .
I think you can avoid duplication of the code.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 .
I prefer to have a single PyFile_WriteString(":\n", file) call in this code path, rather than trying to factorize the code too much, to reduce the risk of failure in the hook.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
if (PyFile_WriteObject(err_msg, file, Py_PRINT_RAW) < 0) {
return -1;
}
if (PyFile_WriteString(":\n", file) < 0) {
return -1;
}
}
if (exc_tb != NULL && exc_tb != Py_None) {
if (PyTraceBack_Print(exc_tb, file) < 0) {
Expand All
@@ -1178,8 +1199,9 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type,
const char *className = PyExceptionClass_Name(exc_type);
if (className != NULL) {
const char *dot = strrchr(className, '.');
if (dot != NULL)
if (dot != NULL) {
className = dot+1;
}
}
_Py_IDENTIFIER(__module__);
Expand Down
Expand Up
@@ -1238,7 +1260,8 @@ write_unraisable_exc_file(PyThreadState *tstate, PyObject *exc_type,
static int
write_unraisable_exc(PyThreadState *tstate, PyObject *exc_type,
PyObject *exc_value, PyObject *exc_tb, PyObject *obj)
PyObject *exc_value, PyObject *exc_tb, PyObject *err_msg,
PyObject *obj)
{
PyObject *file = _PySys_GetObjectId(&PyId_stderr);
if (file == NULL || file == Py_None) {
Expand All
@@ -1249,7 +1272,7 @@ write_unraisable_exc(PyThreadState *tstate, PyObject *exc_type,
while we use it */
Py_INCREF(file);
int res = write_unraisable_exc_file(tstate, exc_type, exc_value, exc_tb,
obj, file);
err_msg, obj, file);
Py_DECREF(file);
return res;
Expand All
@@ -1272,9 +1295,10 @@ _PyErr_WriteUnraisableDefaultHook(PyObject *args)
PyObject *exc_type = PyStructSequence_GET_ITEM(args, 0);
PyObject *exc_value = PyStructSequence_GET_ITEM(args, 1);
PyObject *exc_tb = PyStructSequence_GET_ITEM(args, 2);
PyObject *obj = PyStructSequence_GET_ITEM(args, 3);
PyObject *err_msg = PyStructSequence_GET_ITEM(args, 3);
PyObject *obj = PyStructSequence_GET_ITEM(args, 4);
if (write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, obj) < 0) {
if (write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, err_msg, obj) < 0) {
return NULL;
}
Py_RETURN_NONE;
Expand All
@@ -1287,13 +1311,18 @@ _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.
An exception must be set when calling this function. */
void
PyErr_WriteUnraisable( PyObject *obj)
_PyErr_WriteUnraisableMsg(const char *err_msg_str, PyObject *obj)
{
PyThreadState *tstate = _PyThreadState_GET();
assert(tstate != NULL);
PyObject *err_msg = NULL;
PyObject *exc_type, *exc_value, *exc_tb;
_PyErr_Fetch(tstate, &exc_type, &exc_value, &exc_tb);
Expand Down
Expand Up
@@ -1322,13 +1351,20 @@ PyErr_WriteUnraisable(PyObject *obj)
}
}
if (err_msg_str != NULL) {
err_msg = PyUnicode_FromFormat("Exception ignored %s", err_msg_str);
Comment thread
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 .
Hummm, why don't allowing the caller to control the full error message? (Instead of prepending "Exception ignored")
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 .
@serhiy-storchaka asked me to do so :-) And when I started a draft change to convert existing PyErr_WriteUnraisable() to _PyErr_WriteUnraisableMsg(), I always started error messages with "Exception ignored ".
The hook allows to control the full error message. Only proposed _PyErr_WriteUnraisableMsg() enfoce "Exception ignored " prefix. We might add a new function later if needed to let control the full error message.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
vstinner
May 26, 2019
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
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 .
By the way, making "Exception ignored " prefix part of the API should reduce the size of the Python executable binary since constant strings are shorter (remove common "Exception ignored " prefix ;-)).
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
if (err_msg == NULL) {
PyErr_Clear();
}
}
_Py_IDENTIFIER(unraisablehook);
PyObject *hook = _PySys_GetObjectId(&PyId_unraisablehook);
if (hook != NULL && hook != Py_None) {
PyObject *hook_args;
hook_args = make_unraisable_hook_args(tstate, exc_type, exc_value,
exc_tb, obj);
exc_tb, err_msg, obj);
if (hook_args != NULL) {
PyObject *args[1] = {hook_args};
PyObject *res = _PyObject_FastCall(hook, args, 1);
Expand All
@@ -1337,6 +1373,18 @@ PyErr_WriteUnraisable(PyObject *obj)
Py_DECREF(res);
goto done;
}
err_msg_str = "Exception ignored in sys.unraisablehook";
}
else {
err_msg_str = ("Exception ignored on building "
"sys.unraisablehook arguments");
}
Py_XDECREF(err_msg);
err_msg = PyUnicode_FromString(err_msg_str);
if (err_msg == NULL) {
PyErr_Clear();
}
/* sys.unraisablehook failed: log its error using default hook */
Expand All
@@ -1350,15 +1398,25 @@ PyErr_WriteUnraisable(PyObject *obj)
default_hook:
/* Call the default unraisable hook (ignore failure) */
(void)write_unraisable_exc(tstate, exc_type, exc_value, exc_tb, obj);
(void)write_unraisable_exc(tstate, exc_type, exc_value, exc_tb,
err_msg, obj);
done:
Py_XDECREF(exc_type);
Py_XDECREF(exc_value);
Py_XDECREF(exc_tb);
Py_XDECREF(err_msg);
_PyErr_Clear(tstate); /* Just in case */
}
void
PyErr_WriteUnraisable(PyObject *obj)
{
_PyErr_WriteUnraisableMsg(NULL, obj);
}
extern PyObject *PyModule_GetWarningsModule(void);
Expand Down
Loading
Oops, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.