Skip to content

Commit

Permalink
fix: Always return response from _do_http_method
Browse files Browse the repository at this point in the history
Noticed a bug where the *first* time we queried an API after a long time
(in this case Sierra), we'd see a json decode error coming from the
requests modules `response.json()`. Since we weren't calling it on our
end, I dug through the trace a little and found that we call `.json()`
in this library *only* when the token is expired and we do a refresh,
which would explain the timing of the bug.

So simply, make sure to return the raw response object when we do a
token refresh.

Here's an example stacktrace of the bug we are encountering:

File /ereadingserver/./app/main.py, line 186, in login
File /ereadingserver/./app/auth/login.py, line 62, in log_in_with_card
File /ereadingserver/./app/sierra.py, line 106, in patron_is_valid
File /usr/local/lib/python3.10/site-packages/nypl_py_utils/classes/oauth2_api_client.py, line 63, in post
File /usr/local/lib/python3.10/site-packages/nypl_py_utils/classes/oauth2_api_client.py, line 106, in _do_http_method
File /usr/local/lib/python3.10/site-packages/requests/models.py, line 975, in json
  • Loading branch information
sarangj committed Mar 14, 2024
1 parent 79b6d1b commit 3dbb1d5
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/nypl_py_utils/classes/oauth2_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ def _do_http_method(self, method, request_path, **kwargs):
from None

self._generate_access_token()
return self._do_http_method(method, request_path, **kwargs).json()
return self._do_http_method(method, request_path, **kwargs)

def _create_oauth_client(self):
"""
Expand Down
7 changes: 5 additions & 2 deletions tests/test_oauth2_api_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import json
import pytest
from requests_oauthlib import OAuth2Session
from requests import HTTPError, JSONDecodeError
from requests import HTTPError, JSONDecodeError, Response

from nypl_py_utils.classes.oauth2_api_client import (Oauth2ApiClient,
Oauth2ApiClientError)
Expand Down Expand Up @@ -120,7 +120,10 @@ def test_token_expiration(self, requests_mock, test_instance,
.post(TOKEN_URL, text=json.dumps(second_token_response))

# Perform second request:
test_instance._do_http_method('GET', 'foo')
response = test_instance._do_http_method('GET', 'foo')
# Ensure we still return a plain requests Response object
assert isinstance(response, Response)
assert response.json() == {"foo": "bar"}
# Expect a call on the second token server:
assert len(second_token_server_post.request_history) == 1
# Expect the second GET request to carry the new Bearer token:
Expand Down

0 comments on commit 3dbb1d5

Please sign in to comment.