fix: Fix JSON response validation retry strategy (#831)

Author: congminh1254

boxsdk/session/session.py

@@ -279,7 +279,8 @@ def _raise_on_unsuccessful_request(network_response: 'NetworkResponse', request:
                 context_info=response_json.get('context_info', None),
                 network_response=network_response
             )
-        if request.expect_json_response and not is_json_response(network_response):
+
+        if not Session._is_json_response_if_expected(network_response, request):
             raise BoxAPIException(
                 status=network_response.status_code,
                 headers=network_response.headers,
@@ -289,6 +290,18 @@ def _raise_on_unsuccessful_request(network_response: 'NetworkResponse', request:
                 network_response=network_response,
             )
 
+    @staticmethod
+    def _is_json_response_if_expected(network_response: 'NetworkResponse', request: '_BoxRequest') -> bool:
+        """
+        Validate that the response is json if the request expects json response.
+
+        :param network_response:
+            The network response which is being tested for success.
+        :param request:
+            The API request that could be unsuccessful.
+        """
+        return not request.expect_json_response or is_json_response(network_response)
+
     def _prepare_and_send_request(
             self,
             method: str,
@@ -391,13 +404,15 @@ def _get_retry_request_callable(
             Callable that, when called, will retry the request. Takes the same parameters as :meth:`_send_request`.
         """
         # pylint:disable=unused-argument
-        if network_response is None:
+        # pylint:disable=line-too-long
+        if network_response is None or (network_response.ok and request.method == 'GET' and not self._is_json_response_if_expected(network_response, request)):
             return partial(
                 self._network_layer.retry_after,
                 self.get_retry_after_time(attempt_number, None),
                 self._send_request,
             )
         code = network_response.status_code
+
         if (code in (202, 429) or code >= 500) and code not in skip_retry_codes and not self._is_server_auth_type(kwargs):
             return partial(
                 self._network_layer.retry_after,
@@ -541,6 +556,7 @@ def _get_retry_request_callable(
             self._renew_session(request.access_token)
             request.auto_session_renewal = False
             return self._send_request
+
         return super()._get_retry_request_callable(
             network_response,
             attempt_number,

test/unit/session/test_session.py

@@ -221,9 +221,18 @@ def test_box_session_seeks_file_after_retry(box_session, mock_network_layer, ser
     assert mock_file_2.seek.has_calls(call(3) * 2)
 
 
-def test_box_session_raises_for_non_json_response(box_session, mock_network_layer, non_json_response, test_url):
+def test_box_session_raises_for_non_json_response(box_session, mock_network_layer, non_json_response, generic_successful_response, test_url):
     # pylint:disable=redefined-outer-name
-    mock_network_layer.request.side_effect = [non_json_response]
+    mock_network_layer.request.side_effect = [non_json_response, generic_successful_response]
+    mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)
+
+    box_session.get(url=test_url)
+
+
+def test_box_session_raises_for_non_json_response_after_retry(box_session, mock_network_layer, non_json_response, test_url):
+    # pylint:disable=redefined-outer-name
+    mock_network_layer.request.side_effect = [non_json_response] * (API.MAX_RETRY_ATTEMPTS + 1)
+    mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)
 
     with pytest.raises(BoxAPIException):
         box_session.get(url=test_url)