Merge pull request #18 from NYPL/SCC-3775/get-request-retries

add multiple retries for empty sierra responses

Author: charmingduchess

CHANGELOG.md

@@ -1,4 +1,13 @@
 # Changelog
+## v1.1.0
+- Add retries for empty responses in oauth2 client. This was added to address a known quirk in the Sierra API where this response is returned:
+```
+> GET / HTTP/1.1
+> Host: ilsstaff.nypl.org
+> User-Agent: curl/7.64.1
+> Accept: */*
+> 
+```
 
 ## v1.0.4 - 6/28/23
 - Enforce Kinesis stream 1000 records/second write limit

src/nypl_py_utils/classes/oauth2_api_client.py

@@ -1,4 +1,6 @@
 import os
+from time import sleep
+from requests.models import Response
 from oauthlib.oauth2 import BackendApplicationClient, TokenExpiredError
 from requests_oauthlib import OAuth2Session
 from nypl_py_utils.functions.log_helper import create_log
@@ -7,11 +9,14 @@
 class Oauth2ApiClient:
     """
     Client for interacting with an Oauth2 authenticated API such as NYPL's
-    Platform API endpoints
+    Platform API endpoints. Note with_retries is a boolean flag which
+    determines if empty get requests will be retried 3 times or until
+    they are successful. This is to address a known issue with the Sierra
+    API where empty responses are returned intermittently.
     """
 
     def __init__(self, client_id=None, client_secret=None, base_url=None,
-                 token_url=None):
+                 token_url=None, with_retries=False):
         self.client_id = client_id \
             or os.environ.get('NYPL_API_CLIENT_ID', None)
         self.client_secret = client_secret \
@@ -25,11 +30,30 @@ def __init__(self, client_id=None, client_secret=None, base_url=None,
 
         self.logger = create_log('oauth2_api_client')
 
+        self.with_retries = with_retries
+
     def get(self, request_path, **kwargs):
         """
         Issue an HTTP GET on the given request_path
         """
-        return self._do_http_method('GET', request_path, **kwargs)
+        resp = self._do_http_method('GET', request_path, **kwargs)
+        if resp.json() is None and self.with_retries is True:
+            retries = \
+                kwargs.get('retries', 0) + 1
+            if retries < 3:
+                self.logger.warning(
+                    f'Retrying get request due to empty response from\
+                         Oauth2 Client using path: {request_path}. \
+                         Retry #{retries}')
+                sleep(pow(2, retries - 1))
+                kwargs['retries'] = retries
+                resp = self.get(request_path, **kwargs)
+            else:
+                resp = Response()
+                resp.message = 'Oauth2 Client: Request failed after 3 \
+                        empty responses received from Oauth2 Client'
+                resp.status_code = 500
+        return resp
 
     def post(self, request_path, json, **kwargs):
         """
@@ -79,7 +103,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)
+            return self._do_http_method(method, request_path, **kwargs).json()
 
     def _create_oauth_client(self):
         """

tests/test_oauth2_api_client.py

@@ -20,6 +20,18 @@
 TOKEN_URL = 'https://oauth.example.com/oauth/token'
 
 
+class MockEmptyResponse:
+    def __init__(self, empty, status_code=None):
+        self.status_code = status_code
+        self.empty = empty
+
+    def json(self):
+        if self.empty:
+            return None
+        else:
+            return 'success'
+
+
 class TestOauth2ApiClient:
 
     @pytest.fixture
@@ -36,6 +48,15 @@ def test_instance(self, requests_mock):
                                client_secret='clientsecret'
                                )
 
+    @pytest.fixture
+    def test_instance_with_retries(self, requests_mock):
+        return Oauth2ApiClient(base_url=BASE_URL,
+                               token_url=TOKEN_URL,
+                               client_id='clientid',
+                               client_secret='clientsecret',
+                               with_retries=True
+                               )
+
     def test_uses_env_vars(self):
         env = {
             'NYPL_API_CLIENT_ID': 'env client id',
@@ -136,3 +157,24 @@ def test_token_refresh_failure_raises_error(
             test_instance._do_http_method('GET', 'foo')
         # Expect 1 initial token fetch, plus 3 retries:
         assert len(token_server_post.request_history) == 4
+
+    def test_http_retry_fail(self, requests_mock, test_instance_with_retries,
+                             token_server_post, mocker):
+        mocker.patch.object(test_instance_with_retries, '_do_http_method',
+                            return_value=MockEmptyResponse(empty=True))
+        get_spy = mocker.spy(test_instance_with_retries, 'get')
+        resp = test_instance_with_retries.get('spaghetti')
+        assert get_spy.call_count == 3
+        assert resp.status_code == 500
+
+    def test_http_retry_success(self, requests_mock,
+                                test_instance_with_retries,
+                                token_server_post, mocker):
+        mocker.patch.object(test_instance_with_retries, '_do_http_method',
+                            side_effect=[MockEmptyResponse(empty=True),
+                                         MockEmptyResponse(empty=False,
+                                                           status_code=200)])
+        get_spy = mocker.spy(test_instance_with_retries, 'get')
+        resp = test_instance_with_retries.get('spaghetti')
+        assert get_spy.call_count == 2
+        assert resp.json() == 'success'