Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PyObject_GenericGetAttr suppresses AttributeErrors in descriptors #45956

Closed
gangesmaster mannequin opened this issue Dec 13, 2007 · 16 comments
Closed

PyObject_GenericGetAttr suppresses AttributeErrors in descriptors #45956

gangesmaster mannequin opened this issue Dec 13, 2007 · 16 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@gangesmaster
Copy link
Mannequin

gangesmaster mannequin commented Dec 13, 2007

BPO 1615
Nosy @rhettinger, @pitrou, @benjaminp, @merwok, @durban, @ethanfurman, @ericsnowcurrently
Files
  • demo.txt
  • getattr-descriptors.diff
  • issue1615.stoneleaf.01.patch
  • issue1615.stoneleaf.02.patch
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/ethanfurman'
    closed_at = <Date 2015-07-21.08:15:01.637>
    created_at = <Date 2007-12-13.20:13:37.399>
    labels = ['interpreter-core', 'type-bug', 'invalid']
    title = 'PyObject_GenericGetAttr suppresses AttributeErrors in descriptors'
    updated_at = <Date 2015-07-21.08:15:01.636>
    user = 'https://bugs.python.org/gangesmaster'

    bugs.python.org fields:

    activity = <Date 2015-07-21.08:15:01.636>
    actor = 'ethan.furman'
    assignee = 'ethan.furman'
    closed = True
    closed_date = <Date 2015-07-21.08:15:01.637>
    closer = 'ethan.furman'
    components = ['Interpreter Core']
    creation = <Date 2007-12-13.20:13:37.399>
    creator = 'gangesmaster'
    dependencies = []
    files = ['8943', '29167', '32153', '34657']
    hgrepos = []
    issue_num = 1615
    keywords = ['patch']
    message_count = 16.0
    messages = ['58581', '61312', '61321', '76825', '77424', '116804', '116805', '158556', '182210', '182717', '197991', '198158', '200108', '200109', '215018', '215092']
    nosy_count = 11.0
    nosy_names = ['rhettinger', 'pitrou', 'gangesmaster', 'thomaslee', 'benjamin.peterson', 'eric.araujo', 'Arfrever', 'daniel.urban', 'ethan.furman', 'eric.snow', 'Micah.Friesen']
    pr_nums = []
    priority = 'normal'
    resolution = 'not a bug'
    stage = None
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue1615'
    versions = ['Python 3.5']

    @gangesmaster
    Copy link
    Mannequin Author

    gangesmaster mannequin commented Dec 13, 2007

    it seems the code of PyObject_GenericGetAttr, which invokes the
    descriptor protocol, silences any AttributeErrors raised by the
    descriptor, for classes that also define __getattr__. it should
    propagate up rather than being silently ignored.

    the attached example is quite artificial, but it's a simplification of
    real world code i had hard time debugging. turned out i misspelled an
    attribute name inside the property getter function, which raised an
    AttributeError as expected -- but the exception i got was quite
    misleading, saying the instance has no attribute named so.

    this bug only happens when the class defines a custom __getattr__. see
    attached demo file for details.

    @gangesmaster gangesmaster mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Dec 13, 2007
    @pitrou
    Copy link
    Member

    pitrou commented Jan 20, 2008

    I can confirm that with SVN trunk, and it's actually even worse because
    it can return unexpected results without raising an exception at all:

    >>> class Foo(object):
    ...   def __getattr__(self, name): return 42
    ...   @property
    ...   def bacon(self): return int.lalala
    ... 
    >>> f = Foo()
    >>> f.bacon
    42
    >>> Foo.bacon.__get__(f)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 4, in bacon
    AttributeError: type object 'int' has no attribute 'lalala'

    @pitrou
    Copy link
    Member

    pitrou commented Jan 20, 2008

    PyObject_GenericGetAttr is invoked from slot_tp_getattr_hook in
    typeobject.c via tp_getattro. The problem is that, when tp_getattro
    returns with an AttributeError, there is no way for slot_tp_getattr_hook
    to know whether the error was raised by PyObject_GenericGetAttr itself
    or by subsequent invocation of user code. Perhaps by adding an attribute
    to the raised AttributeError?

    @gangesmaster
    Copy link
    Mannequin Author

    gangesmaster mannequin commented Dec 3, 2008

    here's a short example of the bug

    >>> class Foo(object):
    ...   def __getattr__(self, name): 
    ...     return 42
    ...   @property
    ...   def bacon(self): 
    ...     return int.lalala
    ...   @property
    ...   def eggs(self): 
    ...     return 17
    ... 
    >>> f = Foo()
    >>> f.bacon   # raises an AttributeError, and silently ignores it
    42
    >>> f.eggs
    17
    >>> 

    are there any news in this front?

    @thomaslee
    Copy link
    Mannequin

    thomaslee mannequin commented Dec 9, 2008

    Related reading from a few years back:

    http://coding.derkeiler.com/Archive/Python/comp.lang.python/2005-05/msg03829.html

    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Sep 18, 2010

    This is still an issue with the latest trunk.

    @benjaminp
    Copy link
    Contributor

    I consider this to be a feature. Properties can raise AttributeError to defer to __getattr__.

    @MicahFriesen
    Copy link
    Mannequin

    MicahFriesen mannequin commented Apr 17, 2012

    I ran into this recently, as well, and have lost probably a day's worth of time debugging it. I submit that this is not a feature - I can't imagine a real-world scenario where you actually want to write debuggable code where a descriptor defers to __getattr__ (except perhaps for exception handling, in which case some re-factoring is in order), particularly because descriptors are effectively mix-ins and can be used on multiple classes.

    I worked around this by writing an ancestor descriptor that catches AttributeErrors and re-raises them as a user-defined exception.

    @ericsnowcurrently
    Copy link
    Member

    Got bit by a variation of this today in 2.7:

    class Spam(object):
        def __getattr__(self, attr):
            raise AttributeError(attr)
        @property
        def eggs(self):
            return self.ham
    
    s = Spam()
    s.eggs
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "<stdin>", line 3, in __getattr__
    AttributeError: eggs

    It took me a little while to figure out what was going on. A real head-scratcher. This is because the AttributeError was attributed to the property, but was actually caused by the __getattr__ call triggered by the property's code. I would expect the AttributeError to reference "ham", not "eggs". As already noted, if __getattr__() is not there, that's what happens.

    Regardless, I'm just not seeing where the hurdle is to improving this behavior. I certainly agree that this is not a feature. It is the source of very mysterious failures.

    I was surprised that AttributeError does not have an attribute to which the name would be bound. If it did, then slot_tp_getattr_hook() could check against that:

        if (res == NULL && PyErr_ExceptionMatches(PyExc_AttributeError)) {
            PyObject *tp, *exc, *tb, *exc_attr;
    
            PyErr_Fetch(&tp, &exc, &tb);
            exc_attr = PyObject_GetAttrString(exc, "attribute");
            PyErr_Restore(tp, exc, tb);
    
            if (!exc_attr || exc_attr == name) { 
                PyErr_Clear();
                res = call_attribute(self, getattr, name);
            }
            Py_XDECREF(exc_attr);
        }

    Alternatively, when an AttributeError comes out of a getter in _PyObject_GenericSetAttrWithDict() (in either spot they're called), another exception (not AttributeError) could be raised with the original chained onto it. Then slot_tp_getattr_hook() won't silently ignore it. It would be something like this:

            if (f != NULL && PyDescr_IsData(descr)) {
                res = f(descr, obj, value);
                if (!res && PyErr_ExceptionMatches(PyExc_AttributeError)) {
                    PyObject *msg = PyUnicode_FromFormat("getter failed for '%U'", name);
                    /* implicit chaining here */
                    PyErr_SetObject(PyExc_???Error, msg);
                }
                goto done;
            }

    Conceptually, it's as though locating the attribute and extracting the value are lumped together here. Distinguishing the two would help make this failure situation much less confusing.

    Additionally, it would be really helpful to have a brief mention of this behavior (AttributeErrors in getters falling back to __getattr__) in the language reference entry for __getattr__ and/or descriptors.

    @ericsnowcurrently
    Copy link
    Member

    The whole AttributeError gaining an attribute seems impractical, but the second approach still seems reasonable to me. I've attached a rough patch to demonstrate. If that looks okay, I can flesh it out.

    @ethanfurman
    Copy link
    Member

    Benjamin Peterson added the comment:

    I consider this to be a feature. Properties can raise AttributeError to defer to
    __getattr__.

    Micah Friesen added the comment:

    I submit that this is not a feature - I can't imagine a real-world scenario [...]

    No need to imagine. The new Enum class uses this ability to support having both protected properties on enum members and enum members of the same name:

    --> from enum import Enum
    --> class Field(Enum):
    --> name = 1
    ... age = 2
    ... location = 3
    ...
    --> Field.name
    <Field.name: 1>
    --> Field.name.name
    'name'

    Enum's custom __getattr__ is located in the metaclass, however, not in the class. For future reference, here is a short test script and it's output in 3.4.0a2+:
    =====================================================================================

    class WithOut:
        @property
        def huh(self):
            return self.not_here
    
    class With:
        @property
        def huh(self):
            return self.not_here
        def __getattr__(self, name):
            print('looking up %s' % name)
            raise AttributeError('%s not in class %s' % (name, type(self)))

    try:
    WithOut().huh
    except AttributeError as exc:
    print(exc)

    print()
    try:
        With().huh
    except AttributeError as exc:
        print(exc)
    
    print()
    import enum  # broken value property tries to access self.not_here
    class TestEnum(enum.Enum):
        one = 1
    
    print(TestEnum.one.value)

    =====================================================================================
    'WithOut' object has no attribute 'not_here'

    looking up not_here
    looking up huh
    huh not in class <class '__main__.With'>

    meta getattr with __new_member__
    meta getattr with __new_member__
    meta getattr with one
    'TestEnum' object has no attribute 'not_here'
    =====================================================================================

    @rhettinger rhettinger self-assigned this Sep 20, 2013
    @rhettinger
    Copy link
    Contributor

    Marking this for Python 3.4. It isn't a bug in the descriptor protocol; rather, it is an implementation detail that is sometimes helpful but is mostly annoying.

    @rhettinger rhettinger changed the title descriptor protocol bug PyObject_GenericGetAttr suppresses AttributeErrors in descriptors Sep 20, 2013
    @rhettinger rhettinger removed their assignment Oct 17, 2013
    @ethanfurman
    Copy link
    Member

    Well, attached patch doesn't segfault in debug mode, but the errors aren't any better; in fact, I'd say their worse. Here's the current output from my test script:

    ===============================================================
    getter failed for descriptor 'huh'

    looking up not_here
    looking up huh
    huh not in class <class '__main__.With'>

    Traceback (most recent call last):
      File "break_getattr.py", line 30, in <module>
        print(TestEnum.one.missing)
    AttributeError: getter failed for descriptor 'missing'

    ===============================================================

    As you can see, we have even less information when a class level __getattr__ is /absent/, and when we do have one, there is no change (which is exactly where we really wanted the change). :(

    @ethanfurman
    Copy link
    Member

    If anyone with more experience wants ownership, feel free to take it from me. ;) Otherwise I'll do my best to get this figured out in time for the beta.

    @ethanfurman ethanfurman self-assigned this Oct 17, 2013
    @ethanfurman
    Copy link
    Member

    Results from the first two tests in my test script:
    --------------------------------------------------
    'WithOut' object has no attribute 'not_here'

    looking up not_here
    looking up huh
    'With' object has no attribute 'not_here'

    @ethanfurman
    Copy link
    Member

    Downside to this patch (stoneleaf.02) is that custom AttributeErrors raised in __getattr__ are overridden, which is a pretty severe regression.

    (Removed, renamed, and reloaded patch.)

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants