From 69834eb4c91a5aa4bc294a9fa49ecf753979d029 Mon Sep 17 00:00:00 2001 From: Minh Nguyen Cong Date: Tue, 1 Aug 2023 11:24:50 +0200 Subject: [PATCH] fix: Fix JSON response validation retry strategy (#831) --- boxsdk/session/session.py | 20 ++++++++++++++++++-- test/unit/session/test_session.py | 13 +++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/boxsdk/session/session.py b/boxsdk/session/session.py index 641e3530..ee039bd3 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, diff --git a/test/unit/session/test_session.py b/test/unit/session/test_session.py index aa761868..d6d9e55b 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)