Skip to content

Commit d768900

Browse files
authored
[3.13] gh-141732: Fix ExceptionGroup repr changing when original exception sequence is mutated (GH-141736) (#142391)
* [3.13] gh-141732: Fix `ExceptionGroup` repr changing when original exception sequence is mutated (GH-141736) (cherry picked from commit ff2577f) Co-authored-by: dr-carlos <[email protected]>
1 parent 2c3e3ef commit d768900

File tree

5 files changed

+158
-14
lines changed

5 files changed

+158
-14
lines changed

Doc/library/exceptions.rst

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

964+
.. impl-detail::
965+
966+
The ``excs`` parameter may be any sequence, but lists and tuples are
967+
specifically processed more efficiently here. For optimal performance,
968+
pass a tuple as ``excs``.
969+
964970
.. attribute:: message
965971

966972
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 get_c_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"__repr__ returned non-string \(type 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: 77 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -531,13 +531,13 @@ PyTypeObject _PyExc_ ## EXCNAME = { \
531531

532532
#define ComplexExtendsException(EXCBASE, EXCNAME, EXCSTORE, EXCNEW, \
533533
EXCMETHODS, EXCMEMBERS, EXCGETSET, \
534-
EXCSTR, EXCDOC) \
534+
EXCSTR, EXCREPR, EXCDOC) \
535535
static PyTypeObject _PyExc_ ## EXCNAME = { \
536536
PyVarObject_HEAD_INIT(NULL, 0) \
537537
# EXCNAME, \
538538
sizeof(Py ## EXCSTORE ## Object), 0, \
539-
(destructor)EXCSTORE ## _dealloc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \
540-
(reprfunc)EXCSTR, 0, 0, 0, \
539+
(destructor)EXCSTORE ## _dealloc, 0, 0, 0, 0, (reprfunc)EXCREPR, 0, 0, 0, \
540+
0, 0, (reprfunc)EXCSTR, 0, 0, 0, \
541541
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, \
542542
PyDoc_STR(EXCDOC), (traverseproc)EXCSTORE ## _traverse, \
543543
(inquiry)EXCSTORE ## _clear, 0, 0, 0, 0, EXCMETHODS, \
@@ -619,7 +619,7 @@ StopIteration_traverse(PyStopIterationObject *self, visitproc visit, void *arg)
619619
}
620620

621621
ComplexExtendsException(PyExc_Exception, StopIteration, StopIteration,
622-
0, 0, StopIteration_members, 0, 0,
622+
0, 0, StopIteration_members, 0, 0, 0,
623623
"Signal the end from iterator.__next__().");
624624

625625

@@ -682,7 +682,7 @@ static PyMemberDef SystemExit_members[] = {
682682
};
683683

684684
ComplexExtendsException(PyExc_BaseException, SystemExit, SystemExit,
685-
0, 0, SystemExit_members, 0, 0,
685+
0, 0, SystemExit_members, 0, 0, 0,
686686
"Request to exit from the interpreter.");
687687

688688
/*
@@ -707,6 +707,7 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
707707

708708
PyObject *message = NULL;
709709
PyObject *exceptions = NULL;
710+
PyObject *exceptions_str = NULL;
710711

711712
if (!PyArg_ParseTuple(args,
712713
"UO:BaseExceptionGroup.__new__",
@@ -722,6 +723,18 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
722723
return NULL;
723724
}
724725

726+
/* Save initial exceptions sequence as a string in case sequence is mutated */
727+
if (!PyList_Check(exceptions) && !PyTuple_Check(exceptions)) {
728+
exceptions_str = PyObject_Repr(exceptions);
729+
if (exceptions_str == NULL) {
730+
/* We don't hold a reference to exceptions, so clear it before
731+
* attempting a decref in the cleanup.
732+
*/
733+
exceptions = NULL;
734+
goto error;
735+
}
736+
}
737+
725738
exceptions = PySequence_Tuple(exceptions);
726739
if (!exceptions) {
727740
return NULL;
@@ -805,9 +818,11 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
805818

806819
self->msg = Py_NewRef(message);
807820
self->excs = exceptions;
821+
self->excs_str = exceptions_str;
808822
return (PyObject*)self;
809823
error:
810-
Py_DECREF(exceptions);
824+
Py_XDECREF(exceptions);
825+
Py_XDECREF(exceptions_str);
811826
return NULL;
812827
}
813828

@@ -846,6 +861,7 @@ BaseExceptionGroup_clear(PyBaseExceptionGroupObject *self)
846861
{
847862
Py_CLEAR(self->msg);
848863
Py_CLEAR(self->excs);
864+
Py_CLEAR(self->excs_str);
849865
return BaseException_clear((PyBaseExceptionObject *)self);
850866
}
851867

@@ -863,6 +879,7 @@ BaseExceptionGroup_traverse(PyBaseExceptionGroupObject *self,
863879
{
864880
Py_VISIT(self->msg);
865881
Py_VISIT(self->excs);
882+
Py_VISIT(self->excs_str);
866883
return BaseException_traverse((PyBaseExceptionObject *)self, visit, arg);
867884
}
868885

@@ -879,6 +896,53 @@ BaseExceptionGroup_str(PyBaseExceptionGroupObject *self)
879896
self->msg, num_excs, num_excs > 1 ? "s" : "");
880897
}
881898

899+
static PyObject *
900+
BaseExceptionGroup_repr(PyBaseExceptionGroupObject *self)
901+
{
902+
assert(self->msg);
903+
904+
PyObject *exceptions_str = NULL;
905+
906+
/* Use the saved exceptions string for custom sequences. */
907+
if (self->excs_str) {
908+
exceptions_str = Py_NewRef(self->excs_str);
909+
}
910+
else {
911+
assert(self->excs);
912+
913+
/* Older versions delegated to BaseException, inserting the current
914+
* value of self.args[1]; but this can be mutable and go out-of-sync
915+
* with self.exceptions. Instead, use self.exceptions for accuracy,
916+
* making it look like self.args[1] for backwards compatibility. */
917+
if (PyList_Check(PyTuple_GET_ITEM(self->args, 1))) {
918+
PyObject *exceptions_list = PySequence_List(self->excs);
919+
if (!exceptions_list) {
920+
return NULL;
921+
}
922+
923+
exceptions_str = PyObject_Repr(exceptions_list);
924+
Py_DECREF(exceptions_list);
925+
}
926+
else {
927+
exceptions_str = PyObject_Repr(self->excs);
928+
}
929+
930+
if (!exceptions_str) {
931+
return NULL;
932+
}
933+
}
934+
935+
assert(exceptions_str != NULL);
936+
937+
const char *name = _PyType_Name(Py_TYPE(self));
938+
PyObject *repr = PyUnicode_FromFormat(
939+
"%s(%R, %U)", name,
940+
self->msg, exceptions_str);
941+
942+
Py_DECREF(exceptions_str);
943+
return repr;
944+
}
945+
882946
static PyObject *
883947
BaseExceptionGroup_derive(PyObject *self_, PyObject *excs)
884948
{
@@ -1487,7 +1551,7 @@ static PyMethodDef BaseExceptionGroup_methods[] = {
14871551
ComplexExtendsException(PyExc_BaseException, BaseExceptionGroup,
14881552
BaseExceptionGroup, BaseExceptionGroup_new /* new */,
14891553
BaseExceptionGroup_methods, BaseExceptionGroup_members,
1490-
0 /* getset */, BaseExceptionGroup_str,
1554+
0 /* getset */, BaseExceptionGroup_str, BaseExceptionGroup_repr,
14911555
"A combination of multiple unrelated exceptions.");
14921556

14931557
/*
@@ -1664,7 +1728,7 @@ static PyMethodDef ImportError_methods[] = {
16641728
ComplexExtendsException(PyExc_Exception, ImportError,
16651729
ImportError, 0 /* new */,
16661730
ImportError_methods, ImportError_members,
1667-
0 /* getset */, ImportError_str,
1731+
0 /* getset */, ImportError_str, 0,
16681732
"Import can't find module, or can't find name in "
16691733
"module.");
16701734

@@ -2124,7 +2188,7 @@ static PyGetSetDef OSError_getset[] = {
21242188
ComplexExtendsException(PyExc_Exception, OSError,
21252189
OSError, OSError_new,
21262190
OSError_methods, OSError_members, OSError_getset,
2127-
OSError_str,
2191+
OSError_str, 0,
21282192
"Base class for I/O related errors.");
21292193

21302194

@@ -2255,7 +2319,7 @@ static PyMethodDef NameError_methods[] = {
22552319
ComplexExtendsException(PyExc_Exception, NameError,
22562320
NameError, 0,
22572321
NameError_methods, NameError_members,
2258-
0, BaseException_str, "Name not found globally.");
2322+
0, BaseException_str, 0, "Name not found globally.");
22592323

22602324
/*
22612325
* UnboundLocalError extends NameError
@@ -2377,7 +2441,7 @@ static PyMethodDef AttributeError_methods[] = {
23772441
ComplexExtendsException(PyExc_Exception, AttributeError,
23782442
AttributeError, 0,
23792443
AttributeError_methods, AttributeError_members,
2380-
0, BaseException_str, "Attribute not found.");
2444+
0, BaseException_str, 0, "Attribute not found.");
23812445

23822446
/*
23832447
* SyntaxError extends Exception
@@ -2558,7 +2622,7 @@ static PyMemberDef SyntaxError_members[] = {
25582622

25592623
ComplexExtendsException(PyExc_Exception, SyntaxError, SyntaxError,
25602624
0, 0, SyntaxError_members, 0,
2561-
SyntaxError_str, "Invalid syntax.");
2625+
SyntaxError_str, 0, "Invalid syntax.");
25622626

25632627

25642628
/*
@@ -2616,7 +2680,7 @@ KeyError_str(PyBaseExceptionObject *self)
26162680
}
26172681

26182682
ComplexExtendsException(PyExc_LookupError, KeyError, BaseException,
2619-
0, 0, 0, 0, KeyError_str, "Mapping key not found.");
2683+
0, 0, 0, 0, KeyError_str, 0, "Mapping key not found.");
26202684

26212685

26222686
/*

0 commit comments

Comments
 (0)