diff --git a/Include/pyerrors.h b/Include/pyerrors.h --- a/Include/pyerrors.h +++ b/Include/pyerrors.h @@ -76,6 +76,7 @@ typedef PyOSErrorObject PyWindowsErrorObject; PyAPI_FUNC(void) PyErr_SetNone(PyObject *); PyAPI_FUNC(void) PyErr_SetObject(PyObject *, PyObject *); +PyAPI_FUNC(void) PyErr_SetWithTraceback(PyObject *, PyObject *); #ifndef Py_LIMITED_API PyAPI_FUNC(void) _PyErr_SetKeyError(PyObject *); #endif @@ -128,7 +129,8 @@ PyAPI_FUNC(void) PyException_SetContext(PyObject *, PyObject *); PyAPI_FUNC(void) _PyErr_ChainExceptions(PyObject *, PyObject *, PyObject *); #endif -/* */ +/* Check that an exception has no arguments */ +PyAPI_FUNC(int) PyErr_NoArgs(PyObject *); #define PyExceptionClass_Check(x) \ (PyType_Check((x)) && \ @@ -247,11 +249,20 @@ PyAPI_FUNC(PyObject *) PyErr_Format( const char *format, /* ASCII-encoded string */ ... ); +PyAPI_FUNC(PyObject *) PyErr_FormatWithTraceback( + PyObject *exception, + const char *format, + ... + ); #if !defined(Py_LIMITED_API) || Py_LIMITED_API+0 >= 0x03050000 PyAPI_FUNC(PyObject *) PyErr_FormatV( PyObject *exception, const char *format, va_list vargs); +PyAPI_FUNC(PyObject *) PyErr_FormatVWithTraceback( + PyObject *exception, + const char *format, + va_list vargs); #endif #ifdef MS_WINDOWS diff --git a/Lib/test/test_exceptions.py b/Lib/test/test_exceptions.py --- a/Lib/test/test_exceptions.py +++ b/Lib/test/test_exceptions.py @@ -5,6 +5,7 @@ import sys import unittest import pickle import weakref +import traceback import errno from test.support import (TESTFN, captured_stderr, check_impl_detail, @@ -1075,6 +1076,336 @@ class ExceptionTests(unittest.TestCase): self.assertIn("test message", report) self.assertTrue(report.endswith("\n")) +class AttributeErrorChangedTests(unittest.TestCase): + """Bare attribute errors should be made more informative.""" + + # we can't use self.assertRaises everywhere, because it kills the + # traceback, and we need to check it at least in a few places to + # make sure it's correct. for the sake of simplicity and convenience, + # only a few test cases check for the traceback; we can assume that + # if those cases are fine, the rest will be too. + + # attribute and descriptor getters + + def err_msg_get(self, name, attr): + return (f"'{name}' object has no attribute '{attr}'",) + + def tb_msg_get(self, name): + return f", in __getattr{name=='A' and 'ibute' or ''}__\n" + + def test_getattribute_bare(self): + class A: + def __getattribute__(self, name): + if name == "spam": + return "eggs" + raise AttributeError + + class B: + def __getattr__(self, name): + if name == "spam": + return "eggs" + raise AttributeError + + for a, b in ([A(), "A"], [B(), "B"]): + self.assertEqual(a.spam, "eggs") + try: + a.eggs + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_get(b, "eggs")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_get(b), "".join(formatted_tb[-2:])) + + del exc + + def test_getattribute_none(self): + class A: + def __getattribute__(self, name): + raise AttributeError(None) + + class B: + def __getattr__(self, name): + raise AttributeError(None) + + for a, b in ([A(), "A"], [B(), "B"]): + try: + a.spam + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_get(b, "spam")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_get(b), "".join(formatted_tb[-2:])) + + del exc + + def test_getattribute_normal(self): + class A: + def __getattribute__(self, name): + raise AttributeError(name) + + class B: + def __getattr__(self, name): + raise AttributeError(name) + + for a, b in ([A(), "A"], [B(), "B"]): + with self.assertRaises(AttributeError) as e: + a.eggs + + self.assertEqual(e.exception.args, ("eggs",)) + + def test_descr_get(self): + class A: + def __get__(self, inst, owner): + raise AttributeError # we don't know the name we're bound to + + instance = A() + + class B: + foo = instance + + b = B() + self.assertIs(B.__dict__["foo"], instance) + for a, c in ([B, "type"], [b, "B"]): + with self.assertRaises(AttributeError) as e: + a.foo + + self.assertEqual(e.exception.args, self.err_msg_get(c, "foo")) + + def test_meta_descr_get(self): + class M(type): + def __get__(self, inst, owner): + if inst is None: + return self + raise AttributeError + + class N(metaclass=M): + pass + + class A: + bar = N + + self.assertIs(A.bar, N) + with self.assertRaises(AttributeError) as e: + A().bar + + self.assertEqual(e.exception.args, self.err_msg_get("A", "bar")) + + def test_descr_meta_get(self): + class A: + def __get__(self, inst, owner): + if inst is None: + return self + raise AttributeError + + instance = A() + + class M(type): + spam = instance + + class N(metaclass=M): + pass + + self.assertIs(M.spam, instance) + with self.assertRaises(AttributeError) as e: + N.spam + + self.assertEqual(e.exception.args, self.err_msg_get("M", "spam")) + + # attribute and descriptor setters and deleters + + def err_msg_set(self, name, attr, action): + return (f"cannot {action} attribute '{attr}' of '{name}' object",) + + def tb_msg_set(self, name): + return f", in __{name}attr__\n" + + def test_change_attribute_bare(self): + class A: + def __setattr__(self, name, value): + if name == "spam": + return super().__setattr__(name, value) + raise AttributeError + + def __delattr__(self, name): + if name == "spam": + return super().__delattr__(name) + raise AttributeError + + a = A() + + a.spam = "eggs" + self.assertEqual(a.spam, "eggs") + try: + a.eggs = "scrambled" + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_set("A", "eggs", "set")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_set("set"), "".join(formatted_tb[-2:])) + + del exc + + del a.spam + with self.assertRaises(AttributeError): + a.spam + + try: + del a.eggs + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_set("A", "eggs", "delete")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_set("del"), "".join(formatted_tb[-2:])) + + del exc + + def test_change_attribute_none(self): + class A: + def __setattr__(self, name, value): + raise AttributeError(None) + + def __delattr__(self, name): + raise AttributeError(None) + + a = A() + + try: + a.eggs = "scrambled" + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_set("A", "eggs", "set")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_set("set"), "".join(formatted_tb[-2:])) + + del exc + + try: + del a.eggs + except AttributeError as e: + exc = e + else: + self.fail("AttributeError wasn't raised") + + self.assertEqual(exc.args, self.err_msg_set("A", "eggs", "delete")) + formatted_tb = traceback.format_tb(exc.__traceback__) + self.assertGreater(len(formatted_tb), 1) + self.assertIn(self.tb_msg_set("del"), "".join(formatted_tb[-2:])) + + del exc + + def test_change_attribute_normal(self): + class A: + def __init__(self): + super().__setattr__("spam", "scrambled eggs") + + def __setattr__(self, name, value): + raise AttributeError(name) + + def __delattr__(self, name): + raise AttributeError(name) + + a = A() + with self.assertRaises(AttributeError) as e: + a.spam = "eggs" + + self.assertEqual(e.exception.args, ("spam",)) + + with self.assertRaises(AttributeError) as e: + del a.spam + + self.assertEqual(e.exception.args, ("spam",)) + + def test_descr_set(self): + class A: + def __set__(self, inst, value): + raise AttributeError + + def __delete__(self, instance): + raise AttributeError + + class B: + foo = A() + + b = B() + with self.assertRaises(AttributeError) as e: + b.foo = "bar" + + self.assertEqual(e.exception.args, self.err_msg_set("B", "foo", "set")) + + with self.assertRaises(AttributeError) as e: + del b.foo + + self.assertEqual(e.exception.args, self.err_msg_set("B", "foo", "delete")) + + def test_meta_descr_set(self): + class M(type): + def __set__(self, inst, value): + raise AttributeError + + def __delete__(self, inst): + raise AttributeError + + class N(metaclass=M): + pass + + class A: + bar = N + + a = A() + + with self.assertRaises(AttributeError) as e: + a.bar = "baz" + + self.assertEqual(e.exception.args, self.err_msg_set("A", "bar", "set")) + + with self.assertRaises(AttributeError) as e: + del a.bar + + self.assertEqual(e.exception.args, self.err_msg_set("A", "bar", "delete")) + + def test_descr_meta_set(self): + class A: + def __set__(self, inst, value): + raise AttributeError + + def __delete__(self, inst): + raise AttributeError + + class M(type): + spam = A() + + class N(metaclass=M): + pass + + with self.assertRaises(AttributeError) as e: + N.spam = "scrambled eggs" + + self.assertEqual(e.exception.args, self.err_msg_set("M", "spam", "set")) + + with self.assertRaises(AttributeError) as e: + del N.spam + + self.assertEqual(e.exception.args, self.err_msg_set("M", "spam", "delete")) class ImportErrorTests(unittest.TestCase): diff --git a/Objects/object.c b/Objects/object.c --- a/Objects/object.c +++ b/Objects/object.c @@ -789,8 +789,14 @@ PyObject_GetAttrString(PyObject *v, const char *name) { PyObject *w, *res; - if (Py_TYPE(v)->tp_getattr != NULL) - return (*Py_TYPE(v)->tp_getattr)(v, (char*)name); + if (Py_TYPE(v)->tp_getattr != NULL) { + res = (*Py_TYPE(v)->tp_getattr)(v, (char*)name); + if (res == NULL && PyErr_NoArgs(PyExc_AttributeError)) + PyErr_Format(PyExc_AttributeError, + "'%.50s' object has no attribute '%s'", + Py_TYPE(v)->tp_name, name); + return res; + } w = PyUnicode_InternFromString(name); if (w == NULL) return NULL; @@ -817,8 +823,14 @@ PyObject_SetAttrString(PyObject *v, const char *name, PyObject *w) PyObject *s; int res; - if (Py_TYPE(v)->tp_setattr != NULL) - return (*Py_TYPE(v)->tp_setattr)(v, (char*)name, w); + if (Py_TYPE(v)->tp_setattr != NULL) { + res = (*Py_TYPE(v)->tp_setattr)(v, (char*)name, w); + if (res < 0 && PyErr_NoArgs(PyExc_AttributeError)) + PyErr_Format(PyExc_AttributeError, + "cannot %s attribute '%s' of '%.100s' object", + w==NULL ? "delete" : "set", name, Py_TYPE(v)->tp_name); + return res; + } s = PyUnicode_InternFromString(name); if (s == NULL) return -1; @@ -885,6 +897,7 @@ _PyObject_SetAttrId(PyObject *v, _Py_Identifier *name, PyObject *w) PyObject * PyObject_GetAttr(PyObject *v, PyObject *name) { + PyObject *res = NULL; PyTypeObject *tp = Py_TYPE(v); if (!PyUnicode_Check(name)) { @@ -894,17 +907,24 @@ PyObject_GetAttr(PyObject *v, PyObject *name) return NULL; } if (tp->tp_getattro != NULL) - return (*tp->tp_getattro)(v, name); - if (tp->tp_getattr != NULL) { + res = (*tp->tp_getattro)(v, name); + else if (tp->tp_getattr != NULL) { char *name_str = _PyUnicode_AsString(name); if (name_str == NULL) return NULL; - return (*tp->tp_getattr)(v, name_str); + res = (*tp->tp_getattr)(v, name_str); } - PyErr_Format(PyExc_AttributeError, - "'%.50s' object has no attribute '%U'", - tp->tp_name, name); - return NULL; + if (res == NULL) { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_AttributeError, + "'%.50s' object has no attribute '%U'", + tp->tp_name, name); + else if (PyErr_NoArgs(PyExc_AttributeError)) + PyErr_FormatWithTraceback(PyExc_AttributeError, + "'%.50s' object has no attribute '%U'", + tp->tp_name, name); + } + return res; } int @@ -936,16 +956,14 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) PyUnicode_InternInPlace(&name); if (tp->tp_setattro != NULL) { err = (*tp->tp_setattro)(v, name, value); - Py_DECREF(name); - return err; + goto end; } if (tp->tp_setattr != NULL) { char *name_str = _PyUnicode_AsString(name); if (name_str == NULL) return -1; err = (*tp->tp_setattr)(v, name_str, value); - Py_DECREF(name); - return err; + goto end; } Py_DECREF(name); assert(name->ob_refcnt >= 1); @@ -964,6 +982,17 @@ PyObject_SetAttr(PyObject *v, PyObject *name, PyObject *value) value==NULL ? "del" : "assign to", name); return -1; + + end: + Py_DECREF(name); + assert(name->ob_refcnt >= 1); + if (err < 0 && PyErr_NoArgs(PyExc_AttributeError)) + PyErr_FormatWithTraceback(PyExc_AttributeError, + "cannot %s attribute '%U' of '%.100s' object", + value==NULL ? "delete" : "set", + name, + tp->tp_name); + return err; } /* Helper to get a pointer to an object's __dict__ slot, if any */ @@ -1109,13 +1138,19 @@ _PyObject_GenericGetAttrWithDict(PyObject *obj, PyObject *name, PyObject *dict) if (descr != NULL) { res = descr; descr = NULL; - goto done; } - PyErr_Format(PyExc_AttributeError, - "'%.50s' object has no attribute '%U'", - tp->tp_name, name); done: + if (res == NULL) { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_AttributeError, + "'%.50s' object has no attribute '%U'", + tp->tp_name, name); + else if (PyErr_NoArgs(PyExc_AttributeError)) + PyErr_FormatWithTraceback(PyExc_AttributeError, + "'%.50s' object has no attribute '%U'", + tp->tp_name, name); + } Py_XDECREF(descr); Py_DECREF(name); return res; @@ -1168,11 +1203,6 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, "'%.100s' object has no attribute '%U'", tp->tp_name, name); } - else { - PyErr_Format(PyExc_AttributeError, - "'%.50s' object attribute '%U' is read-only", - tp->tp_name, name); - } goto done; } res = _PyObjectDict_SetItem(tp, dictptr, name, value); @@ -1186,9 +1216,23 @@ _PyObject_GenericSetAttrWithDict(PyObject *obj, PyObject *name, Py_DECREF(dict); } if (res < 0 && PyErr_ExceptionMatches(PyExc_KeyError)) - PyErr_SetObject(PyExc_AttributeError, name); + PyErr_Clear(); /* exception is set below */ done: + if (res < 0) { + if (!PyErr_Occurred()) + PyErr_Format(PyExc_AttributeError, + "cannot %s attribute '%U' of '%.100s' object", + value==NULL ? "delete" : "set", + name, + tp->tp_name); + else if (PyErr_NoArgs(PyExc_AttributeError)) + PyErr_FormatWithTraceback(PyExc_AttributeError, + "cannot %s attribute '%U' of '%.100s' object", + value==NULL ? "delete" : "set", + name, + tp->tp_name); + } Py_XDECREF(descr); Py_DECREF(name); return res; diff --git a/Python/errors.c b/Python/errors.c --- a/Python/errors.c +++ b/Python/errors.c @@ -148,6 +148,14 @@ PyErr_SetString(PyObject *exception, const char *string) Py_XDECREF(value); } +void +PyErr_SetWithTraceback(PyObject *type, PyObject *obj) +{ + PyObject *exc, *value, *tb; + PyErr_Fetch(&exc, &value, &tb); + PyErr_Restore(type, obj, tb); + +} PyObject * PyErr_Occurred(void) @@ -342,6 +350,37 @@ PyErr_Clear(void) PyErr_Restore(NULL, NULL, NULL); } +int +PyErr_NoArgs(PyObject *type) +{ + PyObject *exc, *value, *tb; + PyBaseExceptionObject *obj; + int ret = 0; + + PyErr_Fetch(&exc, &value, &tb); + if (exc == NULL || exc != type || + (value != NULL && PyUnicode_CheckExact(value))) { + PyErr_Restore(exc, value, tb); + return 0; + } + + if (value == NULL && tb == NULL) { + PyErr_Restore(exc, value, tb); + return 1; + } + + obj = (PyBaseExceptionObject *)value; + if (obj->args == NULL) + ret = 1; + else if (PyTuple_Check(obj->args) && PyTuple_GET_SIZE(obj->args) <= 1) + ret = (PyTuple_GET_SIZE(obj->args) == 0 || + PyTuple_GET_ITEM(obj->args, 0) == Py_None); + + PyErr_Restore(exc, value, tb); + return ret; + +} + void PyErr_GetExcInfo(PyObject **p_type, PyObject **p_value, PyObject **p_traceback) { @@ -795,6 +834,33 @@ PyErr_Format(PyObject *exception, const char *format, ...) return NULL; } +PyObject * +PyErr_FormatVWithTraceback(PyObject *exception, const char *format, va_list vargs) +{ + PyObject *exc, *value, *tb; + PyObject *string; + + PyErr_Fetch(&exc, &value, &tb); + + string = PyUnicode_FromFormatV(format, vargs); + + PyErr_Restore(exception, string, tb); + return NULL; +} + +PyObject * +PyErr_FormatWithTraceback(PyObject *exception, const char *format, ...) +{ + va_list vargs; +#ifdef HAVE_STDARG_PROTOTYPES + va_start(vargs, format); +#else + va_start(vargs); +#endif + PyErr_FormatVWithTraceback(exception, format, vargs); + va_end(vargs); + return NULL; +} PyObject * PyErr_NewException(const char *name, PyObject *base, PyObject *dict)