Skip to content

Commit bcaab27

Browse files
authored
fix: Renew connection when Connection reset error occurs (#771)
Closes: SDK-2732 Fixes: #756, fixes: #757, fixes: #763, fixes: #765, fixes: #766, fixes: #770
1 parent 73b73b8 commit bcaab27

File tree

3 files changed

+83
-14
lines changed

3 files changed

+83
-14
lines changed

boxsdk/session/box_request.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ class BoxRequest:
2020
headers: Optional[dict] = attr.ib(default=attr.Factory(dict))
2121
auto_session_renewal: Optional[bool] = attr.ib(default=True)
2222
expect_json_response: Optional[bool] = attr.ib(default=True)
23+
access_token: Optional[str] = attr.ib(default=None)
2324

2425
def __repr__(self) -> str:
2526
return f'<BoxRequest for {self.method} {self.url} with headers {sanitize_dictionary(self.headers)}'

boxsdk/session/session.py

Lines changed: 45 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,9 @@
44
from functools import partial
55
from logging import getLogger
66
from numbers import Number
7-
from typing import TYPE_CHECKING, Optional, Any, Type, Callable
7+
from typing import TYPE_CHECKING, Optional, Any, Type, Callable, Set
88

9+
from requests.exceptions import RequestException, SSLError
910
from boxsdk.exception import BoxException
1011
from .box_request import BoxRequest as _BoxRequest
1112
from .box_response import BoxResponse as _BoxResponse
@@ -301,9 +302,9 @@ def _prepare_and_send_request(
301302
:param headers:
302303
Headers to include with the request.
303304
:param auto_session_renewal:
304-
Whether or not to automatically renew the session if the request fails due to an expired access token.
305+
Whether to automatically renew the session if the request fails due to an expired access token.
305306
:param expect_json_response:
306-
Whether or not the response content should be json.
307+
Whether the response content should be json.
307308
"""
308309
files = kwargs.get('files')
309310
kwargs['file_stream_positions'] = None
@@ -322,27 +323,47 @@ def _prepare_and_send_request(
322323
)
323324

324325
skip_retry_codes = kwargs.pop('skip_retry_codes', set())
325-
network_response = self._send_request(request, **kwargs)
326+
327+
session_renewal_needed = False
328+
try:
329+
network_response = self._send_request(request, **kwargs)
330+
session_renewal_needed = network_response.status_code == 401
331+
except SSLError as ssl_exc:
332+
if 'EOF occurred in violation of protocol' in str(ssl_exc):
333+
session_renewal_needed = True
334+
network_response = None
335+
except RequestException:
336+
network_response = None
326337

327338
while True:
328-
retry = self._get_retry_request_callable(network_response, attempt_number, request, **kwargs)
339+
retry = self._get_retry_request_callable(
340+
network_response, attempt_number, request, skip_retry_codes, session_renewal_needed, **kwargs
341+
)
329342

330-
if retry is None or attempt_number >= API.MAX_RETRY_ATTEMPTS or network_response.status_code in skip_retry_codes:
343+
if retry is None or attempt_number >= API.MAX_RETRY_ATTEMPTS:
331344
break
332345

333346
attempt_number += 1
334347
self._logger.debug('Retrying request')
335-
network_response = retry(request, **kwargs)
348+
349+
try:
350+
network_response = retry(request, **kwargs)
351+
except RequestException:
352+
if attempt_number >= API.MAX_RETRY_ATTEMPTS:
353+
raise
354+
network_response = None
336355

337356
self._raise_on_unsuccessful_request(network_response, request)
338357

339358
return network_response
340359

341360
def _get_retry_request_callable(
342361
self,
343-
network_response: 'NetworkResponse',
362+
network_response: Optional['NetworkResponse'],
344363
attempt_number: int,
345364
request: '_BoxRequest',
365+
skip_retry_codes: Set[int],
366+
session_renewal_needed: bool = False,
346367
**kwargs: Any
347368
) -> Optional[Callable]:
348369
"""
@@ -364,6 +385,12 @@ def _get_retry_request_callable(
364385
Callable that, when called, will retry the request. Takes the same parameters as :meth:`_send_request`.
365386
"""
366387
# pylint:disable=unused-argument
388+
if network_response is None:
389+
return partial(
390+
self._network_layer.retry_after,
391+
self.get_retry_after_time(attempt_number, None),
392+
self._send_request,
393+
)
367394
data = kwargs.get('data', {})
368395
grant_type = None
369396
try:
@@ -372,7 +399,7 @@ def _get_retry_request_callable(
372399
except TypeError:
373400
pass
374401
code = network_response.status_code
375-
if (code in (202, 429) or code >= 500) and grant_type != self._JWT_GRANT_TYPE:
402+
if (code in (202, 429) or code >= 500) and code not in skip_retry_codes and grant_type != self._JWT_GRANT_TYPE:
376403
return partial(
377404
self._network_layer.retry_after,
378405
self.get_retry_after_time(attempt_number, network_response.headers.get('Retry-After', None)),
@@ -431,12 +458,13 @@ def _send_request(self, request: '_BoxRequest', **kwargs: Any) -> 'NetworkRespon
431458
request_kwargs['data'] = multipart_stream
432459
del request_kwargs['files']
433460
request.headers['Content-Type'] = multipart_stream.content_type
461+
request.access_token = request_kwargs.pop('access_token', None)
434462

435463
# send the request
436464
network_response = self._network_layer.request(
437465
request.method,
438466
request.url,
439-
access_token=request_kwargs.pop('access_token', None),
467+
access_token=request.access_token,
440468
headers=request.headers,
441469
log_response_content=request.expect_json_response,
442470
**request_kwargs
@@ -477,9 +505,11 @@ def _renew_session(self, access_token_used: Optional[str]) -> str:
477505

478506
def _get_retry_request_callable(
479507
self,
480-
network_response: 'NetworkResponse',
508+
network_response: Optional['NetworkResponse'],
481509
attempt_number: int,
482510
request: '_BoxRequest',
511+
skip_retry_codes: Set[int],
512+
session_renewal_needed: bool = False,
483513
**kwargs: Any
484514
) -> Callable:
485515
"""
@@ -498,15 +528,16 @@ def _get_retry_request_callable(
498528
:return:
499529
Callable that, when called, will retry the request. Takes the same parameters as :meth:`_send_request`.
500530
"""
501-
code = network_response.status_code
502-
if code == 401 and request.auto_session_renewal:
503-
self._renew_session(network_response.access_token_used)
531+
if request.auto_session_renewal and session_renewal_needed:
532+
self._renew_session(request.access_token)
504533
request.auto_session_renewal = False
505534
return self._send_request
506535
return super()._get_retry_request_callable(
507536
network_response,
508537
attempt_number,
509538
request,
539+
skip_retry_codes,
540+
session_renewal_needed,
510541
**kwargs
511542
)
512543

test/unit/session/test_session.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
from io import IOBase
33
from numbers import Number
44
from unittest.mock import MagicMock, Mock, PropertyMock, call, patch, ANY
5+
from requests.exceptions import RequestException, SSLError
56

67
import pytest
78

@@ -221,6 +222,42 @@ def test_box_session_raises_for_failed_response(box_session, mock_network_layer,
221222
box_session.get(url=test_url)
222223

223224

225+
def test_box_session_retries_requests_library_exceptions(box_session, mock_network_layer, generic_successful_request_response, test_url):
226+
mock_network_layer.request.side_effect = [RequestException(), generic_successful_request_response]
227+
mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)
228+
229+
box_response = box_session.get(url=test_url)
230+
assert box_response.status_code == 200
231+
232+
233+
def test_box_session_raises_requests_library_exception_when_max_retries_limit_is_reached(box_session, mock_network_layer, test_url):
234+
mock_network_layer.request.side_effect = [RequestException()] * (API.MAX_RETRY_ATTEMPTS + 1)
235+
mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)
236+
237+
with pytest.raises(RequestException):
238+
box_session.get(url=test_url)
239+
240+
241+
def test_box_session_raises_requests_s(box_session, mock_oauth, mock_network_layer, generic_successful_request_response, test_url):
242+
def refresh(access_token_used):
243+
assert access_token_used == mock_oauth.access_token
244+
mock_oauth.access_token = 'fake_new_access_token'
245+
return (mock_oauth.access_token, None)
246+
247+
mock_network_layer.request.side_effect = [
248+
SSLError("SSLError(SSLEOFError(8, 'EOF occurred in violation of protocol (_ssl.c:2396)')))"),
249+
generic_successful_request_response
250+
]
251+
mock_network_layer.retry_after.side_effect = lambda delay, request, *args, **kwargs: request(*args, **kwargs)
252+
mock_oauth.refresh.side_effect = refresh
253+
254+
box_session.get(url=test_url)
255+
256+
assert mock_network_layer.request.call_count == 2
257+
assert mock_network_layer.request.mock_calls[0][2]['access_token'] == 'fake_access_token'
258+
assert mock_network_layer.request.mock_calls[1][2]['access_token'] == 'fake_new_access_token'
259+
260+
224261
def test_box_session_raises_for_failed_response_with_error_and_error_description(box_session, mock_network_layer, bad_network_response_400, test_url):
225262
mock_network_layer.request.side_effect = [bad_network_response_400]
226263
try:

0 commit comments

Comments
 (0)