classification
Title: Tidy up error handling in traceback.c / python run.c
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.11
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: iritkatriel Nosy List: gvanrossum, iritkatriel
Priority: normal Keywords: patch

Created on 2021-10-27 21:27 by iritkatriel, last changed 2021-12-16 23:00 by iritkatriel. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 29905 merged iritkatriel, 2021-12-03 14:04
PR 29981 merged iritkatriel, 2021-12-08 09:51
PR 29996 merged iritkatriel, 2021-12-09 00:27
PR 30015 merged iritkatriel, 2021-12-09 18:48
PR 30073 merged iritkatriel, 2021-12-12 15:47
Messages (7)
msg405126 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-10-27 21:27
They do things like 

    err = PyFile_WriteString("TypeError: print_exception(): Exception expected for value, ", f);
    err += PyFile_WriteString(Py_TYPE(value)->tp_name, f);
    err += PyFile_WriteString(" found\n", f);

which means that PyFile_XXX functions are called after error. They should return (abort printing the exception) instead.


It gets even more interesting with PyFile_WriteObject calls - this function asserts that the exception is not set, so the code calls PyErr_Clear() before calling PyFile_WriteObject(). It should avoid making the call altogether.
msg405431 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-11-01 11:23
It's more intricate than I initially thought - the assertion that no exception is set is in PyObject_Str rather than PyFile_WriteObject, and it should remain there because it ensures that exceptions are not accidentally cleared by an str() call.

In the traceback display code, there are places where the exception is cleared because it's being overridden (like when modulename is not found and is replace by "<unknown>") or places where the code makes a best-effort attempt to print something anyway. In these situations the error is cleared before calling PyFile_WriteObject. It is probably correct, but it's hard to follow the logic when making changes to the code, so it should be made more explicit.
msg407947 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-07 16:17
New changeset d596acbd3b4f6716ed98895eb0b48e9830e0b320 by Irit Katriel in branch 'main':
bpo-45635: standardize error handling in traceback.c (GH-29905)
https://github.com/python/cpython/commit/d596acbd3b4f6716ed98895eb0b48e9830e0b320
msg408040 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-08 18:47
New changeset f893bb2e01307c92ca19597f44874ecaffe7f06f by Irit Katriel in branch 'main':
bpo-45635: refactor print_exception() into smaller functions (GH-29981)
https://github.com/python/cpython/commit/f893bb2e01307c92ca19597f44874ecaffe7f06f
msg408129 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-09 14:38
New changeset dc4a212bd305831cb4b187a2e0cc82666fcb15ca by Irit Katriel in branch 'main':
bpo-45635: continue refactor of print_exception() to standardize error handling (GH-29996)
https://github.com/python/cpython/commit/dc4a212bd305831cb4b187a2e0cc82666fcb15ca
msg408253 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-10 23:02
New changeset 0fe104fce7da709edddb701baa2249e3275db1fd by Irit Katriel in branch 'main':
bpo-45635: refactor print_exception_recursive into smaller functions to standardize error handling (GH-30015)
https://github.com/python/cpython/commit/0fe104fce7da709edddb701baa2249e3275db1fd
msg408741 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2021-12-16 23:00
New changeset 8d6155ee9d1b05946f951d0ba602b9f63810fe0f by Irit Katriel in branch 'main':
bpo-45635: Do not suppress errors in functions called from _PyErr_Display (GH-30073)
https://github.com/python/cpython/commit/8d6155ee9d1b05946f951d0ba602b9f63810fe0f
History
Date User Action Args
2021-12-16 23:00:44iritkatrielsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2021-12-16 23:00:22iritkatrielsetmessages: + msg408741
2021-12-12 15:47:51iritkatrielsetpull_requests: + pull_request28295
2021-12-10 23:02:15iritkatrielsetmessages: + msg408253
2021-12-09 18:48:37iritkatrielsetpull_requests: + pull_request28239
2021-12-09 14:38:09iritkatrielsetmessages: + msg408129
2021-12-09 00:27:54iritkatrielsetpull_requests: + pull_request28219
2021-12-08 18:47:35iritkatrielsetmessages: + msg408040
2021-12-08 09:51:45iritkatrielsetpull_requests: + pull_request28207
2021-12-07 16:17:30iritkatrielsetmessages: + msg407947
2021-12-03 14:04:56iritkatrielsetkeywords: + patch
stage: patch review
pull_requests: + pull_request28129
2021-11-03 22:20:57gvanrossumsetnosy: + gvanrossum
2021-11-01 11:23:33iritkatrielsetmessages: + msg405431
2021-10-27 21:27:24iritkatrielsetcomponents: + Interpreter Core
versions: + Python 3.11
2021-10-27 21:27:06iritkatrielcreate