Skip to content

Commit 30667ed

Browse files
dr-carlosmiss-islington
authored andcommitted
pythongh-141732: Fix ExceptionGroup repr changing when original exception sequence is mutated (pythonGH-141736)
(cherry picked from commit ff2577f) Co-authored-by: dr-carlos <[email protected]>
1 parent 7308015 commit 30667ed

File tree

5 files changed

+157
-12
lines changed

5 files changed

+157
-12
lines changed

Doc/library/exceptions.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -971,6 +971,12 @@ their subgroups based on the types of the contained exceptions.
971971
raises a :exc:`TypeError` if any contained exception is not an
972972
:exc:`Exception` subclass.
973973

974+
.. impl-detail::
975+
976+
The ``excs`` parameter may be any sequence, but lists and tuples are
977+
specifically processed more efficiently here. For optimal performance,
978+
pass a tuple as ``excs``.
979+
974980
.. attribute:: message
975981

976982
The ``msg`` argument to the constructor. This is a read-only attribute.

Include/cpython/pyerrors.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ typedef struct {
1818
PyException_HEAD
1919
PyObject *msg;
2020
PyObject *excs;
21+
PyObject *excs_str;
2122
} PyBaseExceptionGroupObject;
2223

2324
typedef struct {

Lib/test/test_exception_group.py

Lines changed: 72 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import collections.abc
1+
import collections
22
import types
33
import unittest
44
from test.support import skip_emscripten_stack_overflow, skip_wasi_stack_overflow, exceeds_recursion_limit
@@ -193,6 +193,77 @@ class MyEG(ExceptionGroup):
193193
"MyEG('flat', [ValueError(1), TypeError(2)]), "
194194
"TypeError(2)])"))
195195

196+
def test_exceptions_mutation(self):
197+
class MyEG(ExceptionGroup):
198+
pass
199+
200+
excs = [ValueError(1), TypeError(2)]
201+
eg = MyEG('test', excs)
202+
203+
self.assertEqual(repr(eg), "MyEG('test', [ValueError(1), TypeError(2)])")
204+
excs.clear()
205+
206+
# Ensure that clearing the exceptions sequence doesn't change the repr.
207+
self.assertEqual(repr(eg), "MyEG('test', [ValueError(1), TypeError(2)])")
208+
209+
# Ensure that the args are still as passed.
210+
self.assertEqual(eg.args, ('test', []))
211+
212+
excs = (ValueError(1), KeyboardInterrupt(2))
213+
eg = BaseExceptionGroup('test', excs)
214+
215+
# Ensure that immutable sequences still work fine.
216+
self.assertEqual(
217+
repr(eg),
218+
"BaseExceptionGroup('test', (ValueError(1), KeyboardInterrupt(2)))"
219+
)
220+
221+
# Test non-standard custom sequences.
222+
excs = collections.deque([ValueError(1), TypeError(2)])
223+
eg = ExceptionGroup('test', excs)
224+
225+
self.assertEqual(
226+
repr(eg),
227+
"ExceptionGroup('test', deque([ValueError(1), TypeError(2)]))"
228+
)
229+
excs.clear()
230+
231+
# Ensure that clearing the exceptions sequence doesn't change the repr.
232+
self.assertEqual(
233+
repr(eg),
234+
"ExceptionGroup('test', deque([ValueError(1), TypeError(2)]))"
235+
)
236+
237+
def test_repr_raises(self):
238+
class MySeq(collections.abc.Sequence):
239+
def __init__(self, raises):
240+
self.raises = raises
241+
242+
def __len__(self):
243+
return 1
244+
245+
def __getitem__(self, index):
246+
if index == 0:
247+
return ValueError(1)
248+
raise IndexError
249+
250+
def __repr__(self):
251+
if self.raises:
252+
raise self.raises
253+
return None
254+
255+
seq = MySeq(None)
256+
with self.assertRaisesRegex(
257+
TypeError,
258+
r".*MySeq\.__repr__\(\) must return a str, not NoneType"
259+
):
260+
ExceptionGroup("test", seq)
261+
262+
seq = MySeq(ValueError)
263+
with self.assertRaises(ValueError):
264+
BaseExceptionGroup("test", seq)
265+
266+
196267

197268
def create_simple_eg():
198269
excs = []
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Ensure the :meth:`~object.__repr__` for :exc:`ExceptionGroup` and :exc:`BaseExceptionGroup` does
2+
not change when the exception sequence that was original passed in to its constructor is subsequently mutated.

Objects/exceptions.c

Lines changed: 76 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -695,12 +695,12 @@ PyTypeObject _PyExc_ ## EXCNAME = { \
695695

696696
#define ComplexExtendsException(EXCBASE, EXCNAME, EXCSTORE, EXCNEW, \
697697
EXCMETHODS, EXCMEMBERS, EXCGETSET, \
698-
EXCSTR, EXCDOC) \
698+
EXCSTR, EXCREPR, EXCDOC) \
699699
static PyTypeObject _PyExc_ ## EXCNAME = { \
700700
PyVarObject_HEAD_INIT(NULL, 0) \
701701
# EXCNAME, \
702702
sizeof(Py ## EXCSTORE ## Object), 0, \
703-
EXCSTORE ## _dealloc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
703+
EXCSTORE ## _dealloc, 0, 0, 0, 0, EXCREPR, 0, 0, 0, 0, 0, \
704704
EXCSTR, 0, 0, 0, \
705705
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, \
706706
PyDoc_STR(EXCDOC), EXCSTORE ## _traverse, \
@@ -793,7 +793,7 @@ StopIteration_traverse(PyObject *op, visitproc visit, void *arg)
793793
}
794794

795795
ComplexExtendsException(PyExc_Exception, StopIteration, StopIteration,
796-
0, 0, StopIteration_members, 0, 0,
796+
0, 0, StopIteration_members, 0, 0, 0,
797797
"Signal the end from iterator.__next__().");
798798

799799

@@ -866,7 +866,7 @@ static PyMemberDef SystemExit_members[] = {
866866
};
867867

868868
ComplexExtendsException(PyExc_BaseException, SystemExit, SystemExit,
869-
0, 0, SystemExit_members, 0, 0,
869+
0, 0, SystemExit_members, 0, 0, 0,
870870
"Request to exit from the interpreter.");
871871

872872
/*
@@ -891,6 +891,7 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
891891

892892
PyObject *message = NULL;
893893
PyObject *exceptions = NULL;
894+
PyObject *exceptions_str = NULL;
894895

895896
if (!PyArg_ParseTuple(args,
896897
"UO:BaseExceptionGroup.__new__",
@@ -906,6 +907,18 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
906907
return NULL;
907908
}
908909

910+
/* Save initial exceptions sequence as a string in case sequence is mutated */
911+
if (!PyList_Check(exceptions) && !PyTuple_Check(exceptions)) {
912+
exceptions_str = PyObject_Repr(exceptions);
913+
if (exceptions_str == NULL) {
914+
/* We don't hold a reference to exceptions, so clear it before
915+
* attempting a decref in the cleanup.
916+
*/
917+
exceptions = NULL;
918+
goto error;
919+
}
920+
}
921+
909922
exceptions = PySequence_Tuple(exceptions);
910923
if (!exceptions) {
911924
return NULL;
@@ -989,9 +1002,11 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
9891002

9901003
self->msg = Py_NewRef(message);
9911004
self->excs = exceptions;
1005+
self->excs_str = exceptions_str;
9921006
return (PyObject*)self;
9931007
error:
994-
Py_DECREF(exceptions);
1008+
Py_XDECREF(exceptions);
1009+
Py_XDECREF(exceptions_str);
9951010
return NULL;
9961011
}
9971012

@@ -1030,6 +1045,7 @@ BaseExceptionGroup_clear(PyObject *op)
10301045
PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op);
10311046
Py_CLEAR(self->msg);
10321047
Py_CLEAR(self->excs);
1048+
Py_CLEAR(self->excs_str);
10331049
return BaseException_clear(op);
10341050
}
10351051

@@ -1047,6 +1063,7 @@ BaseExceptionGroup_traverse(PyObject *op, visitproc visit, void *arg)
10471063
PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op);
10481064
Py_VISIT(self->msg);
10491065
Py_VISIT(self->excs);
1066+
Py_VISIT(self->excs_str);
10501067
return BaseException_traverse(op, visit, arg);
10511068
}
10521069

@@ -1064,6 +1081,54 @@ BaseExceptionGroup_str(PyObject *op)
10641081
self->msg, num_excs, num_excs > 1 ? "s" : "");
10651082
}
10661083

1084+
static PyObject *
1085+
BaseExceptionGroup_repr(PyObject *op)
1086+
{
1087+
PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op);
1088+
assert(self->msg);
1089+
1090+
PyObject *exceptions_str = NULL;
1091+
1092+
/* Use the saved exceptions string for custom sequences. */
1093+
if (self->excs_str) {
1094+
exceptions_str = Py_NewRef(self->excs_str);
1095+
}
1096+
else {
1097+
assert(self->excs);
1098+
1099+
/* Older versions delegated to BaseException, inserting the current
1100+
* value of self.args[1]; but this can be mutable and go out-of-sync
1101+
* with self.exceptions. Instead, use self.exceptions for accuracy,
1102+
* making it look like self.args[1] for backwards compatibility. */
1103+
if (PyList_Check(PyTuple_GET_ITEM(self->args, 1))) {
1104+
PyObject *exceptions_list = PySequence_List(self->excs);
1105+
if (!exceptions_list) {
1106+
return NULL;
1107+
}
1108+
1109+
exceptions_str = PyObject_Repr(exceptions_list);
1110+
Py_DECREF(exceptions_list);
1111+
}
1112+
else {
1113+
exceptions_str = PyObject_Repr(self->excs);
1114+
}
1115+
1116+
if (!exceptions_str) {
1117+
return NULL;
1118+
}
1119+
}
1120+
1121+
assert(exceptions_str != NULL);
1122+
1123+
const char *name = _PyType_Name(Py_TYPE(self));
1124+
PyObject *repr = PyUnicode_FromFormat(
1125+
"%s(%R, %U)", name,
1126+
self->msg, exceptions_str);
1127+
1128+
Py_DECREF(exceptions_str);
1129+
return repr;
1130+
}
1131+
10671132
/*[clinic input]
10681133
@critical_section
10691134
BaseExceptionGroup.derive
@@ -1698,7 +1763,7 @@ static PyMethodDef BaseExceptionGroup_methods[] = {
16981763
ComplexExtendsException(PyExc_BaseException, BaseExceptionGroup,
16991764
BaseExceptionGroup, BaseExceptionGroup_new /* new */,
17001765
BaseExceptionGroup_methods, BaseExceptionGroup_members,
1701-
0 /* getset */, BaseExceptionGroup_str,
1766+
0 /* getset */, BaseExceptionGroup_str, BaseExceptionGroup_repr,
17021767
"A combination of multiple unrelated exceptions.");
17031768

17041769
/*
@@ -2356,7 +2421,7 @@ static PyGetSetDef OSError_getset[] = {
23562421
ComplexExtendsException(PyExc_Exception, OSError,
23572422
OSError, OSError_new,
23582423
OSError_methods, OSError_members, OSError_getset,
2359-
OSError_str,
2424+
OSError_str, 0,
23602425
"Base class for I/O related errors.");
23612426

23622427

@@ -2497,7 +2562,7 @@ static PyMethodDef NameError_methods[] = {
24972562
ComplexExtendsException(PyExc_Exception, NameError,
24982563
NameError, 0,
24992564
NameError_methods, NameError_members,
2500-
0, BaseException_str, "Name not found globally.");
2565+
0, BaseException_str, 0, "Name not found globally.");
25012566

25022567
/*
25032568
* UnboundLocalError extends NameError
@@ -2631,7 +2696,7 @@ static PyMethodDef AttributeError_methods[] = {
26312696
ComplexExtendsException(PyExc_Exception, AttributeError,
26322697
AttributeError, 0,
26332698
AttributeError_methods, AttributeError_members,
2634-
0, BaseException_str, "Attribute not found.");
2699+
0, BaseException_str, 0, "Attribute not found.");
26352700

26362701
/*
26372702
* SyntaxError extends Exception
@@ -2830,7 +2895,7 @@ static PyMemberDef SyntaxError_members[] = {
28302895

28312896
ComplexExtendsException(PyExc_Exception, SyntaxError, SyntaxError,
28322897
0, 0, SyntaxError_members, 0,
2833-
SyntaxError_str, "Invalid syntax.");
2898+
SyntaxError_str, 0, "Invalid syntax.");
28342899

28352900

28362901
/*
@@ -2890,7 +2955,7 @@ KeyError_str(PyObject *op)
28902955
}
28912956

28922957
ComplexExtendsException(PyExc_LookupError, KeyError, BaseException,
2893-
0, 0, 0, 0, KeyError_str, "Mapping key not found.");
2958+
0, 0, 0, 0, KeyError_str, 0, "Mapping key not found.");
28942959

28952960

28962961
/*

0 commit comments

Comments
 (0)