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

Confusing handling of unexpected server response in fetch_token #302

Open
awbacker opened this issue Jan 4, 2018 · 1 comment
Open

Confusing handling of unexpected server response in fetch_token #302

awbacker opened this issue Jan 4, 2018 · 1 comment

Comments

@awbacker
Copy link

awbacker commented Jan 4, 2018

I am using fetch_token with the standard web client, but the server I am talking to returns a non-standard response (its just a text string saying 403/ bad auth ...).

The error I see is a MissingTokenError, rather than something more explanatory. This left me chasing my tail since I thought there was something wrong with the response, etc.

Ideas:

  • In the 403 case, re-raise that as an explicit error
  • In the "unexpected body" or "bad format", raise a BadResponseError or similar
  • Add status_code/resp.text to the exception as well, to help with debugging
@JonathanHuot
Copy link
Contributor

Another user experiences the same issue at oauthlib/oauthlib#510. A status check must be added at https://github.com/requests/requests-oauthlib/blob/master/requests_oauthlib/oauth2_session.py#L232-L244

jamielennox added a commit to jamielennox/requests-oauthlib that referenced this issue Jun 16, 2018
If the token request fails with an error such as Unauthenticated or
anything else the existing code doesn't check for this code and will try
and retrieve the token anyway. This gives a somewhat difficult to debug
key error when the response doesn't contain the token data.

This is wrong, we should always be checking the response code before
trusting the response data.

This is a slight change in behaviour, we now return this exception
instead of a KeyError, however the other exception was difficult to
catch.

This reuses the failure exception from OAuth1 and makes that public.

Closes: requests#302
jamielennox added a commit to jamielennox/requests-oauthlib that referenced this issue Jun 16, 2018
If the token request fails with an error such as Unauthenticated or
anything else the existing code doesn't check for this code and will try
and retrieve the token anyway. This gives a somewhat difficult to debug
key error when the response doesn't contain the token data.

This is wrong, we should always be checking the response code before
trusting the response data.

This is a slight change in behaviour, we now return this exception
instead of a KeyError, however the other exception was difficult to
catch.

This reuses the failure exception from OAuth1 and makes that public.

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

No branches or pull requests

2 participants