[issue39865] getattr silences an unrelated AttributeError

[Raymond Hettinger]

Raymond Hettinger  added the comment:

Am going to mark this as closed for the reasons listed above.  If another 
coredev wants to champion this, feel free to resurrect the issue.

--
resolution:  -> rejected
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[Raymond Hettinger]

Raymond Hettinger  added the comment:

My instincts are to leave this alone and not gum up heavily trafficked core 
business logic.  I don't like the external calls, the extra increfs and decrefs 
(and possible rentrancy issues), performance impact, or the pattern of holding 
the exception across a call that can invoke arbitrary code.   The code for 
slot_tp_getattr_hook() is currently very clean and it would be nice to keep it 
that way.

ISTM that the problem is better addressed by unittesting, linting, and static 
type checking rather than by gumming-up runtime.

--
nosy: +tim.peters
versions:  -Python 2.7, Python 3.5, Python 3.6, Python 3.7, Python 3.8

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[Ammar Askar]

Ammar Askar  added the comment:

Update: opened up https://github.com/python/cpython/pull/19303 as a quick first 
pass at implementing this. It works as expected and retains the context from 
the original exception just in case it's needed. The code isn't too pretty, 
looks like exception chaining was primarily designed to be a first class 
citizen using the `raise e2 from e` syntax. A helper in exceptions.c would 
definitely go a long way to making it more readable.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[Ammar Askar]

Change by Ammar Askar :


--
keywords: +patch
pull_requests: +18665
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/19303

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[Ammar Askar]

Ammar Askar  added the comment:

As unfortunate as this is, I don't think there's an easy way to solve this 
while adhering to descriptors and the attribute lookup model. This is a 
consequence of the following rule:

> object.__getattr__(self, name):
>   Called when the default attribute access fails with an
>   AttributeError (either __getattribute__() raises an AttributeError
>   because name is not an instance attribute or an attribute in the
>   class tree for self; or __get__() of a name property raises
>   AttributeError)

As it notes, if the __get__() raises an AttributeError then a fallback to 
__getattr__ is initiated. One may think that maybe we can just catch 
AttributeError in @property to try to fix this problem but a quick search shows 
that people do intentionally raise AttributeError in @property methods:

* 
https://github.com/kdschlosser/EventGhost-TPLink/blob/a4a642fde8dd4deba66262a36d673cbbf71b8ceb/TPLink/tp_link/rule.py#L148-L152

* 
https://github.com/ajayau404/sniffer/blob/cd0c813b8b526a3c791735a41b13c7677eb4aa0e/lib/python3.5/site-packages/vpython/vpython.py#L1942-L1944

While this in combination with a __getattr__ is rare, I was able to find one 
example:

* 
https://github.com/xrg/behave_manners/blob/19a5feb0b67fe73cd902a959f0d038b905a69b38/behave_manners/context.py#L37

I don't think that changing this behavior is acceptable as people might be 
relying on it and it's well documented.


In your case, I think it's really the "catch-all" __getattr__ that's at fault 
here which really shouldn't be returning None for all attributes blindly. This 
does bring up a good point though that in the process of this fall-back 
behavior, the original AttributeError from A's property does get masked. What 
can be done and might be a good idea is to show the original AttributeError 
failure as the cause for the second. Something like this:

  Traceback (most recent call last):
File "", line 2, in 
File "", line 5, in myprop
  AttributeError: 'int' object has no attribute 'foo'

  The above exception was the direct cause of the following exception:

  Traceback (most recent call last):
File "", line 7, in 
File "", line 5, in 
File "", line 7, in __getattr__
  AttributeError: not found in __getattr__

This at least somewhat indicates that the original descriptor __get__ failed 
and then __getattr__ also raised. As opposed to now where the original 
exception gets masked:

  >>> class A:
  ...   @property
  ...   def myprop(self):
  ... a = 1
  ... a.foo
  ...   def __getattr__(self, attr_name):
  ... raise AttributeError("not found in __getattr__")
  ...
  >>> a = A()
  >>> a.myprop
  Traceback (most recent call last):
File "", line 1, in 
File "", line 7, in __getattr__
  AttributeError: not found in __getattr__

I'm gonna take a stab at implementing this real quick to see if it's actually 
useful and viable.

--
nosy: +ammar2

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[Raymond Hettinger]

Change by Raymond Hettinger :


--
nosy: +rhettinger

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue39865] getattr silences an unrelated AttributeError

[pasenor]

New submission from pasenor :

if a class has a descriptor and a defined __getattr__ method, and an 
AttributeError (unrelated to the descriptor lookup) is raised inside the 
descriptor, it will be silenced:

class A:
@property
def myprop(self):
print("property called")
a = 1
a.foo  # <-- AttributeError that should not be silenced

def __getattr__(self, attr_name):
print("__getattr__ called")

a = A()
a.myprop


In this example myprop() is called, the error silenced, then __getattr__() is 
called.
This can lead to rather subtle bugs. Probably an explicit AttributeError should 
be raised instead.

--
messages: 363449
nosy: pasenor
priority: normal
severity: normal
status: open
title: getattr silences an unrelated AttributeError
versions: Python 2.7, Python 3.5, Python 3.6, Python 3.7, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com