Repository: box/box-python-sdk
Pull request #831 (2023-07-27 08:42:04)

fix: Fix JSON response validation retry strategy

coveralls: ## Pull Request Test Coverage Report for [Build 5678991386](https://coveralls.io/builds/61611404)

* **9** of **9**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage increased (+**0.01%**) to **93.534%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/61611404/badge)](https://coveralls.io/builds/61611404) |
| :-- | --: |
| Change from base [Build 5655911124](https://coveralls.io/builds/61557089): |  0.01% |
| Covered Lines: | 3457 |
| Relevant Lines: | 3696 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/boxsdk/session/session.py b/boxsdk/session/session.py
index 641e353..ee039bd 100644
--- a/boxsdk/session/session.py
+++ b/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 patch:
diff --git a/test/unit/session/test_session.py b/test/unit/session/test_session.py
index aa76186..d6d9e55 100644
--- a/test/unit/session/test_session.py
+++ b/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)