bpo-36829: Add _PyErr_WriteUnraisableMsg()#13488
bpo-36829: Add _PyErr_WriteUnraisableMsg()#13488vstinner merged 1 commit intopython:masterfrom vstinner:unraisable_msg
Conversation
|
Interesting case: _PyErr_WarnUnawaitedCoroutine(): The unraisable exception is logged with default "Exception ignored in: ..." message. Should we use a customized error message here? In my PR #13490, I have to catch stderr and sys.unraisablehook to catch the warning and the unraisable exception in test_coroutines.test_unawaited_warning_when_module_broken(): |
|
@graingert @pablogsal @serhiy-storchaka: Would you mind to review this PR? I don't recall who asked me to allow to specify a custom error message :-) |
Python/errors.c
Outdated
There was a problem hiding this comment.
I think you can avoid duplication of the code.
There was a problem hiding this comment.
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.
|
I rebased my PR and merged conflicts. |
* sys.unraisablehook: add 'err_msg' field to UnraisableHookArgs. * Use _PyErr_WriteUnraisableMsg() in _ctypes _DictRemover_call() and gc delete_garbage().
|
@serhiy-storchaka: Would you mind to review the update PR? |
| exceptions are handled. | ||
|
|
||
| The default hook formats *err_msg* and *object* as: | ||
| ``f'{err_msg}: {object!r}'``; use "Exception ignored in" error message |
There was a problem hiding this comment.
| ``f'{err_msg}: {object!r}'``; use "Exception ignored in" error message | |
| ``f'Exception ignored {err_msg}: {object!r}'``; use "Exception ignored in" error message |
There was a problem hiding this comment.
It really logs f'{err_msg}: {object!r}'. _PyErr_WriteUnraisableMsg("xxx") sets err_msg to "Exception ignored xxx".
| } | ||
|
|
||
| if (err_msg_str != NULL) { | ||
| err_msg = PyUnicode_FromFormat("Exception ignored %s", err_msg_str); |
There was a problem hiding this comment.
Hummm, why don't allowing the caller to control the full error message? (Instead of prepending "Exception ignored")
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
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 ;-)).
|
@serhiy-storchaka @pablogsal: So do you agree each other on "Exception ignored" prefix? :-) |
Yup, I'm happy with it :) |
* sys.unraisablehook: add 'err_msg' field to UnraisableHookArgs. * Use _PyErr_WriteUnraisableMsg() in _ctypes _DictRemover_call() and gc delete_garbage().
and gc delete_garbage().
https://bugs.python.org/issue36829