update retry logic

Author: charmingduchess

src/nypl_py_utils/classes/oauth2_api_client.py

@@ -10,9 +10,9 @@ class Oauth2ApiClient:
     """
     Client for interacting with an Oauth2 authenticated API such as NYPL's
     Platform API endpoints. Note with_retries is a boolean flag which
-    determines if empty get requests will be retried self.MAX_RETRIES times or 
-    until they are successful. This is to address a known issue with the Sierra
-    API where empty and misformed responses are returned intermittently.
+    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,
@@ -32,30 +32,42 @@ def __init__(self, client_id=None, client_secret=None, base_url=None,
 
         self.with_retries = with_retries
 
-        self.MAX_RETRIES = 3
-
     def get(self, request_path, **kwargs):
         """
         Issue an HTTP GET on the given request_path
         """
         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 < self.MAX_RETRIES:
-                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 = f'Oauth2 Client: Request failed after \
-                    {self.MAX_RETRIES} empty responses received \
-                    from Oauth2 Client'
-                resp.status_code = 500
+        self.logger.debug('spaghetti')
+        # This try/except block is to handle one of at least two possible
+        # Sierra server errors. One is an empty response, and another is a
+        # response with a 200 status code but response text in HTML declaring
+        # Status 500.
+        try:
+            resp.json()
+        except Exception:
+            # build default server error response
+            resp = Response()
+            resp.message = 'Oauth2 Client: Bad response from OauthClient'
+            resp.status_code = 500
+            self.logger.warning(f'Bad response text: {resp.text}')
+            # if client has specified that we want to retry failed requests and
+            # we haven't hit max retries
+            if 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
+                    # try request again
+                    resp = self.get(request_path, **kwargs)
+                else:
+                    resp.message = 'Oauth2 Client: Request failed after 3 \
+                            empty responses received from Oauth2 Client'
+        # Return request. If retries returned real data, it will be here,
+        # otherwise it will be the default 500 response generated earlier.
         return resp
 
     def post(self, request_path, json, **kwargs):
@@ -101,7 +113,7 @@ def _do_http_method(self, method, request_path, **kwargs):
             # Raise error after 3 successive token refreshes
             kwargs['_do_http_method_token_refreshes'] = \
                 kwargs.get('_do_http_method_token_refreshes', 0) + 1
-            if kwargs['_do_http_method_token_refreshes'] > self.MAX_RETRIES:
+            if kwargs['_do_http_method_token_refreshes'] > 3:
                 raise Oauth2ApiClientError('Exhausted token refreshes') \
                     from None
 

tests/test_oauth2_api_client.py

@@ -3,7 +3,7 @@
 import json
 import pytest
 from requests_oauthlib import OAuth2Session
-from requests import HTTPError
+from requests import HTTPError, JSONDecodeError
 
 from nypl_py_utils.classes.oauth2_api_client import (Oauth2ApiClient,
                                                      Oauth2ApiClientError)
@@ -24,10 +24,11 @@ class MockEmptyResponse:
     def __init__(self, empty, status_code=None):
         self.status_code = status_code
         self.empty = empty
+        self.text = "error text"
 
     def json(self):
         if self.empty:
-            return None
+            raise JSONDecodeError
         else:
             return 'success'
 
@@ -158,6 +159,16 @@ def test_token_refresh_failure_raises_error(
         # Expect 1 initial token fetch, plus 3 retries:
         assert len(token_server_post.request_history) == 4
 
+    def test_bad_response_no_retries(self, requests_mock, test_instance,
+                             token_server_post, mocker):
+        mocker.patch.object(test_instance, '_do_http_method',
+                            return_value=MockEmptyResponse(empty=True))
+        get_spy = mocker.spy(test_instance, 'get')
+        resp = test_instance.get('spaghetti')
+        assert get_spy.call_count == 1
+        assert resp.status_code == 500
+        assert resp.message == 'Oauth2 Client: Bad response from OauthClient'
+
     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',
@@ -166,6 +177,8 @@ def test_http_retry_fail(self, requests_mock, test_instance_with_retries,
         resp = test_instance_with_retries.get('spaghetti')
         assert get_spy.call_count == 3
         assert resp.status_code == 500
+        assert resp.message == 'Oauth2 Client: Request failed after 3 \
+                            empty responses received from Oauth2 Client'
 
     def test_http_retry_success(self, requests_mock,
                                 test_instance_with_retries,