Issue1041

classification
Title: AttributeError raised from descriptors __get__ method are swallowed
Type: behaviour Severity: normal
Components: Core Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: leosoto Nosy List: leosoto, pjenvey
Priority: Keywords:

Created on 2008-05-25.20:47:55 by leosoto, last changed 2008-08-11.23:28:42 by leosoto.

Files
File name Uploaded Description Edit Remove
test_for_1041.patch leosoto, 2008-05-26.01:46:52
Messages
msg3204 (view) Author: Leonardo Soto (leosoto) Date: 2008-05-25.20:47:37
When accessing an instance attribute defined by a descriptor, any
AttributeError raised by the descriptor itself will be swallowed, and
replaced by the standard AttributeError:

Jython 2.3a0 on java1.6.0_06
Type "copyright", "credits" or "license" for more information.
>>> class Desc(object):
...     def __get__(self, instance, type):
...         raise AttributeError("Custom message")     
... 
>>> class Foo(object):
...     desc = Desc()
... 
>>> Foo.desc
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<stdin>", line 3, in __get__
AttributeError: Custom message
>>> Foo().desc
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Foo' object has no attribute 'desc'
msg3205 (view) Author: Leonardo Soto (leosoto) Date: 2008-05-25.21:07:56
As noted by Phillipe, the problem is on *Derived __findattr__:

    public PyObject __findattr__(String name) {
        PyType self_type=getType();
        PyObject getattribute=self_type.lookup("__getattribute__");
        PyString py_name=null;
        try {
            if (getattribute!=null) {
                return
getattribute.__get__(this,self_type).__call__(py_name=PyString.fromInterned(name));
            } else {
                return super.__findattr__(name);
            }
        } catch (PyException e) {
            if (Py.matchException(e,Py.AttributeError)) {
                PyObject getattr=self_type.lookup("__getattr__");
                if (getattr!=null)
                    try {
                        return
getattr.__get__(this,self_type).__call__(py_name!=null?py_name:PyString.fromInterned(name));
                    } catch (PyException e1) {
                        if (!Py.matchException(e1,Py.AttributeError))
                            throw e1;
                    }
                return null;
            }
            throw e;
        }
    }

The last "return null" is translating all the AttributeErrors into a
nulll return value, which is right from the perspective of the API
offered by PyObject#__findattr__, but causes this problem.

I would like to override PyObject#__getattr__ on *Derived (just as I
overrided __getitem__ on #1038), but it is marked final on PyObject. Why?
msg3207 (view) Author: Leonardo Soto (leosoto) Date: 2008-05-26.01:46:44
Attaching a patch adding a simple test for this problem
msg3279 (view) Author: Philip Jenvey (pjenvey) Date: 2008-06-13.01:46:24
this same bug is exercised in test_class. besides a small issue with 
garbage collection changing its expected output, this is the main issue 
preventing test_class from passing
msg3299 (view) Author: Leonardo Soto (leosoto) Date: 2008-06-22.06:33:14
As discussed on IRC I'm trying to change __findattr__ and __getattr__
roles, allowing *Derived classes to throw custom AttributeErrors without
a hack similar to what I did on #1038 (which does a double lookup when
it doesn't find anything).

I made the change on my hg repo
<https://hg.leosoto.com/jython.doj/rev/351f96f8e9cf>, and nothing seems
broken.

For consistency, I want to make a similar change for __finditem__ and
___getitem__ and then submit the patch here.

One thing I'm not sure about is the "__getattr__" name. As it is after
the change I made, it has the semantics of "__getattribute__". But then
we have "__setattr__", "__delattr__", etc, so I think it should keep the
"__getattr__" name, and a bit of documentation should avoid it be
misleading.
msg3304 (view) Author: Leonardo Soto (leosoto) Date: 2008-06-24.01:21:33
Forget the previous link/patch: it's wrong. I'm going to post a new
patch soon.
msg3394 (view) Author: Leonardo Soto (leosoto) Date: 2008-08-04.19:41:15
I should have commented this before, but my approach is flawed because
the cost of throwing an exception is too high.

The approach suggested by Samuele on
http://article.gmane.org/gmane.comp.lang.jython.devel/4489 sounds like
the right path to take.
msg3395 (view) Author: Leonardo Soto (leosoto) Date: 2008-08-04.19:41:58
#1095 is exactly the same problem
msg3418 (view) Author: Leonardo Soto (leosoto) Date: 2008-08-11.03:37:29
http://codereview.appspot.com/2888 should fix this
msg3426 (view) Author: Leonardo Soto (leosoto) Date: 2008-08-11.23:28:42
Fixed in r5155 (asm branch)
History
Date User Action Args
2008-08-11 23:28:42leosotosetstatus: open -> closed
resolution: fixed
messages: + msg3426
2008-08-11 03:37:29leosotosetmessages: + msg3418
2008-08-04 19:43:05leosotosetassignee: leosoto
2008-08-04 19:41:58leosotosetmessages: + msg3395
2008-08-04 19:41:15leosotosetmessages: + msg3394
2008-06-24 01:21:34leosotosetmessages: + msg3304
2008-06-22 06:34:02leosotosetmessages: + msg3299
2008-06-13 01:46:25pjenveysetnosy: + pjenvey
messages: + msg3279
2008-05-26 01:46:53leosotosetfiles: + test_for_1041.patch
messages: + msg3207
2008-05-25 21:08:04leosotosetmessages: + msg3205
2008-05-25 20:47:56leosotocreate