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

bpo-39865: Don't lose exception context when __getattr__ is present #19303

Closed
wants to merge 1 commit into from

Conversation

ammaraskar
Copy link
Member

@ammaraskar ammaraskar commented Apr 2, 2020

When a class has a __getattr__ hook, it attempts to retrieve the attribute (e.g through a descriptor or the __dict__). If this initial attempt fails with AttributeError then a fallback is attempted on __getattr__. In turn, if the __getattr__ raises another AttributeError, then the original exception that caused the fallback is lost. This PR attempts to surface that information to avoid subtle bugs that might be caused by this.

The motivating example for this, as taken from the test is:

class A:
    @property
    def foo(self):
        a = 0
        a.accidental_attribute_error
        return 42
    def __getattr__(self, attr):
        if attr == 'foo':
            raise AttributeError("foo not supported in getattr")
        else:
            return 'fallback'

In this case, this class supports a.foo through a descriptor/property but has a bug in it. Previously attempting to access a.foo would lead to this traceback:

Traceback (most recent call last):
  File "lib\test\test_descr.py", line 2301, in test_property_fallback_exception
    a.foo
  File "lib\test\test_descr.py", line 2294, in __getattr__
    raise AttributeError("foo not supported in getattr")
AttributeError: foo not supported in getattr

This masks the original exception that caused the lookup to be delegated to __getattr__. However with the right cause set, the error message now looks like:

  File "lib\test\test_descr.py", line 2290, in foo
    a.accidental_attribute_error
AttributeError: 'int' object has no attribute 'accidental_attribute_error'

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

Traceback (most recent call last):
  File "lib\test\test_descr.py", line 2301, in test_property_fallback_exception
    a.foo
  File "lib\test\test_descr.py", line 2294, in __getattr__
    raise AttributeError("foo not supported in getattr")
AttributeError: foo not supported in getattr

which makes the real root cause of the bug a lot easier to diagnose. There's also external reason to believe that this might be a problem:


The code to set the cause could definitely use a helper in the PyErr_ namespace. It's pretty ugly as it stands now.

https://bugs.python.org/issue39865

@rhettinger
Copy link
Contributor

Am marking this as rejected for the reasons listed in the bpo.

That said, I want to comment that this was really nice work. You did your homework and a good job on the PR. The only bone of contention is that the net cost/benefit suggests just leaving this alone.

@rhettinger rhettinger closed this Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants