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

Usage of both @property and __getattr__ can lead to obscure and hard to debug errors #2108

Closed
skrivanos opened this issue Nov 21, 2014 · 9 comments · Fixed by #2530
Closed
Labels
Milestone

Comments

@skrivanos
Copy link

Encountered this error when accessing request.DATA in a view:

  File "views.py", line 146, in put
    if request.DATA is not None:
  File "/usr/local/lib/python2.7/dist-packages/rest_framework/request.py", line 453, in __getattr__
    return getattr(self._request, attr)
AttributeError: 'WSGIRequest' object has no attribute 'DATA'

This was mind-boggling. I couldn't understand why __getattr__ was ever called on the request object, since a property called DATA already exists (confirmed this via dir(request) before accessing DATA), and as we all know, getattr is only used for undefined properties.

After an intense debugging session it turned out that this is caused by a "feature" in python. This may be common knowledge, but it definitely was new news to me: if a property getter raises an AttributeError, python (__getattribute__) falls back to using __getattr__. The issue is that the traceback is lost, which makes it extremely hard to debug, since the actual exception raised isn't shown anywhere. So, the actual error was:

AttributeError: 'TemporaryFileUploadHandler' object has no attribute 'file'

Further explanation/discussion: https://groups.google.com/forum/#!topic/comp.lang.python/BZf-d0rLP8U

I'm not sure if this is an issue that you can/want to address, but I wanted to make people aware of it since it was a pain to track down and others might stumble into the same situation.

@tomchristie
Copy link
Member

Assuming related to #1939. Not clear if something similar to #1938 would fix.

@skrivanos
Copy link
Author

Yes, definitely the same error. Pretty sure the same solution applied in #1938 would fix it. Probably needs fixing in all getters/setters in classes with custom properties and getattr defined.

Another option might be to override __getattribute__ instead of __getattr__ and handle it there instead as that'd be more DRY.

@clintonb
Copy link
Contributor

clintonb commented Feb 4, 2015

Any update on this issue?

@tomchristie
Copy link
Member

Not yet. Plenty of other things going on, but anyone's welcome to dig into this and come up with clear proposal/example pull request etc.

@foresmac
Copy link

foresmac commented Feb 6, 2015

Dang, I'm having the same problem but I can't trace it.

@tomchristie
Copy link
Member

One route someone to start on this would be to demonstrate the most minimal possible python example that demonstates the issue (ie forget about REST framework at all to start with)

That'd give us a much better point to work towards a fix from, as it'd narrow things down to the core issue.

@skrivanos
Copy link
Author

class Test(object):
    @property
    def foo(self):
        # This will raise an AttributeError, after which
        # __getattr__ will be used as a fallback.
        return self.doesntexist

    def __getattr__(self, name):
        raise AttributeError('No attribute ' + name)

t = Test()
t.foo

The output will be

Traceback (most recent call last):
  File "2108.py", line 12, in <module>
    t.foo
  File "2108.py", line 9, in __getattr__
    raise AttributeError('No attribute ' + name)
AttributeError: No attribute foo

As you can see, the original exception and traceback is lost (from trying to access self.doesntexist).

@tomchristie
Copy link
Member

Now resolved. Thanks everyone for your input on this! :)

@foresmac
Copy link

foresmac commented Feb 9, 2015

I finally found my problem; I was using settings.AUTH_USER_MODEL where I should have been using get_user_model(). So the actual error was that I was trying to call .objects.get() on a string object instead of the user class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants