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

Authenticator class errors get lost #1939

Closed
harmon opened this issue Oct 11, 2014 · 5 comments
Closed

Authenticator class errors get lost #1939

harmon opened this issue Oct 11, 2014 · 5 comments
Labels

Comments

@harmon
Copy link

harmon commented Oct 11, 2014

I had a custom authenticator that was misconfigured, and it tried to import a settings.SOME_NAME when SOME_NAME did not exist in my settings module. This caused an AttributeError, so when the APIView tried to authenticate and called request.user, the Python property call swallowed this error, and gave me a "'WSGIRequest' object has no attribute 'successful_authenticator'" error instead, masking the real issue in my code.

I've got a pull request that demos this issue. It seems to happen on bad module references. I don't have a good solution, but maybe not having an authentication side effect inside a property definition might do the trick.

I'm happy to take a stab at a solution, but could use some guidance!

Pull Request: #1938

@tomchristie
Copy link
Member

Probably specific to AttributeError and something to do with how our Request attempts to proxy attributes not on that class to Django's underlying HTTPRequest instance.

Needs further investigation.

@tomchristie
Copy link
Member

Still seems valid to me, but not clear how we'd resolve.
Possible that we'd need to do something weird like inspecting the call stack when an AttributeError is raised, but unsure.

@harmon
Copy link
Author

harmon commented Nov 5, 2014

Hi Tom,

I've updated my pull request #1938 with a potential fix. I catch any AttributeErrors raised by the _authenticate() method and bubble up a RuntimeError with the details of the AttributeError. It's not the most error-prone approach (might be some odd edge cases for this odd edge-case), but I think it would help developers get to the bottom of the issue a lot quicker.

I thought about adding another catch for AttributeError here instead (https://github.com/harmon/django-rest-framework/blob/python-swallows-errors-in-properties/rest_framework/request.py#L405), which might be better, since it would cover the other properties that call _authenticate() internally within Request, but I didn't run into issues with those :P

It seems this is a problem some others have:

https://groups.google.com/forum/#!topic/comp.lang.python/BZf-d0rLP8U
http://blog.devork.be/2011/06/using-getattr-and-property_17.html

Thanks for your consideration. I'd be happy to try a different approach. I'm not the most experienced Python dev, so your thoughts are greatly appreciated.

@tomchristie
Copy link
Member

Assuming this report on the discussion group is for the same issue: https://groups.google.com/forum/#!topic/django-rest-framework/emldSe_gxfI

@tomchristie
Copy link
Member

Closing in favor of #2108.

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

No branches or pull requests

2 participants