Skip to content

Commit ff2577f

Browse files
authored
gh-141732: Fix ExceptionGroup repr changing when original exception sequence is mutated (#141736)
1 parent dc9f238 commit ff2577f

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
@@ -978,6 +978,12 @@ their subgroups based on the types of the contained exceptions.
978978
raises a :exc:`TypeError` if any contained exception is not an
979979
:exc:`Exception` subclass.
980980

981+
.. impl-detail::
982+
983+
The ``excs`` parameter may be any sequence, but lists and tuples are
984+
specifically processed more efficiently here. For optimal performance,
985+
pass a tuple as ``excs``.
986+
981987
.. attribute:: message
982988

983989
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
@@ -694,12 +694,12 @@ PyTypeObject _PyExc_ ## EXCNAME = { \
694694

695695
#define ComplexExtendsException(EXCBASE, EXCNAME, EXCSTORE, EXCNEW, \
696696
EXCMETHODS, EXCMEMBERS, EXCGETSET, \
697-
EXCSTR, EXCDOC) \
697+
EXCSTR, EXCREPR, EXCDOC) \
698698
static PyTypeObject _PyExc_ ## EXCNAME = { \
699699
PyVarObject_HEAD_INIT(NULL, 0) \
700700
# EXCNAME, \
701701
sizeof(Py ## EXCSTORE ## Object), 0, \
702-
EXCSTORE ## _dealloc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
702+
EXCSTORE ## _dealloc, 0, 0, 0, 0, EXCREPR, 0, 0, 0, 0, 0, \
703703
EXCSTR, 0, 0, 0, \
704704
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, \
705705
PyDoc_STR(EXCDOC), EXCSTORE ## _traverse, \
@@ -792,7 +792,7 @@ StopIteration_traverse(PyObject *op, visitproc visit, void *arg)
792792
}
793793

794794
ComplexExtendsException(PyExc_Exception, StopIteration, StopIteration,
795-
0, 0, StopIteration_members, 0, 0,
795+
0, 0, StopIteration_members, 0, 0, 0,
796796
"Signal the end from iterator.__next__().");
797797

798798

@@ -865,7 +865,7 @@ static PyMemberDef SystemExit_members[] = {
865865
};
866866

867867
ComplexExtendsException(PyExc_BaseException, SystemExit, SystemExit,
868-
0, 0, SystemExit_members, 0, 0,
868+
0, 0, SystemExit_members, 0, 0, 0,
869869
"Request to exit from the interpreter.");
870870

871871
/*
@@ -890,6 +890,7 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
890890

891891
PyObject *message = NULL;
892892
PyObject *exceptions = NULL;
893+
PyObject *exceptions_str = NULL;
893894

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

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

9891002
self->msg = Py_NewRef(message);
9901003
self->excs = exceptions;
1004+
self->excs_str = exceptions_str;
9911005
return (PyObject*)self;
9921006
error:
993-
Py_DECREF(exceptions);
1007+
Py_XDECREF(exceptions);
1008+
Py_XDECREF(exceptions_str);
9941009
return NULL;
9951010
}
9961011

@@ -1029,6 +1044,7 @@ BaseExceptionGroup_clear(PyObject *op)
10291044
PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op);
10301045
Py_CLEAR(self->msg);
10311046
Py_CLEAR(self->excs);
1047+
Py_CLEAR(self->excs_str);
10321048
return BaseException_clear(op);
10331049
}
10341050

@@ -1046,6 +1062,7 @@ BaseExceptionGroup_traverse(PyObject *op, visitproc visit, void *arg)
10461062
PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op);
10471063
Py_VISIT(self->msg);
10481064
Py_VISIT(self->excs);
1065+
Py_VISIT(self->excs_str);
10491066
return BaseException_traverse(op, visit, arg);
10501067
}
10511068

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

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

17031768
/*
@@ -2425,7 +2490,7 @@ static PyGetSetDef OSError_getset[] = {
24252490
ComplexExtendsException(PyExc_Exception, OSError,
24262491
OSError, OSError_new,
24272492
OSError_methods, OSError_members, OSError_getset,
2428-
OSError_str,
2493+
OSError_str, 0,
24292494
"Base class for I/O related errors.");
24302495

24312496

@@ -2566,7 +2631,7 @@ static PyMethodDef NameError_methods[] = {
25662631
ComplexExtendsException(PyExc_Exception, NameError,
25672632
NameError, 0,
25682633
NameError_methods, NameError_members,
2569-
0, BaseException_str, "Name not found globally.");
2634+
0, BaseException_str, 0, "Name not found globally.");
25702635

25712636
/*
25722637
* UnboundLocalError extends NameError
@@ -2700,7 +2765,7 @@ static PyMethodDef AttributeError_methods[] = {
27002765
ComplexExtendsException(PyExc_Exception, AttributeError,
27012766
AttributeError, 0,
27022767
AttributeError_methods, AttributeError_members,
2703-
0, BaseException_str, "Attribute not found.");
2768+
0, BaseException_str, 0, "Attribute not found.");
27042769

27052770
/*
27062771
* SyntaxError extends Exception
@@ -2899,7 +2964,7 @@ static PyMemberDef SyntaxError_members[] = {
28992964

29002965
ComplexExtendsException(PyExc_Exception, SyntaxError, SyntaxError,
29012966
0, 0, SyntaxError_members, 0,
2902-
SyntaxError_str, "Invalid syntax.");
2967+
SyntaxError_str, 0, "Invalid syntax.");
29032968

29042969

29052970
/*
@@ -2959,7 +3024,7 @@ KeyError_str(PyObject *op)
29593024
}
29603025

29613026
ComplexExtendsException(PyExc_LookupError, KeyError, BaseException,
2962-
0, 0, 0, 0, KeyError_str, "Mapping key not found.");
3027+
0, 0, 0, 0, KeyError_str, 0, "Mapping key not found.");
29633028

29643029

29653030
/*

0 commit comments

Comments
 (0)