Skip to content

Commit

Permalink
Merge branch 'main' into allanparsons/updatedocs
Browse files Browse the repository at this point in the history
  • Loading branch information
allanparsons authored Dec 30, 2022
2 parents a511008 + 833cd46 commit 202d36f
Show file tree
Hide file tree
Showing 4 changed files with 93 additions and 34 deletions.
61 changes: 34 additions & 27 deletions boxsdk/session/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
from numbers import Number
from typing import TYPE_CHECKING, Optional, Any, Type, Callable, Set

from requests.exceptions import RequestException, SSLError
from requests.exceptions import RequestException
from boxsdk.exception import BoxException
from .box_request import BoxRequest as _BoxRequest
from .box_response import BoxResponse as _BoxResponse
Expand All @@ -29,6 +29,7 @@ class Session:
_retry_randomization_factor = 0.5
_retry_base_interval = 1
_JWT_GRANT_TYPE = 'urn:ietf:params:oauth:grant-type:jwt-bearer'
_CCG_GRANT_TYPE = 'client_credentials'

"""
Box API session. Provides automatic retry of failed requests.
Expand Down Expand Up @@ -247,7 +248,7 @@ def get_retry_after_time(self, attempt_number: int, retry_after_header: Optional
return exponential * self._retry_base_interval * randomization

@staticmethod
def _raise_on_unsuccessful_request(network_response: 'NetworkResponse', request: '_BoxRequest') -> None:
def _raise_on_unsuccessful_request(network_response: 'NetworkResponse', request: '_BoxRequest', raised_exception: Exception) -> None:
"""
Raise an exception if the request was unsuccessful.
Expand All @@ -256,6 +257,9 @@ def _raise_on_unsuccessful_request(network_response: 'NetworkResponse', request:
:param request:
The API request that could be unsuccessful.
"""
if network_response is None:
raise raised_exception

if not network_response.ok:
response_json = {}
try:
Expand Down Expand Up @@ -324,36 +328,34 @@ def _prepare_and_send_request(

skip_retry_codes = kwargs.pop('skip_retry_codes', set())

session_renewal_needed = False
raised_exception = None
try:
network_response = self._send_request(request, **kwargs)
session_renewal_needed = network_response.status_code == 401
except SSLError as ssl_exc:
if 'EOF occurred in violation of protocol' in str(ssl_exc):
session_renewal_needed = True
network_response = None
except RequestException:
reauthentication_needed = network_response.status_code == 401
except RequestException as request_exc:
raised_exception = request_exc
network_response = None
if 'EOF occurred in violation of protocol' in str(request_exc):
reauthentication_needed = True
elif 'Connection aborted' in str(request_exc):
reauthentication_needed = False
else:
raise

while True:
retry = self._get_retry_request_callable(
network_response, attempt_number, request, skip_retry_codes, session_renewal_needed, **kwargs
)
network_response, attempt_number, request, skip_retry_codes, reauthentication_needed, **kwargs)

if retry is None or attempt_number >= API.MAX_RETRY_ATTEMPTS:
if network_response is None:
raise raised_exception
break

attempt_number += 1
self._logger.debug('Retrying request')
network_response = retry(request, **kwargs)

try:
network_response = retry(request, **kwargs)
except RequestException:
if attempt_number >= API.MAX_RETRY_ATTEMPTS:
raise
network_response = None

self._raise_on_unsuccessful_request(network_response, request)
self._raise_on_unsuccessful_request(network_response, request, raised_exception)

return network_response

Expand Down Expand Up @@ -385,28 +387,33 @@ 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 self._is_server_auth_type(kwargs):
return None
if network_response is None:
return partial(
self._network_layer.retry_after,
self.get_retry_after_time(attempt_number, None),
self._send_request,
)
data = kwargs.get('data', {})
grant_type = None
try:
if 'grant_type' in data:
grant_type = data['grant_type']
except TypeError:
pass
code = network_response.status_code
if (code in (202, 429) or code >= 500) and code not in skip_retry_codes and grant_type != self._JWT_GRANT_TYPE:
if (code in (202, 429) or code >= 500) and code not in skip_retry_codes:
return partial(
self._network_layer.retry_after,
self.get_retry_after_time(attempt_number, network_response.headers.get('Retry-After', None)),
self._send_request,
)
return None

def _is_server_auth_type(self, kwargs: dict) -> bool:
data = kwargs.get('data', {})
grant_type = None
try:
if 'grant_type' in data:
grant_type = data['grant_type']
except TypeError:
pass
return grant_type in (self._JWT_GRANT_TYPE, self._CCG_GRANT_TYPE)

def _get_request_headers(self) -> dict:
return self._default_headers.copy()

Expand Down
18 changes: 17 additions & 1 deletion boxsdk/util/log.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re
import sys

from collections.abc import Mapping
Expand All @@ -25,6 +26,11 @@ class Logging:
'password',
)

PROXY_KEYS_TO_SANITIZE = (
'http',
'https',
)

def setup_logging(self, stream_or_file=_no_logger, debug=False, name=None):
if not self._has_setup:
self._has_setup = True
Expand All @@ -43,13 +49,23 @@ def _setup_logging(stream_or_file=_no_logger, debug=False, name=None):
def sanitize_value(value):
return f'---{value[-4:]}'

def sanitize_dictionary(self, dictionary):
@staticmethod
def sanitize_proxy_value(value: str) -> str:
return re.sub(
'^(.*://)(.*):(.*)(@.*)$',
lambda repl: f'{repl.group(1)}{"---"}:{"---"}{repl.group(4)}',
value
)

def sanitize_dictionary(self, dictionary: Mapping) -> Mapping:
if not isinstance(dictionary, Mapping):
return dictionary
sanitized_dictionary = {}
for key, value in dictionary.items():
if key in self.KEYS_TO_SANITIZE and isinstance(value, str):
sanitized_dictionary[key] = self.sanitize_value(value)
elif key in self.PROXY_KEYS_TO_SANITIZE and isinstance(value, str):
sanitized_dictionary[key] = self.sanitize_proxy_value(value)
elif isinstance(value, Mapping):
sanitized_dictionary[key] = self.sanitize_dictionary(value)
else:
Expand Down
38 changes: 32 additions & 6 deletions test/unit/session/test_session.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from io import IOBase
from numbers import Number
from unittest.mock import MagicMock, Mock, PropertyMock, call, patch, ANY
from requests.exceptions import RequestException, SSLError
from requests.exceptions import RequestException, SSLError, ConnectionError as RequestsConnectionError

import pytest

Expand Down Expand Up @@ -222,23 +222,42 @@ def test_box_session_raises_for_failed_response(box_session, mock_network_layer,
box_session.get(url=test_url)


def test_box_session_retries_requests_library_exceptions(box_session, mock_network_layer, generic_successful_request_response, test_url):
mock_network_layer.request.side_effect = [RequestException(), generic_successful_request_response]
def test_box_session_retries_connection_aborted_exception(box_session, mock_network_layer, generic_successful_request_response, test_url):
mock_network_layer.request.side_effect = [RequestsConnectionError('Connection aborted'), generic_successful_request_response]
mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)

box_response = box_session.get(url=test_url)
assert box_response.status_code == 200


def test_box_session_raises_requests_library_exception_when_max_retries_limit_is_reached(box_session, mock_network_layer, test_url):
mock_network_layer.request.side_effect = [RequestException()] * (API.MAX_RETRY_ATTEMPTS + 1)
def test_box_session_retries_requests_library_exceptions_only_once(box_session, mock_network_layer, test_url, generic_successful_request_response):
mock_network_layer.request.side_effect = [
RequestException('Connection aborted'),
RequestException('Connection aborted'),
generic_successful_request_response
]
mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)

with pytest.raises(RequestException):
box_session.get(url=test_url)


def test_box_session_raises_requests_s(box_session, mock_oauth, mock_network_layer, generic_successful_request_response, test_url):
def test_box_session_raises_requests_library_exception_when_set_no_retries(
box_session, mock_network_layer, test_url, generic_successful_request_response
):
API.MAX_RETRY_ATTEMPTS = 0
try:
mock_network_layer.request.side_effect = [RequestException('Connection aborted'), generic_successful_request_response]

with pytest.raises(RequestException):
box_session.get(url=test_url)
finally:
API.MAX_RETRY_ATTEMPTS = 5


def test_box_session_retries_and_reauthenticates_on_violation_of_protocol_exception(
box_session, mock_oauth, mock_network_layer, generic_successful_request_response, test_url
):
def refresh(access_token_used):
assert access_token_used == mock_oauth.access_token
mock_oauth.access_token = 'fake_new_access_token'
Expand All @@ -258,6 +277,13 @@ def refresh(access_token_used):
assert mock_network_layer.request.mock_calls[1][2]['access_token'] == 'fake_new_access_token'


def test_box_session_does_not_retry_other_requests_library_exceptions_than_specified(box_session, mock_network_layer, test_url):
mock_network_layer.request.side_effect = [RequestException('Unknown error')]

with pytest.raises(RequestException):
box_session.get(url=test_url)


def test_box_session_raises_for_failed_response_with_error_and_error_description(box_session, mock_network_layer, bad_network_response_400, test_url):
mock_network_layer.request.side_effect = [bad_network_response_400]
try:
Expand Down
10 changes: 10 additions & 0 deletions test/unit/util/test_log.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ def test_setup_logging_is_reentrant(mock_logger):
{'download_url': None},
{'download_url': None},
),
# Test for proxy http
(
{'http': 'http://username:password@localhost:8080'},
{'http': 'http://---:---@localhost:8080'},
),
# Test for proxy https
(
{'https': 'http://username:password@localhost:8080'},
{'https': 'http://---:---@localhost:8080'},
),
]
)
def test_sanitize_dictionary_correctly_sanitizes_params(mock_logger, unsanitized_dict, expected_result):
Expand Down

0 comments on commit 202d36f

Please sign in to comment.