Tidy up error handling in traceback.c / python run.c

[iritkatriel]

BPO	45635
Nosy	@gvanrossum, @iritkatriel
PRs	bpo-45635: standardize error handling in traceback.c #29905 bpo-45635: refactor print_exception() into smaller functions #29981 bpo-45635: continue refactor of print_exception() to standardize error handling #29996 bpo-45635: refactor print_exception_recursive into smaller functions to standardize error handling #30015 bpo-45635: Do not suppress errors in functions called from _PyErr_Display #30073

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/iritkatriel'
closed_at = <Date 2021-12-16.23:00:44.983>
created_at = <Date 2021-10-27.21:27:06.452>
labels = ['interpreter-core', '3.11']
title = 'Tidy up error handling in traceback.c / python run.c'
updated_at = <Date 2021-12-16.23:00:44.982>
user = 'https://github.com/iritkatriel'

bugs.python.org fields:

activity = <Date 2021-12-16.23:00:44.982>
actor = 'iritkatriel'
assignee = 'iritkatriel'
closed = True
closed_date = <Date 2021-12-16.23:00:44.983>
closer = 'iritkatriel'
components = ['Interpreter Core']
creation = <Date 2021-10-27.21:27:06.452>
creator = 'iritkatriel'
dependencies = []
files = []
hgrepos = []
issue_num = 45635
keywords = ['patch']
message_count = 7.0
messages = ['405126', '405431', '407947', '408040', '408129', '408253', '408741']
nosy_count = 2.0
nosy_names = ['gvanrossum', 'iritkatriel']
pr_nums = ['29905', '29981', '29996', '30015', '30073']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue45635'
versions = ['Python 3.11']

[iritkatriel]

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.

[iritkatriel]

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.

[iritkatriel]

New changeset d596acb by Irit Katriel in branch 'main':
bpo-45635: standardize error handling in traceback.c (GH-29905)
d596acb

[iritkatriel]

New changeset f893bb2 by Irit Katriel in branch 'main':
bpo-45635: refactor print_exception() into smaller functions (GH-29981)
f893bb2

[iritkatriel]

New changeset dc4a212 by Irit Katriel in branch 'main':
bpo-45635: continue refactor of print_exception() to standardize error handling (GH-29996)
dc4a212

[iritkatriel]

New changeset 0fe104f by Irit Katriel in branch 'main':
bpo-45635: refactor print_exception_recursive into smaller functions to standardize error handling (GH-30015)
0fe104f

[iritkatriel]

New changeset 8d6155e by Irit Katriel in branch 'main':
bpo-45635: Do not suppress errors in functions called from _PyErr_Display (GH-30073)
8d6155e