Raising an exception from a property silently dispatches to wrapped property, instead of raising the exception

[apirogov]

Consider following code:

class Foo:

    @property
    def bar(self):
        return "value"

class WrappedFoo(wrapt.ObjectProxy):

    @property
    def bar(self):
        raise ValueError("exception")

Now WrappedFoo(Foo()).bar == "value", even though I want the exception to be raised.

Is this a bug? Or is this expected? Is there some way I can get the desired behaviour?

Thanks in advance!

[GrahamDumpleton]

It possibly relates to a deficiency in the Python C APIs whereby there is no equivalent to hasattr(). There are certain cases when accessing attributes that if an exception occurs it is not possible to tell the difference between the attribute not existing and it being a code error.

If you use the code:

import wrapt
import traceback

class Foo:

    @property
    def bar(self):
        return "value"

class WrappedFoo(wrapt.ObjectProxy):

    @property
    def bar(self):
        print("XXX")
        traceback.print_stack()
        raise ValueError("exception")

print(WrappedFoo(Foo()).bar == "value")

then XXX is printed, indicating that it was actually called.

XXX
  File "/private/tmp/wrapt-tests/check.py", line 18, in <module>
    print(WrappedFoo(Foo()).bar == "value")
  File "/private/tmp/wrapt-tests/check.py", line 15, in bar
    traceback.print_stack()
True

If you run the same code, but this time disable the use of the C API based extension by doing WRAPT_DISABLE_EXTENSIONS=true python check.py, you instead get:

XXX
  File "/private/tmp/wrapt-tests/check.py", line 18, in <module>
    print(WrappedFoo(Foo()).bar == "value")
  File "/private/tmp/wrapt-tests/check.py", line 15, in bar
    traceback.print_stack()
Traceback (most recent call last):
  File "/private/tmp/wrapt-tests/check.py", line 18, in <module>
    print(WrappedFoo(Foo()).bar == "value")
  File "/private/tmp/wrapt-tests/check.py", line 16, in bar
    raise ValueError("exception")
ValueError: exception

So for pure Python implementation of wrapt you see what you expect, but I suspect when using the C API based extension it is hitting this problem with lack of C API for hasattr() which means that parts of the C API sees the exception and thinks it means the attribute doesn't exist and so it falls back to using the attribute from the wrapped object.

To write my own implementation of what was needed using C APIs is far from trivial and what you need to do would differ for different Python versions, making it even harder to do and support. Anyway, I can't remember the actual details and I can't find a specific GitHub issue about it where I have captured the information. I'll try and refresh my memory about where in the usage of the C APIs the problem arises and add some more notes here about it.

[GrahamDumpleton]

Will have to do some more testing but I think it is this function:

static PyObject *WraptObjectProxy_getattro(
        WraptObjectProxyObject *self, PyObject *name)
{
    PyObject *object = NULL;
    PyObject *result = NULL;

    static PyObject *getattr_str = NULL;

    object = PyObject_GenericGetAttr((PyObject *)self, name);

    if (object)
        return object;

    PyErr_Clear();

    if (!getattr_str) {
#if PY_MAJOR_VERSION >= 3
        getattr_str = PyUnicode_InternFromString("__getattr__");
#else
        getattr_str = PyString_InternFromString("__getattr__");
#endif
    }

    object = PyObject_GenericGetAttr((PyObject *)self, getattr_str);

    if (!object)
        return NULL;

    result = PyObject_CallFunctionObjArgs(object, name, NULL);

    Py_DECREF(object);

    return result;
}

I can only try and access the attribute using PyObject_GenericGetAttr() as no way to check its existence.

If it doesn't exist it would raise an AttributeError. The problem is that a property could instead be called with the function internally raising AttributeError. There is no way to distinguish between then two. The code therefore unfortunately assumes that an error occurring indicates it didn't exist.

The code could be made to specifically look for AttributeError to narrow down the erroneous cases, which would help in this case of a ValueError, but would still have the same issue if the property function raised an AttributeError instead.

[GrahamDumpleton]

I should clarify there does exist PyObject_HasAttr() but there must have been something about it which was problematic when used in this situation. Note that it isn't named PyObject_GenericHasAttr() and no such function exists, so I suspect it looks up attributes in a totally different way than what PyObject_GenericGetAttr() does to look up and access the object so would behave quite differently.

I will try do some more tests but I may still not have a good solution.

[GrahamDumpleton]

So as explained above, what I could do is:

    object = PyObject_GenericGetAttr((PyObject *)self, name);

    if (object)
        return object;

    if (!PyErr_ExceptionMatches(PyExc_AttributeError))
        return NULL;

    PyErr_Clear();

but you would still get the wrong result if the Python code was:

class WrappedFoo(wrapt.ObjectProxy):

    @property
    def bar(self):
        print("XXX")
        traceback.print_stack()
        #raise ValueError("exception")
        raise AttributeError("exception")

But now he realises he may have been an idiot in not doing this all these years because the pure Python version doesn't work correctly for this anyway, so at least they would do the wrong thing in the same way for both pure Python and C extension version. I am not sure there is any way around that. :-(

[mentalisttraceur]

I think if a property raises AttributeError, it's totally fine to treat it as if it does not exist.

After all, that's how we say "this attribute does not exist" in Python. And unlike normal attributes, which can be deleted from an instance or never set to being with, a property's descriptor is always on the class, so it can only behave as-if it's not there by raising an AttributeError.

So:

• Properties implemented with thoroughly careful code would only let AttributeError reach you if they want to be treated as if they do not exist.

  If they encounter an internal AttributeError they can't handle, they would use exception chaining or wrap it to re-raise it as another exception type.

• Any other AttributeError bubbling up from a property is wrong use of that protocol - not your fault if you follow the protocol correctly.

  Like when an iterator raises a StopIteration - we don't worry if it was intentional or premature due to careless use of another iterator internally, we just follow the protocol and stop iterating, and if that wasn't intended, that's the iterator's fault.

[apirogov]

@GrahamDumpleton thank you very much for your quick and thorough reply!

I think I agree with @mentalisttraceur about the case with AttributeError being one where this actually is ok due to the semantics of this concrete exception.

If a fix in the C extension would work for everything except for AttributeError, and this quirk is documented, I think that would be a reasonable and useful solution!

My temporary work-around until then is to just return None (which works fine), making it break silently in code consuming the wrapped objects. It is not nice, but at least it does effectively restrict accidental access to the "forbidden" properties.

[GrahamDumpleton]

Please re-evaluate your code against version 1.15.0 of wrapt. Thanks.

[apirogov]

Thanks a lot! Now it works as expected :)