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

raise_for_status after requesting oauth token #219

Open
adammcmaster opened this issue Jun 20, 2019 · 6 comments
Open

raise_for_status after requesting oauth token #219

adammcmaster opened this issue Jun 20, 2019 · 6 comments
Labels

Comments

@adammcmaster
Copy link
Contributor

Here:

Otherwise a HTTP 401 with no errors key can result in a KeyError:

app_1       | DEBUG:urllib3.connectionpool:https://www.zooniverse.org:443 "POST /oauth/token HTTP/1.1" 401 213
app_1       | Internal Server Error: /
app_1       | Traceback (most recent call last):
app_1       |   File "/usr/local/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
app_1       |     response = get_response(request)
app_1       |   File "/usr/local/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
app_1       |     response = self.process_exception_by_middleware(e, request)
app_1       |   File "/usr/local/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
app_1       |     response = wrapped_callback(request, *callback_args, **callback_kwargs)
app_1       |   File "/usr/local/lib/python3.7/site-packages/django/contrib/auth/decorators.py", line 21, in _wrapped_view
app_1       |     return view_func(request, *args, **kwargs)
app_1       |   File "/usr/src/app/hamlet/views.py", line 25, in index
app_1       |     'projects': Project.where(owner=request.user.username),
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 672, in where
app_1       |     return cls.paginated_results(*cls.http_get(_id, params=kwargs))
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 623, in http_get
app_1       |     **kwargs
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 320, in get
app_1       |     retry=retry,
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 267, in json_request
app_1       |     retry=retry,
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 191, in http_request
app_1       |     token = self.get_bearer_token()
app_1       |   File "/usr/local/lib/python3.7/site-packages/panoptes_client/panoptes.py", line 557, in get_bearer_token
app_1       |     self.bearer_token = token_response['access_token']
app_1       | KeyError: 'access_token'
@adammcmaster
Copy link
Contributor Author

The actual response to the request is:

{'error': 'invalid_grant', 'error_description': 'The provided authorization grant is invalid, expired, revoked, does not match the redirection URI used in the authorization request, or was issued to another client.'}

So error rather than errors in the JSON.

@camallen
Copy link
Contributor

FYI those are 3rd party library responses and we don't format them like our API endpoint error responses, i.e. /api/resource_name -> response errors: {}.

@adammcmaster
Copy link
Contributor Author

Ah, that explains the difference then. It'll be simple enough to handle both cases and I'll add a fallback based on the status code.

@adammcmaster
Copy link
Contributor Author

Noting that this should also handle JSONDecodeError:

JSONDecodeError: Expecting value: line 1 column 1 (char 0)
(20 additional frame(s) were not displayed)
...
  File "panoptes_client/panoptes.py", line 551, in get_bearer_token
    bearer_data
  File "requests/models.py", line 897, in json
    return complexjson.loads(self.text, **kwargs)
  File "__init__.py", line 348, in loads
    return _default_decoder.decode(s)
  File "json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None

A PanoptesAPIException with the raw response data would be more descriptive.

@camallen
Copy link
Contributor

Is the JSONDecodeError error caused by an API 500? If so I think Iknow what this error is and it might be better to fix it API side.

@adammcmaster
Copy link
Contributor Author

In this case I'm not sure what caused it. It certainly could have been a 500, but all I can tell from the error is that the response body was empty. When we handle the JSONDecodeError we should definitely include the HTTP status code in the raised exception.

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