Skip to content

Commit

Permalink
gh-103333: Pickle the keyword attributes of AttributeError (#103352)
Browse files Browse the repository at this point in the history 
* Pickle the `name` and `args` attributes of AttributeError when present.

Co-authored-by: Gregory P. Smith <[email protected]>
Co-authored-by: Erlend E. Aasland <[email protected]>
  • Loading branch information
@csm10495 @gpshead @erlend-aasland
3 people authored May 12, 2023
1 parent cf720ac commit 79b17f2
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 28 deletions.
61 changes: 34 additions & 27 deletions Lib/test/test_exceptions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,45 +417,45 @@ def testAttributes(self):
# test that exception attributes are happy

exceptionList = [
(BaseException, (), {'args' : ()}),
(BaseException, (1, ), {'args' : (1,)}),
(BaseException, ('foo',),
(BaseException, (), {}, {'args' : ()}),
(BaseException, (1, ), {}, {'args' : (1,)}),
(BaseException, ('foo',), {},
{'args' : ('foo',)}),
(BaseException, ('foo', 1),
(BaseException, ('foo', 1), {},
{'args' : ('foo', 1)}),
(SystemExit, ('foo',),
(SystemExit, ('foo',), {},
{'args' : ('foo',), 'code' : 'foo'}),
(OSError, ('foo',),
(OSError, ('foo',), {},
{'args' : ('foo',), 'filename' : None, 'filename2' : None,
'errno' : None, 'strerror' : None}),
(OSError, ('foo', 'bar'),
(OSError, ('foo', 'bar'), {},
{'args' : ('foo', 'bar'),
'filename' : None, 'filename2' : None,
'errno' : 'foo', 'strerror' : 'bar'}),
(OSError, ('foo', 'bar', 'baz'),
(OSError, ('foo', 'bar', 'baz'), {},
{'args' : ('foo', 'bar'),
'filename' : 'baz', 'filename2' : None,
'errno' : 'foo', 'strerror' : 'bar'}),
(OSError, ('foo', 'bar', 'baz', None, 'quux'),
(OSError, ('foo', 'bar', 'baz', None, 'quux'), {},
{'args' : ('foo', 'bar'), 'filename' : 'baz', 'filename2': 'quux'}),
(OSError, ('errnoStr', 'strErrorStr', 'filenameStr'),
(OSError, ('errnoStr', 'strErrorStr', 'filenameStr'), {},
{'args' : ('errnoStr', 'strErrorStr'),
'strerror' : 'strErrorStr', 'errno' : 'errnoStr',
'filename' : 'filenameStr'}),
(OSError, (1, 'strErrorStr', 'filenameStr'),
(OSError, (1, 'strErrorStr', 'filenameStr'), {},
{'args' : (1, 'strErrorStr'), 'errno' : 1,
'strerror' : 'strErrorStr',
'filename' : 'filenameStr', 'filename2' : None}),
(SyntaxError, (), {'msg' : None, 'text' : None,
(SyntaxError, (), {}, {'msg' : None, 'text' : None,
'filename' : None, 'lineno' : None, 'offset' : None,
'end_offset': None, 'print_file_and_line' : None}),
(SyntaxError, ('msgStr',),
(SyntaxError, ('msgStr',), {},
{'args' : ('msgStr',), 'text' : None,
'print_file_and_line' : None, 'msg' : 'msgStr',
'filename' : None, 'lineno' : None, 'offset' : None,
'end_offset': None}),
(SyntaxError, ('msgStr', ('filenameStr', 'linenoStr', 'offsetStr',
'textStr', 'endLinenoStr', 'endOffsetStr')),
'textStr', 'endLinenoStr', 'endOffsetStr')), {},
{'offset' : 'offsetStr', 'text' : 'textStr',
'args' : ('msgStr', ('filenameStr', 'linenoStr',
'offsetStr', 'textStr',
Expand All @@ -465,46 +465,48 @@ def testAttributes(self):
'end_lineno': 'endLinenoStr', 'end_offset': 'endOffsetStr'}),
(SyntaxError, ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr',
'textStr', 'endLinenoStr', 'endOffsetStr',
'print_file_and_lineStr'),
'print_file_and_lineStr'), {},
{'text' : None,
'args' : ('msgStr', 'filenameStr', 'linenoStr', 'offsetStr',
'textStr', 'endLinenoStr', 'endOffsetStr',
'print_file_and_lineStr'),
'print_file_and_line' : None, 'msg' : 'msgStr',
'filename' : None, 'lineno' : None, 'offset' : None,
'end_lineno': None, 'end_offset': None}),
(UnicodeError, (), {'args' : (),}),
(UnicodeError, (), {}, {'args' : (),}),
(UnicodeEncodeError, ('ascii', 'a', 0, 1,
'ordinal not in range'),
'ordinal not in range'), {},
{'args' : ('ascii', 'a', 0, 1,
'ordinal not in range'),
'encoding' : 'ascii', 'object' : 'a',
'start' : 0, 'reason' : 'ordinal not in range'}),
(UnicodeDecodeError, ('ascii', bytearray(b'\xff'), 0, 1,
'ordinal not in range'),
'ordinal not in range'), {},
{'args' : ('ascii', bytearray(b'\xff'), 0, 1,
'ordinal not in range'),
'encoding' : 'ascii', 'object' : b'\xff',
'start' : 0, 'reason' : 'ordinal not in range'}),
(UnicodeDecodeError, ('ascii', b'\xff', 0, 1,
'ordinal not in range'),
'ordinal not in range'), {},
{'args' : ('ascii', b'\xff', 0, 1,
'ordinal not in range'),
'encoding' : 'ascii', 'object' : b'\xff',
'start' : 0, 'reason' : 'ordinal not in range'}),
(UnicodeTranslateError, ("\u3042", 0, 1, "ouch"),
(UnicodeTranslateError, ("\u3042", 0, 1, "ouch"), {},
{'args' : ('\u3042', 0, 1, 'ouch'),
'object' : '\u3042', 'reason' : 'ouch',
'start' : 0, 'end' : 1}),
(NaiveException, ('foo',),
(NaiveException, ('foo',), {},
{'args': ('foo',), 'x': 'foo'}),
(SlottedNaiveException, ('foo',),
(SlottedNaiveException, ('foo',), {},
{'args': ('foo',), 'x': 'foo'}),
(AttributeError, ('foo',), dict(name='name', obj='obj'),
dict(args=('foo',), name='name', obj='obj')),
]
try:
# More tests are in test_WindowsError
exceptionList.append(
(WindowsError, (1, 'strErrorStr', 'filenameStr'),
(WindowsError, (1, 'strErrorStr', 'filenameStr'), {},
{'args' : (1, 'strErrorStr'),
'strerror' : 'strErrorStr', 'winerror' : None,
'errno' : 1,
Expand All @@ -513,11 +515,11 @@ def testAttributes(self):
except NameError:
pass

for exc, args, expected in exceptionList:
for exc, args, kwargs, expected in exceptionList:
try:
e = exc(*args)
e = exc(*args, **kwargs)
except:
print("\nexc=%r, args=%r" % (exc, args), file=sys.stderr)
print(f"\nexc={exc!r}, args={args!r}", file=sys.stderr)
# raise
else:
# Verify module name
Expand All @@ -540,7 +542,12 @@ def testAttributes(self):
new = p.loads(s)
for checkArgName in expected:
got = repr(getattr(new, checkArgName))
want = repr(expected[checkArgName])
if exc == AttributeError and checkArgName == 'obj':
# See GH-103352, we're not pickling
# obj at this point. So verify it's None.
want = repr(None)
else:
want = repr(expected[checkArgName])
self.assertEqual(got, want,
'pickled "%r", attribute "%s' %
(e, checkArgName))
Expand Down
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/Core and Builtins/2023-04-09-04-30-02.gh-issue-103333.gKOetS.rst
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
:exc:`AttributeError` now retains the ``name`` attribute when pickled and unpickled.
44 changes: 43 additions & 1 deletion Objects/exceptions.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2287,14 +2287,56 @@ AttributeError_traverse(PyAttributeErrorObject *self, visitproc visit, void *arg
return BaseException_traverse((PyBaseExceptionObject *)self, visit, arg);
}

/* Pickling support */
static PyObject *
AttributeError_getstate(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored))
{
PyObject *dict = ((PyAttributeErrorObject *)self)->dict;
if (self->name || self->args) {
dict = dict ? PyDict_Copy(dict) : PyDict_New();
if (dict == NULL) {
return NULL;
}
if (self->name && PyDict_SetItemString(dict, "name", self->name) < 0) {
Py_DECREF(dict);
return NULL;
}
/* We specifically are not pickling the obj attribute since there are many
cases where it is unlikely to be picklable. See GH-103352.
*/
if (self->args && PyDict_SetItemString(dict, "args", self->args) < 0) {
Py_DECREF(dict);
return NULL;
}
return dict;
}
else if (dict) {
return Py_NewRef(dict);
}
Py_RETURN_NONE;
}

static PyObject *
AttributeError_reduce(PyAttributeErrorObject *self, PyObject *Py_UNUSED(ignored))
{
PyObject *state = AttributeError_getstate(self, NULL);
if (state == NULL) {
return NULL;
}

return PyTuple_Pack(3, Py_TYPE(self), self->args, state);
}

static PyMemberDef AttributeError_members[] = {
{"name", T_OBJECT, offsetof(PyAttributeErrorObject, name), 0, PyDoc_STR("attribute name")},
{"obj", T_OBJECT, offsetof(PyAttributeErrorObject, obj), 0, PyDoc_STR("object")},
{NULL} /* Sentinel */
};

static PyMethodDef AttributeError_methods[] = {
{NULL} /* Sentinel */
{"__getstate__", (PyCFunction)AttributeError_getstate, METH_NOARGS},
{"__reduce__", (PyCFunction)AttributeError_reduce, METH_NOARGS },
{NULL}
};

ComplexExtendsException(PyExc_Exception, AttributeError,
Expand Down

0 comments on commit 79b17f2

Please sign in to comment.