From 48106446f68d17b012f15b3327f40869db2908c7 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 08:07:19 -0700 Subject: [PATCH 01/12] retry working with test --- stripe/error.py | 6 ++- stripe/http_client.py | 65 +++++++++++++++++++++++++++++++- tests/test_http_client.py | 78 ++++++++++++++++++++++++++++++++++----- 3 files changed, 137 insertions(+), 12 deletions(-) diff --git a/stripe/error.py b/stripe/error.py index 66d8a648f..86e1b3f50 100644 --- a/stripe/error.py +++ b/stripe/error.py @@ -53,7 +53,11 @@ class APIError(StripeError): class APIConnectionError(StripeError): - pass + def __init__(self, message, http_body=None, http_status=None, + json_body=None, headers=None, code=None, should_retry=False): + super(APIConnectionError, self).__init__(message, http_body, http_status, + json_body, headers, code) + self.should_retry = should_retry class StripeErrorWithParamCode(StripeError): diff --git a/stripe/http_client.py b/stripe/http_client.py index f0a590c53..d17a092e8 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -5,6 +5,7 @@ import textwrap import warnings import email +import time, random from stripe import error, util, six @@ -77,6 +78,10 @@ def new_default_http_client(*args, **kwargs): class HTTPClient(object): + MAX_RETRIES = 3 + MAX_DELAY = 2 + INITIAL_DELAY = 0.5 + def __init__(self, verify_ssl_certs=True, proxy=None): self._verify_ssl_certs = verify_ssl_certs if proxy: @@ -93,10 +98,65 @@ def request(self, method, url, headers, post_data=None): raise NotImplementedError( 'HTTPClient subclasses must implement `request`') + def request_with_retry(self, method, url, headers, post_data=None): + num_retries = 0 + response = None + connection_error = None + + while True: + try: + num_retries += 1 + response = self.request(method, url, headers, post_data) + except error.APIConnectionError as e: + connection_error = e + + if self.should_retry(response, connection_error, num_retries): + self._sleep(num_retries) + else: + if response is not None: + return response + else: + raise connection_error + + def should_retry(self, response, connection_error, num_retries): + if response is not None: + _, status_code, _ = response + should_retry = self.should_retry_on_codes(status_code) + else: + should_retry = connection_error.should_retry + return should_retry and num_retries <= HTTPClient.MAX_RETRIES + + def should_retry_on_error(self, connection_error): + if isinstance(connection_error, error.APIConnectionError): + return error.should_retry + else: + return False + + def should_retry_on_codes(self, code): + return code in [409] + + def _sleep(self, num_retries): + time.sleep(self._sleep_time(num_retries)) + def close(self): raise NotImplementedError( 'HTTPClient subclasses must implement `close`') + def _sleep_time(self, num_retries): + # Apply exponential backoff with initial_network_retry_delay on the + # number of num_retries so far as inputs. Do not allow the number to exceed + # max_network_retry_delay. + sleep_seconds = min([HTTPClient.INITIAL_DELAY * (2**(num_retries - 1)), HTTPClient.MAX_DELAY]) + + # Apply some jitter by randomizing the value in the range of (sleep_seconds + # / 2) to (sleep_seconds). + sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) + + # But never sleep less than the base sleep seconds. + sleep_seconds = max(HTTPClient.INITIAL_DELAY, sleep_seconds) + + return sleep_seconds + class RequestsClient(HTTPClient): name = 'requests' @@ -151,6 +211,7 @@ def _handle_request_error(self, e): "If this problem persists, let us know at " "support@stripe.com.") err = "%s: %s" % (type(e).__name__, str(e)) + should_retry = True else: msg = ("Unexpected error communicating with Stripe. " "It looks like there's probably a configuration " @@ -161,8 +222,10 @@ def _handle_request_error(self, e): err += " with error message %s" % (str(e),) else: err += " with no error message" + should_retry = False + msg = textwrap.fill(msg) + "\n\n(Network error: %s)" % (err,) - raise error.APIConnectionError(msg) + raise error.APIConnectionError(msg, should_retry=should_retry) def close(self): if self._session is not None: diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 6271bef20..c0a53a739 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -136,6 +136,15 @@ def mock_response(mock, body, code): mock.Session = mocker.MagicMock(return_value=session) return mock_response + @pytest.fixture + def response(self, mocker): + def response(code=200): + result = mocker.Mock() + result.content = '{}' + result.status_code = code + return result + return response + @pytest.fixture def mock_error(self, mocker, session): def mock_error(mock): @@ -144,27 +153,39 @@ def mock_error(mock): mock.Session = mocker.MagicMock(return_value=session) return mock_error + @pytest.fixture + def mock_retry(self, mocker, session, request_mock): + def mock_retry(retryable_error_num=0, non_retryable_error_num=0, responses=[]): + retryable_errors = [ZeroDivisionError()] * retryable_error_num + non_retryable_errors = [KeyError()] * non_retryable_error_num + request_mock.exceptions.RequestException = ArithmeticError + session.request.side_effect = retryable_errors + non_retryable_errors + responses + request_mock.Session = mocker.MagicMock(return_value=session) + return mock_retry + # Note that unlike other modules, we don't use the "mock" argument here # because we need to run the request call against the internal mock # session. @pytest.fixture def check_call(self, session): - def check_call(mock, method, url, post_data, headers, timeout=80): - session.request. \ - assert_called_with(method, url, - headers=headers, - data=post_data, - verify=RequestsVerify(), - proxies={"http": "http://slap/", - "https": "http://slap/"}, - timeout=timeout) + def check_call(mock, method, url, post_data, headers, timeout=80, times=None): + times = times or 1 + args = (method, url) + kwargs = {'headers': headers, + 'data': post_data, + 'verify': RequestsVerify(), + 'proxies': {"http": "http://slap/", "https": "http://slap/"}, + 'timeout': timeout} + calls = [(args, kwargs) for _ in range(times)] + session.request.assert_has_calls(calls) return check_call def make_request(self, method, url, headers, post_data, timeout=80): client = self.REQUEST_CLIENT(verify_ssl_certs=True, timeout=timeout, proxy='http://slap/') - return client.request(method, url, headers, post_data) + client._sleep_time = lambda _: 0.001 + return client.request_with_retry(method, url, headers, post_data) def test_timeout(self, request_mock, mock_response, check_call): headers = {'my-header': 'header val'} @@ -175,6 +196,43 @@ def test_timeout(self, request_mock, mock_response, check_call): check_call(None, 'POST', self.valid_url, data, headers, timeout=5) + def test_exception(self, request_mock, mock_error): + pass + + def max_retries(self): + return self.REQUEST_CLIENT.MAX_RETRIES + + def test_retry_on_exception_until_response(self, mock_retry, response, check_call): + mock_retry(retryable_error_num=1, responses=[response(code=202)]) + _, code, _ = self.make_request('GET', self.valid_url, {}, None) + assert code == 202 + check_call(None, 'GET', self.valid_url, None, {}, times=2) + + def test_retry_on_exception_until_exceeded(self, mock_retry, response, check_call): + mock_retry(retryable_error_num=3) + with pytest.raises(stripe.error.APIConnectionError): + self.make_request('GET', self.valid_url, {}, None) + + check_call(None, 'GET', self.valid_url, None, {}, times=self.max_retries()) + + def test_no_retry_on_exception(self, mock_retry, response, check_call): + mock_retry(non_retryable_error_num=3) + with pytest.raises(stripe.error.APIConnectionError): + self.make_request('GET', self.valid_url, {}, None) + check_call(None, 'GET', self.valid_url, None, {}, times=1) + + def test_retry_on_codes(self, mock_retry, response, check_call): + mock_retry(responses=[response(code=409), response(code=202)]) + _, code, _ = self.make_request('GET', self.valid_url, {}, None) + assert code == 202 + check_call(None, 'GET', self.valid_url, None, {}, times=2) + + def test_retry_on_codes_until_exceeded(self, mock_retry, response, check_call): + mock_retry(responses=[response(code=409)]*3) + _, code, _ = self.make_request('GET', self.valid_url, {}, None) + assert code == 409 + check_call(None, 'GET', self.valid_url, None, {}, times=self.max_retries()) + class TestUrlFetchClient(StripeClientTestCase, ClientTestBase): REQUEST_CLIENT = stripe.http_client.UrlFetchClient From 7a6488c0ca8e2ea670ea36732eaf3e70ea8d4862 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 11:02:56 -0700 Subject: [PATCH 02/12] catch specific errors --- stripe/error.py | 3 +- stripe/http_client.py | 37 +++++++++++++++---- tests/test_http_client.py | 77 ++++++++++++++++++++++++++++++--------- 3 files changed, 90 insertions(+), 27 deletions(-) diff --git a/stripe/error.py b/stripe/error.py index 86e1b3f50..ae0830d1a 100644 --- a/stripe/error.py +++ b/stripe/error.py @@ -55,7 +55,8 @@ class APIError(StripeError): class APIConnectionError(StripeError): def __init__(self, message, http_body=None, http_status=None, json_body=None, headers=None, code=None, should_retry=False): - super(APIConnectionError, self).__init__(message, http_body, http_status, + super(APIConnectionError, self).__init__(message, http_body, + http_status, json_body, headers, code) self.should_retry = should_retry diff --git a/stripe/http_client.py b/stripe/http_client.py index d17a092e8..76b594f2a 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -5,7 +5,8 @@ import textwrap import warnings import email -import time, random +import time +import random from stripe import error, util, six @@ -144,12 +145,14 @@ def close(self): def _sleep_time(self, num_retries): # Apply exponential backoff with initial_network_retry_delay on the - # number of num_retries so far as inputs. Do not allow the number to exceed - # max_network_retry_delay. - sleep_seconds = min([HTTPClient.INITIAL_DELAY * (2**(num_retries - 1)), HTTPClient.MAX_DELAY]) - - # Apply some jitter by randomizing the value in the range of (sleep_seconds - # / 2) to (sleep_seconds). + # number of num_retries so far as inputs. + # Do not allow the number to exceed max_network_retry_delay. + sleep_seconds = min( + [HTTPClient.INITIAL_DELAY * (2 ** (num_retries - 1)), + HTTPClient.MAX_DELAY]) + + # Apply some jitter by randomizing the value in the range of + # (sleep_seconds/ 2) to (sleep_seconds). sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) # But never sleep less than the base sleep seconds. @@ -206,12 +209,30 @@ def request(self, method, url, headers, post_data=None): return content, status_code, result.headers def _handle_request_error(self, e): - if isinstance(e, requests.exceptions.RequestException): + + # Catch SSL error first as it belongs to ConnectionError + if isinstance(e, requests.exceptions.SSLError): + msg = ("Could not verify Stripe's SSL certificate. Please make " + "sure that your network is not intercepting certificates. " + "If this problem persists, let us know at " + "support@stripe.com.") + err = "%s: %s" % (type(e).__name__, str(e)) + should_retry = False + # Retry only timeout and connect errors; similar to urllib3 Retry + elif isinstance(e, requests.exceptions.Timeout) or \ + isinstance(e, requests.exceptions.ConnectionError): msg = ("Unexpected error communicating with Stripe. " "If this problem persists, let us know at " "support@stripe.com.") err = "%s: %s" % (type(e).__name__, str(e)) should_retry = True + # Catch all request specific with descriptive class/messages + elif isinstance(e, requests.exceptions.RequestException): + msg = ("Unexpected error communicating with Stripe. " + "If this problem persists, let us know at " + "support@stripe.com.") + err = "%s: %s" % (type(e).__name__, str(e)) + should_retry = False else: msg = ("Unexpected error communicating with Stripe. " "It looks like there's probably a configuration " diff --git a/tests/test_http_client.py b/tests/test_http_client.py index c0a53a739..9404fea3c 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -155,12 +155,38 @@ def mock_error(mock): @pytest.fixture def mock_retry(self, mocker, session, request_mock): - def mock_retry(retryable_error_num=0, non_retryable_error_num=0, responses=[]): - retryable_errors = [ZeroDivisionError()] * retryable_error_num - non_retryable_errors = [KeyError()] * non_retryable_error_num - request_mock.exceptions.RequestException = ArithmeticError - session.request.side_effect = retryable_errors + non_retryable_errors + responses + def mock_retry(retry_error_num=0, + no_retry_error_num=0, + uncaught_error_num=0, + responses=[]): + + # Mocking classes of exception we catch + # Any other exceptions with the same inheritance will work + # We consider request exceptions first, and differentiate which + # specific ones to retry. + root_error_class = Exception + request_mock.exceptions.RequestException = root_error_class + + no_retry_parent_class = LookupError + no_retry_child_class = KeyError + request_mock.exceptions.SSLError = no_retry_parent_class + no_retry_errors = [no_retry_child_class()] * no_retry_error_num + + retry_parent_class = EnvironmentError + retry_child_class = IOError + request_mock.exceptions.Timeout = retry_parent_class + request_mock.exceptions.ConnectionError = retry_parent_class + retry_errors = [retry_child_class()] * retry_error_num + + uncaught_errors = [KeyboardInterrupt()] * uncaught_error_num + + # Include mock responses as possible side-effects + # Eg. return proper responses after some exceptions + session.request.side_effect = retry_errors + no_retry_errors + \ + uncaught_errors + responses + request_mock.Session = mocker.MagicMock(return_value=session) + return mock_retry # Note that unlike other modules, we don't use the "mock" argument here @@ -168,16 +194,19 @@ def mock_retry(retryable_error_num=0, non_retryable_error_num=0, responses=[]): # session. @pytest.fixture def check_call(self, session): - def check_call(mock, method, url, post_data, headers, timeout=80, times=None): + def check_call(mock, method, url, post_data, headers, timeout=80, + times=None): times = times or 1 args = (method, url) kwargs = {'headers': headers, 'data': post_data, 'verify': RequestsVerify(), - 'proxies': {"http": "http://slap/", "https": "http://slap/"}, + 'proxies': {"http": "http://slap/", + "https": "http://slap/"}, 'timeout': timeout} calls = [(args, kwargs) for _ in range(times)] session.request.assert_has_calls(calls) + return check_call def make_request(self, method, url, headers, post_data, timeout=80): @@ -202,36 +231,48 @@ def test_exception(self, request_mock, mock_error): def max_retries(self): return self.REQUEST_CLIENT.MAX_RETRIES - def test_retry_on_exception_until_response(self, mock_retry, response, check_call): - mock_retry(retryable_error_num=1, responses=[response(code=202)]) + def test_retry_error_until_response(self, mock_retry, response, + check_call): + mock_retry(retry_error_num=1, responses=[response(code=202)]) _, code, _ = self.make_request('GET', self.valid_url, {}, None) assert code == 202 check_call(None, 'GET', self.valid_url, None, {}, times=2) - def test_retry_on_exception_until_exceeded(self, mock_retry, response, check_call): - mock_retry(retryable_error_num=3) + def test_retry_error_until_exceeded(self, mock_retry, response, + check_call): + mock_retry(retry_error_num=3) with pytest.raises(stripe.error.APIConnectionError): self.make_request('GET', self.valid_url, {}, None) - check_call(None, 'GET', self.valid_url, None, {}, times=self.max_retries()) + check_call(None, 'GET', self.valid_url, None, {}, + times=self.max_retries()) + + def test_retry_error_until_uncaught(self, mock_retry, response, + check_call): + mock_retry(retry_error_num=1, uncaught_error_num=1) + with pytest.raises(stripe.error.APIConnectionError): + self.make_request('GET', self.valid_url, {}, None) + check_call(None, 'GET', self.valid_url, None, {}, times=2) - def test_no_retry_on_exception(self, mock_retry, response, check_call): - mock_retry(non_retryable_error_num=3) + def test_no_retry_error(self, mock_retry, response, check_call): + mock_retry(no_retry_error_num=3) with pytest.raises(stripe.error.APIConnectionError): self.make_request('GET', self.valid_url, {}, None) check_call(None, 'GET', self.valid_url, None, {}, times=1) - def test_retry_on_codes(self, mock_retry, response, check_call): + def test_retry_codes(self, mock_retry, response, check_call): mock_retry(responses=[response(code=409), response(code=202)]) _, code, _ = self.make_request('GET', self.valid_url, {}, None) assert code == 202 check_call(None, 'GET', self.valid_url, None, {}, times=2) - def test_retry_on_codes_until_exceeded(self, mock_retry, response, check_call): - mock_retry(responses=[response(code=409)]*3) + def test_retry_codes_until_exceeded(self, mock_retry, response, + check_call): + mock_retry(responses=[response(code=409)] * 3) _, code, _ = self.make_request('GET', self.valid_url, {}, None) assert code == 409 - check_call(None, 'GET', self.valid_url, None, {}, times=self.max_retries()) + check_call(None, 'GET', self.valid_url, None, {}, + times=self.max_retries()) class TestUrlFetchClient(StripeClientTestCase, ClientTestBase): From 002ce8350eae94666dc2d939fa5e7b242d3c27c4 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 11:56:26 -0700 Subject: [PATCH 03/12] add test for sleeping behavior --- stripe/http_client.py | 15 ++++++---- tests/test_http_client.py | 63 ++++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 16 deletions(-) diff --git a/stripe/http_client.py b/stripe/http_client.py index 76b594f2a..b5703c7ff 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -79,6 +79,7 @@ def new_default_http_client(*args, **kwargs): class HTTPClient(object): + MAX_RETRIES = 3 MAX_DELAY = 2 INITIAL_DELAY = 0.5 @@ -143,17 +144,21 @@ def close(self): raise NotImplementedError( 'HTTPClient subclasses must implement `close`') + def _add_jitter_time(self, sleep_seconds): + # Randomize the value in [(sleep_seconds/ 2) to (sleep_seconds)] + # Also separated here to isolate randomness for testing purposes + sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) + return sleep_seconds + def _sleep_time(self, num_retries): # Apply exponential backoff with initial_network_retry_delay on the # number of num_retries so far as inputs. # Do not allow the number to exceed max_network_retry_delay. sleep_seconds = min( - [HTTPClient.INITIAL_DELAY * (2 ** (num_retries - 1)), - HTTPClient.MAX_DELAY]) + HTTPClient.INITIAL_DELAY * (2 ** (num_retries - 1)), + HTTPClient.MAX_DELAY) - # Apply some jitter by randomizing the value in the range of - # (sleep_seconds/ 2) to (sleep_seconds). - sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) + sleep_seconds = self._add_jitter_time(sleep_seconds) # But never sleep less than the base sleep seconds. sleep_seconds = max(HTTPClient.INITIAL_DELAY, sleep_seconds) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 9404fea3c..334d1cf61 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -52,6 +52,57 @@ def test_new_default_http_client_urllib2(self): stripe.http_client.Urllib2Client) +class TestRetrySleepTimeDefaultHttpClient(StripeClientTestCase): + from contextlib import contextmanager + + def assert_sleep_times(self, client, until, expected): + assert len(expected) == until + actual = list(map(lambda i: client._sleep_time(i+1), range(until))) + assert expected == actual + + @contextmanager + def mock_max_delay(self, new_value): + original_value = stripe.http_client.HTTPClient.MAX_DELAY + stripe.http_client.HTTPClient.MAX_DELAY = new_value + try: + yield self + finally: + stripe.http_client.HTTPClient.MAX_DELAY = original_value + + def test_sleep_time_exponential_back_off(self): + client = stripe.http_client.new_default_http_client() + client._add_jitter_time = lambda t: t + with self.mock_max_delay(10): + self.assert_sleep_times(client, 5, [0.5, 1.0, 2.0, 4.0, 8.0]) + + def test_initial_delay_as_minimum(self): + client = stripe.http_client.new_default_http_client() + client._add_jitter_time = lambda t: t * 0.001 + initial_delay = stripe.http_client.HTTPClient.INITIAL_DELAY + self.assert_sleep_times(client, 5, [initial_delay] * 5) + + def test_maximum_delay(self): + client = stripe.http_client.new_default_http_client() + client._add_jitter_time = lambda t: t + max_delay = stripe.http_client.HTTPClient.MAX_DELAY + expected = [0.5, 1.0, max_delay, max_delay, max_delay] + self.assert_sleep_times(client, 5, expected) + + def test_randomness_added(self): + client = stripe.http_client.new_default_http_client() + random_value = 0.8 + client._add_jitter_time = lambda t: t * random_value + base_value = stripe.http_client.HTTPClient.INITIAL_DELAY * random_value + + with self.mock_max_delay(10): + expected = [stripe.http_client.HTTPClient.INITIAL_DELAY, + base_value * 2, + base_value * 4, + base_value * 8, + base_value * 16] + self.assert_sleep_times(client, 5, expected) + + class ClientTestBase(object): @pytest.fixture def request_mock(self, request_mocks): @@ -157,7 +208,6 @@ def mock_error(mock): def mock_retry(self, mocker, session, request_mock): def mock_retry(retry_error_num=0, no_retry_error_num=0, - uncaught_error_num=0, responses=[]): # Mocking classes of exception we catch @@ -178,12 +228,10 @@ def mock_retry(retry_error_num=0, request_mock.exceptions.ConnectionError = retry_parent_class retry_errors = [retry_child_class()] * retry_error_num - uncaught_errors = [KeyboardInterrupt()] * uncaught_error_num - # Include mock responses as possible side-effects # Eg. return proper responses after some exceptions session.request.side_effect = retry_errors + no_retry_errors + \ - uncaught_errors + responses + responses request_mock.Session = mocker.MagicMock(return_value=session) @@ -247,13 +295,6 @@ def test_retry_error_until_exceeded(self, mock_retry, response, check_call(None, 'GET', self.valid_url, None, {}, times=self.max_retries()) - def test_retry_error_until_uncaught(self, mock_retry, response, - check_call): - mock_retry(retry_error_num=1, uncaught_error_num=1) - with pytest.raises(stripe.error.APIConnectionError): - self.make_request('GET', self.valid_url, {}, None) - check_call(None, 'GET', self.valid_url, None, {}, times=2) - def test_no_retry_error(self, mock_retry, response, check_call): mock_retry(no_retry_error_num=3) with pytest.raises(stripe.error.APIConnectionError): From de1e1813a65deb1a1c6341da0b682a66c4c13843 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 15:17:06 -0700 Subject: [PATCH 04/12] add idempotency setting --- stripe/api_requestor.py | 2 ++ tests/test_api_requestor.py | 59 ++++++++++++++++++++++++++++++++----- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/stripe/api_requestor.py b/stripe/api_requestor.py index 8625835c5..af3c43c2c 100644 --- a/stripe/api_requestor.py +++ b/stripe/api_requestor.py @@ -5,6 +5,7 @@ import json import platform import time +import uuid import stripe from stripe import error, oauth_error, http_client, version, util, six @@ -220,6 +221,7 @@ def request_headers(self, api_key, method): if method == 'post': headers['Content-Type'] = 'application/x-www-form-urlencoded' + headers.setdefault('Idempotency-Key', str(uuid.uuid4())) if self.api_version is not None: headers['Stripe-Version'] = self.api_version diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index 98e1347cf..28e62f0a8 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -3,6 +3,7 @@ import datetime import json import tempfile +import uuid import pytest @@ -35,28 +36,32 @@ class APIHeaderMatcher(object): 'User-Agent', 'X-Stripe-Client-User-Agent', ] - METHOD_EXTRA_KEYS = {"post": ["Content-Type"]} + METHOD_EXTRA_KEYS = {"post": ["Content-Type", "Idempotency-Key"]} def __init__(self, api_key=None, extra={}, request_method=None, - user_agent=None, app_info=None): + user_agent=None, app_info=None, idempotency_key=None): self.request_method = request_method self.api_key = api_key or stripe.api_key self.extra = extra self.user_agent = user_agent self.app_info = app_info + self.idempotency_key = idempotency_key def __eq__(self, other): return (self._keys_match(other) and self._auth_match(other) and self._user_agent_match(other) and self._x_stripe_ua_contains_app_info(other) and + self._idempotency_key_match(other) and self._extra_match(other)) def __repr__(self): return ("APIHeaderMatcher(request_method=%s, api_key=%s, extra=%s, " - "user_agent=%s, app_info=%s)" % + "user_agent=%s, app_info=%s, idempotency_key=%s)" % (repr(self.request_method), repr(self.api_key), - repr(self.extra), repr(self.user_agent), repr(self.app_info))) + repr(self.extra), repr(self.user_agent), repr(self.app_info), + repr(self.idempotency_key)) + ) def _keys_match(self, other): expected_keys = list(set(self.EXP_KEYS + list(self.extra.keys()))) @@ -74,6 +79,12 @@ def _user_agent_match(self, other): return True + def _idempotency_key_match(self, other): + if self.idempotency_key is not None: + return other['Idempotency-Key'] == self.idempotency_key + + return True + def _x_stripe_ua_contains_app_info(self, other): if self.app_info: ua = json.loads(other['X-Stripe-Client-User-Agent']) @@ -129,6 +140,19 @@ def __repr__(self): return ("UrlMatcher(exp_parts=%s)" % (repr(self.exp_parts))) +class AnyUUID4Matcher(object): + + def __eq__(self, other): + try: + uuid.UUID(other, version=4) + except ValueError: + return False + return True + + def __repr__(self): + return "AnyUUID4Matcher()" + + class TestAPIRequestor(object): ENCODE_INPUTS = { 'dict': { @@ -417,11 +441,30 @@ def test_uses_app_info(self, requestor, mock_response, check_call): finally: stripe.app_info = old - def test_fails_without_api_key(self, requestor): - stripe.api_key = None + def test_uses_given_idempotency_key(self, requestor, mock_response, + check_call): + mock_response('{}', 200) + meth = 'post' + requestor.request(meth, self.valid_path, {}, + {'Idempotency-Key': '123abc'}) - with pytest.raises(stripe.error.AuthenticationError): - requestor.request('get', self.valid_path, {}) + header_matcher = APIHeaderMatcher( + request_method=meth, + idempotency_key='123abc' + ) + check_call(meth, headers=header_matcher, post_data='') + + def test_create_uuid4_idempotency_key(self, requestor, mock_response, + check_call): + mock_response('{}', 200) + meth = 'post' + requestor.request(meth, self.valid_path, {}) + + header_matcher = APIHeaderMatcher( + request_method=meth, + idempotency_key=AnyUUID4Matcher() + ) + check_call(meth, headers=header_matcher, post_data='') def test_invalid_request_error_404(self, requestor, mock_response): mock_response('{"error": {}}', 404) From f84865669dbbb0cb80c8373c83e7449d8b81f35f Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 15:46:46 -0700 Subject: [PATCH 05/12] refactor test --- stripe/api_requestor.py | 1 - stripe/http_client.py | 19 ++--- tests/test_api_requestor.py | 6 ++ tests/test_http_client.py | 136 +++++++++++++++++++++--------------- 4 files changed, 90 insertions(+), 72 deletions(-) diff --git a/stripe/api_requestor.py b/stripe/api_requestor.py index af3c43c2c..ef89f7ad1 100644 --- a/stripe/api_requestor.py +++ b/stripe/api_requestor.py @@ -273,7 +273,6 @@ def request_raw(self, method, url, params=None, supplied_headers=None): 'assistance.' % (method,)) headers = self.request_headers(my_api_key, method) - if supplied_headers is not None: for key, value in six.iteritems(supplied_headers): headers[key] = value diff --git a/stripe/http_client.py b/stripe/http_client.py index b5703c7ff..195339c38 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -102,15 +102,15 @@ def request(self, method, url, headers, post_data=None): def request_with_retry(self, method, url, headers, post_data=None): num_retries = 0 - response = None - connection_error = None while True: try: num_retries += 1 response = self.request(method, url, headers, post_data) + connection_error = None except error.APIConnectionError as e: connection_error = e + response = None if self.should_retry(response, connection_error, num_retries): self._sleep(num_retries) @@ -123,19 +123,10 @@ def request_with_retry(self, method, url, headers, post_data=None): def should_retry(self, response, connection_error, num_retries): if response is not None: _, status_code, _ = response - should_retry = self.should_retry_on_codes(status_code) + should_retry = status_code == 409 else: should_retry = connection_error.should_retry - return should_retry and num_retries <= HTTPClient.MAX_RETRIES - - def should_retry_on_error(self, connection_error): - if isinstance(connection_error, error.APIConnectionError): - return error.should_retry - else: - return False - - def should_retry_on_codes(self, code): - return code in [409] + return should_retry and num_retries < HTTPClient.MAX_RETRIES def _sleep(self, num_retries): time.sleep(self._sleep_time(num_retries)) @@ -146,7 +137,7 @@ def close(self): def _add_jitter_time(self, sleep_seconds): # Randomize the value in [(sleep_seconds/ 2) to (sleep_seconds)] - # Also separated here to isolate randomness for testing purposes + # Also separated method here to isolate randomness for tests sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) return sleep_seconds diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index 28e62f0a8..c77e4a716 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -466,6 +466,12 @@ def test_create_uuid4_idempotency_key(self, requestor, mock_response, ) check_call(meth, headers=header_matcher, post_data='') + def test_fails_without_api_key(self, requestor): + stripe.api_key = None + + with pytest.raises(stripe.error.AuthenticationError): + requestor.request('get', self.valid_path, {}) + def test_invalid_request_error_404(self, requestor, mock_response): mock_response('{"error": {}}', 404) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 334d1cf61..42f32b4b8 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -187,6 +187,52 @@ def mock_response(mock, body, code): mock.Session = mocker.MagicMock(return_value=session) return mock_response + @pytest.fixture + def mock_error(self, mocker, session): + def mock_error(mock): + mock.exceptions.SSLError = Exception + session.request.side_effect = mock.exceptions.SSLError() + mock.Session = mocker.MagicMock(return_value=session) + return mock_error + + # Note that unlike other modules, we don't use the "mock" argument here + # because we need to run the request call against the internal mock + # session. + @pytest.fixture + def check_call(self, session): + def check_call(mock, method, url, post_data, headers, timeout=80, + times=None): + times = times or 1 + args = (method, url) + kwargs = {'headers': headers, + 'data': post_data, + 'verify': RequestsVerify(), + 'proxies': {"http": "http://slap/", + "https": "http://slap/"}, + 'timeout': timeout} + calls = [(args, kwargs) for _ in range(times)] + session.request.assert_has_calls(calls) + + return check_call + + def make_request(self, method, url, headers, post_data, timeout=80): + client = self.REQUEST_CLIENT(verify_ssl_certs=True, + timeout=timeout, + proxy='http://slap/') + return client.request(method, url, headers, post_data) + + def test_timeout(self, request_mock, mock_response, check_call): + headers = {'my-header': 'header val'} + data = '' + mock_response(request_mock, '{"foo": "baz"}', 200) + self.make_request('POST', self.valid_url, + headers, data, timeout=5) + + check_call(None, 'POST', self.valid_url, data, headers, timeout=5) + + +class TestRetryRequestClient(TestRequestsClient): + @pytest.fixture def response(self, mocker): def response(code=200): @@ -196,14 +242,6 @@ def response(code=200): return result return response - @pytest.fixture - def mock_error(self, mocker, session): - def mock_error(mock): - mock.exceptions.RequestException = Exception - session.request.side_effect = mock.exceptions.RequestException() - mock.Session = mocker.MagicMock(return_value=session) - return mock_error - @pytest.fixture def mock_retry(self, mocker, session, request_mock): def mock_retry(retry_error_num=0, @@ -237,83 +275,67 @@ def mock_retry(retry_error_num=0, return mock_retry - # Note that unlike other modules, we don't use the "mock" argument here - # because we need to run the request call against the internal mock - # session. @pytest.fixture - def check_call(self, session): - def check_call(mock, method, url, post_data, headers, timeout=80, - times=None): - times = times or 1 - args = (method, url) - kwargs = {'headers': headers, - 'data': post_data, - 'verify': RequestsVerify(), - 'proxies': {"http": "http://slap/", - "https": "http://slap/"}, - 'timeout': timeout} - calls = [(args, kwargs) for _ in range(times)] - session.request.assert_has_calls(calls) + def check_call_numbers(self, check_call): + valid_url = self.valid_url - return check_call + def check_call_numbers(times): + check_call(None, 'GET', valid_url, None, {}, times=times) + return check_call_numbers - def make_request(self, method, url, headers, post_data, timeout=80): + def make_request(self): client = self.REQUEST_CLIENT(verify_ssl_certs=True, - timeout=timeout, + timeout=80, proxy='http://slap/') client._sleep_time = lambda _: 0.001 - return client.request_with_retry(method, url, headers, post_data) - - def test_timeout(self, request_mock, mock_response, check_call): - headers = {'my-header': 'header val'} - data = '' - mock_response(request_mock, '{"foo": "baz"}', 200) - self.make_request('POST', self.valid_url, - headers, data, timeout=5) - - check_call(None, 'POST', self.valid_url, data, headers, timeout=5) - - def test_exception(self, request_mock, mock_error): - pass + return client.request_with_retry('GET', self.valid_url, {}, None) def max_retries(self): return self.REQUEST_CLIENT.MAX_RETRIES def test_retry_error_until_response(self, mock_retry, response, - check_call): + check_call_numbers): mock_retry(retry_error_num=1, responses=[response(code=202)]) - _, code, _ = self.make_request('GET', self.valid_url, {}, None) + _, code, _ = self.make_request() assert code == 202 - check_call(None, 'GET', self.valid_url, None, {}, times=2) + check_call_numbers(2) def test_retry_error_until_exceeded(self, mock_retry, response, - check_call): + check_call_numbers): mock_retry(retry_error_num=3) with pytest.raises(stripe.error.APIConnectionError): - self.make_request('GET', self.valid_url, {}, None) + self.make_request() - check_call(None, 'GET', self.valid_url, None, {}, - times=self.max_retries()) + check_call_numbers(self.max_retries()) - def test_no_retry_error(self, mock_retry, response, check_call): + def test_no_retry_error(self, mock_retry, response, check_call_numbers): mock_retry(no_retry_error_num=3) with pytest.raises(stripe.error.APIConnectionError): - self.make_request('GET', self.valid_url, {}, None) - check_call(None, 'GET', self.valid_url, None, {}, times=1) + self.make_request() + check_call_numbers(1) - def test_retry_codes(self, mock_retry, response, check_call): + def test_retry_codes(self, mock_retry, response, check_call_numbers): mock_retry(responses=[response(code=409), response(code=202)]) - _, code, _ = self.make_request('GET', self.valid_url, {}, None) + _, code, _ = self.make_request() assert code == 202 - check_call(None, 'GET', self.valid_url, None, {}, times=2) + check_call_numbers(2) def test_retry_codes_until_exceeded(self, mock_retry, response, - check_call): + check_call_numbers): mock_retry(responses=[response(code=409)] * 3) - _, code, _ = self.make_request('GET', self.valid_url, {}, None) + _, code, _ = self.make_request() assert code == 409 - check_call(None, 'GET', self.valid_url, None, {}, - times=self.max_retries()) + check_call_numbers(self.max_retries()) + + # Basic requests client behavior tested in TestRequestsClient + def test_request(self, request_mock, mock_response, check_call): + pass + + def test_exception(self, request_mock, mock_error): + pass + + def test_timeout(self, request_mock, mock_response, check_call): + pass class TestUrlFetchClient(StripeClientTestCase, ClientTestBase): From 3a40824ad2dc5917e4f902103a606bcce75008d0 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 17:04:27 -0700 Subject: [PATCH 06/12] test exception retry handling --- stripe/http_client.py | 5 ++-- tests/test_api_requestor.py | 5 ++-- tests/test_http_client.py | 47 +++++++++++++++++++++++++++++-------- 3 files changed, 42 insertions(+), 15 deletions(-) diff --git a/stripe/http_client.py b/stripe/http_client.py index 195339c38..709439813 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -206,7 +206,8 @@ def request(self, method, url, headers, post_data=None): def _handle_request_error(self, e): - # Catch SSL error first as it belongs to ConnectionError + # Catch SSL error first as it belongs to ConnectionError, + # but we don't want to retry if isinstance(e, requests.exceptions.SSLError): msg = ("Could not verify Stripe's SSL certificate. Please make " "sure that your network is not intercepting certificates. " @@ -222,7 +223,7 @@ def _handle_request_error(self, e): "support@stripe.com.") err = "%s: %s" % (type(e).__name__, str(e)) should_retry = True - # Catch all request specific with descriptive class/messages + # Catch remaining request exceptions elif isinstance(e, requests.exceptions.RequestException): msg = ("Unexpected error communicating with Stripe. " "If this problem persists, let us know at " diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index c77e4a716..580fb6e1d 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -82,7 +82,6 @@ def _user_agent_match(self, other): def _idempotency_key_match(self, other): if self.idempotency_key is not None: return other['Idempotency-Key'] == self.idempotency_key - return True def _x_stripe_ua_contains_app_info(self, other): @@ -454,8 +453,8 @@ def test_uses_given_idempotency_key(self, requestor, mock_response, ) check_call(meth, headers=header_matcher, post_data='') - def test_create_uuid4_idempotency_key(self, requestor, mock_response, - check_call): + def test_uuid4_idempotency_key_when_not_given(self, requestor, + mock_response, check_call): mock_response('{}', 200) meth = 'post' requestor.request(meth, self.valid_path, {}) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 42f32b4b8..6f7114360 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -190,6 +190,7 @@ def mock_response(mock, body, code): @pytest.fixture def mock_error(self, mocker, session): def mock_error(mock): + # The first kind of request exceptions we catch mock.exceptions.SSLError = Exception session.request.side_effect = mock.exceptions.SSLError() mock.Session = mocker.MagicMock(return_value=session) @@ -231,7 +232,7 @@ def test_timeout(self, request_mock, mock_response, check_call): check_call(None, 'POST', self.valid_url, data, headers, timeout=5) -class TestRetryRequestClient(TestRequestsClient): +class TestRequestClientRetryBehavior(TestRequestsClient): @pytest.fixture def response(self, mocker): @@ -250,10 +251,8 @@ def mock_retry(retry_error_num=0, # Mocking classes of exception we catch # Any other exceptions with the same inheritance will work - # We consider request exceptions first, and differentiate which - # specific ones to retry. - root_error_class = Exception - request_mock.exceptions.RequestException = root_error_class + request_root_error_class = Exception + request_mock.exceptions.RequestException = request_root_error_class no_retry_parent_class = LookupError no_retry_child_class = KeyError @@ -272,7 +271,7 @@ def mock_retry(retry_error_num=0, responses request_mock.Session = mocker.MagicMock(return_value=session) - + return request_mock return mock_retry @pytest.fixture @@ -287,6 +286,7 @@ def make_request(self): client = self.REQUEST_CLIENT(verify_ssl_certs=True, timeout=80, proxy='http://slap/') + # Override sleep time to speed up tests client._sleep_time = lambda _: 0.001 return client.request_with_retry('GET', self.valid_url, {}, None) @@ -302,14 +302,14 @@ def test_retry_error_until_response(self, mock_retry, response, def test_retry_error_until_exceeded(self, mock_retry, response, check_call_numbers): - mock_retry(retry_error_num=3) + mock_retry(retry_error_num=self.max_retries()) with pytest.raises(stripe.error.APIConnectionError): self.make_request() check_call_numbers(self.max_retries()) def test_no_retry_error(self, mock_retry, response, check_call_numbers): - mock_retry(no_retry_error_num=3) + mock_retry(no_retry_error_num=self.max_retries()) with pytest.raises(stripe.error.APIConnectionError): self.make_request() check_call_numbers(1) @@ -322,12 +322,39 @@ def test_retry_codes(self, mock_retry, response, check_call_numbers): def test_retry_codes_until_exceeded(self, mock_retry, response, check_call_numbers): - mock_retry(responses=[response(code=409)] * 3) + mock_retry(responses=[response(code=409)] * self.max_retries()) _, code, _ = self.make_request() assert code == 409 check_call_numbers(self.max_retries()) - # Basic requests client behavior tested in TestRequestsClient + def test_handle_request_error_retry(self, mock_retry, session): + client = self.REQUEST_CLIENT() + request_mock = mock_retry(retry_error_num=1, no_retry_error_num=1) + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error(request_mock.exceptions.SSLError()) + assert error.value.should_retry is False + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error(request_mock.exceptions.Timeout()) + assert error.value.should_retry + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error( + request_mock.exceptions.ConnectionError()) + assert error.value.should_retry + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error( + request_mock.exceptions.RequestException()) + assert error.value.should_retry is False + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error(KeyError("")) + assert error.value.should_retry is False + + # Basic requests client behavior tested in TestRequestsClient + def test_request(self, request_mock, mock_response, check_call): pass From b0b321b2134a7a9f429f7bfcc81236e9b48484ce Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 17:16:19 -0700 Subject: [PATCH 07/12] standard request (with retry) calling delegated equest --- stripe/http_client.py | 15 ++++++++------- tests/test_error.py | 9 +++++++++ tests/test_http_client.py | 2 +- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/stripe/http_client.py b/stripe/http_client.py index 709439813..76c016f54 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -96,17 +96,18 @@ def __init__(self, verify_ssl_certs=True, proxy=None): " ""https"" and/or ""http"" keys.") self._proxy = proxy.copy() if proxy else None - def request(self, method, url, headers, post_data=None): + def delegated_request(self, method, url, headers, post_data=None): raise NotImplementedError( 'HTTPClient subclasses must implement `request`') - def request_with_retry(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): num_retries = 0 while True: try: num_retries += 1 - response = self.request(method, url, headers, post_data) + response = self.delegated_request(method, url, headers, + post_data) connection_error = None except error.APIConnectionError as e: connection_error = e @@ -165,7 +166,7 @@ def __init__(self, timeout=80, session=None, **kwargs): self._timeout = timeout self._session = session or requests.Session() - def request(self, method, url, headers, post_data=None): + def delegated_request(self, method, url, headers, post_data=None): kwargs = {} if self._verify_ssl_certs: kwargs['verify'] = os.path.join( @@ -271,7 +272,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None, deadline=55): # to 55 seconds to allow for a slow Stripe self._deadline = deadline - def request(self, method, url, headers, post_data=None): + def delegated_request(self, method, url, headers, post_data=None): try: result = urlfetch.fetch( url=url, @@ -337,7 +338,7 @@ def parse_headers(self, data): headers = email.message_from_string(raw_headers) return dict((k.lower(), v) for k, v in six.iteritems(dict(headers))) - def request(self, method, url, headers, post_data=None): + def delegated_request(self, method, url, headers, post_data=None): b = util.io.BytesIO() rheaders = util.io.BytesIO() @@ -446,7 +447,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None): proxy = urllib.request.ProxyHandler(self._proxy) self._opener = urllib.request.build_opener(proxy) - def request(self, method, url, headers, post_data=None): + def delegated_request(self, method, url, headers, post_data=None): if six.PY3 and isinstance(post_data, six.string_types): post_data = post_data.encode('utf-8') diff --git a/tests/test_error.py b/tests/test_error.py index 00f072cde..3f5d9d10a 100644 --- a/tests/test_error.py +++ b/tests/test_error.py @@ -62,3 +62,12 @@ def test_repr(self): assert repr(err) == \ "CardError(message='öre', param='cparam', code='ccode', " \ "http_status=403, request_id='123')" + + +class TestApiConnectionError(object): + def test_default_no_retry(self): + err = error.APIConnectionError('msg') + assert err.should_retry is False + + err = error.APIConnectionError('msg', should_retry=True) + assert err.should_retry diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 6f7114360..e9fda43fd 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -288,7 +288,7 @@ def make_request(self): proxy='http://slap/') # Override sleep time to speed up tests client._sleep_time = lambda _: 0.001 - return client.request_with_retry('GET', self.valid_url, {}, None) + return client.request('GET', self.valid_url, {}, None) def max_retries(self): return self.REQUEST_CLIENT.MAX_RETRIES From 8acc1cc1e94b6eb15cb2ca2b5a2b61d3851bbcb0 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Thu, 6 Sep 2018 17:40:58 -0700 Subject: [PATCH 08/12] add tests on retry conditions --- stripe/http_client.py | 47 ++++++++++++++++---------------- tests/test_http_client.py | 56 ++++++++++++++++++++++++++++++++++----- 2 files changed, 72 insertions(+), 31 deletions(-) diff --git a/stripe/http_client.py b/stripe/http_client.py index 76c016f54..56a1f30e1 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -79,7 +79,6 @@ def new_default_http_client(*args, **kwargs): class HTTPClient(object): - MAX_RETRIES = 3 MAX_DELAY = 2 INITIAL_DELAY = 0.5 @@ -96,24 +95,20 @@ def __init__(self, verify_ssl_certs=True, proxy=None): " ""https"" and/or ""http"" keys.") self._proxy = proxy.copy() if proxy else None - def delegated_request(self, method, url, headers, post_data=None): - raise NotImplementedError( - 'HTTPClient subclasses must implement `request`') - def request(self, method, url, headers, post_data=None): num_retries = 0 while True: try: num_retries += 1 - response = self.delegated_request(method, url, headers, - post_data) + response = self._delegated_request(method, url, headers, + post_data) connection_error = None except error.APIConnectionError as e: connection_error = e response = None - if self.should_retry(response, connection_error, num_retries): + if self._should_retry(response, connection_error, num_retries): self._sleep(num_retries) else: if response is not None: @@ -121,27 +116,21 @@ def request(self, method, url, headers, post_data=None): else: raise connection_error - def should_retry(self, response, connection_error, num_retries): + def _delegated_request(self, method, url, headers, post_data=None): + raise NotImplementedError( + 'HTTPClient subclasses must implement `request`') + + def _should_retry(self, response, api_connection_error, num_retries): if response is not None: _, status_code, _ = response should_retry = status_code == 409 else: - should_retry = connection_error.should_retry + should_retry = api_connection_error.should_retry return should_retry and num_retries < HTTPClient.MAX_RETRIES def _sleep(self, num_retries): time.sleep(self._sleep_time(num_retries)) - def close(self): - raise NotImplementedError( - 'HTTPClient subclasses must implement `close`') - - def _add_jitter_time(self, sleep_seconds): - # Randomize the value in [(sleep_seconds/ 2) to (sleep_seconds)] - # Also separated method here to isolate randomness for tests - sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) - return sleep_seconds - def _sleep_time(self, num_retries): # Apply exponential backoff with initial_network_retry_delay on the # number of num_retries so far as inputs. @@ -157,6 +146,16 @@ def _sleep_time(self, num_retries): return sleep_seconds + def _add_jitter_time(self, sleep_seconds): + # Randomize the value in [(sleep_seconds/ 2) to (sleep_seconds)] + # Also separated method here to isolate randomness for tests + sleep_seconds *= (0.5 * (1 + random.uniform(0, 1))) + return sleep_seconds + + def close(self): + raise NotImplementedError( + 'HTTPClient subclasses must implement `close`') + class RequestsClient(HTTPClient): name = 'requests' @@ -166,7 +165,7 @@ def __init__(self, timeout=80, session=None, **kwargs): self._timeout = timeout self._session = session or requests.Session() - def delegated_request(self, method, url, headers, post_data=None): + def _delegated_request(self, method, url, headers, post_data=None): kwargs = {} if self._verify_ssl_certs: kwargs['verify'] = os.path.join( @@ -272,7 +271,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None, deadline=55): # to 55 seconds to allow for a slow Stripe self._deadline = deadline - def delegated_request(self, method, url, headers, post_data=None): + def _delegated_request(self, method, url, headers, post_data=None): try: result = urlfetch.fetch( url=url, @@ -338,7 +337,7 @@ def parse_headers(self, data): headers = email.message_from_string(raw_headers) return dict((k.lower(), v) for k, v in six.iteritems(dict(headers))) - def delegated_request(self, method, url, headers, post_data=None): + def _delegated_request(self, method, url, headers, post_data=None): b = util.io.BytesIO() rheaders = util.io.BytesIO() @@ -447,7 +446,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None): proxy = urllib.request.ProxyHandler(self._proxy) self._opener = urllib.request.build_opener(proxy) - def delegated_request(self, method, url, headers, post_data=None): + def _delegated_request(self, method, url, headers, post_data=None): if six.PY3 and isinstance(post_data, six.string_types): post_data = post_data.encode('utf-8') diff --git a/tests/test_http_client.py b/tests/test_http_client.py index e9fda43fd..29e4a0122 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -103,6 +103,44 @@ def test_randomness_added(self): self.assert_sleep_times(client, 5, expected) +class TestRetryConditionsDefaultHttpClient(StripeClientTestCase): + + def test_should_retry_on_codes(self): + one_xx = list(range(100, 104)) + two_xx = list(range(200, 209)) + three_xx = list(range(300, 308)) + four_xx = list(range(400, 431)) + five_xx = list(range(500, 512)) + + client = stripe.http_client.new_default_http_client() + codes = one_xx + two_xx + three_xx + four_xx + five_xx + codes.remove(409) + + for code in codes: + assert client._should_retry((None, code, None), None, 1) is False + + def test_should_retry_on_error(self, mocker): + client = stripe.http_client.new_default_http_client() + api_connection_error = mocker.Mock() + + api_connection_error.should_retry = True + assert client._should_retry(None, api_connection_error, 1) is True + + api_connection_error.should_retry = False + assert client._should_retry(None, api_connection_error, 1) is False + + def test_should_retry_on_num_retries(self, mocker): + client = stripe.http_client.new_default_http_client() + api_connection_error = mocker.Mock() + api_connection_error.should_retry = True + + max_retries = stripe.http_client.HTTPClient.MAX_RETRIES + assert client._should_retry( + None, api_connection_error, max_retries + 1) is False + assert client._should_retry( + (None, 409, None), None, max_retries + 1) is False + + class ClientTestBase(object): @pytest.fixture def request_mock(self, request_mocks): @@ -266,7 +304,7 @@ def mock_retry(retry_error_num=0, retry_errors = [retry_child_class()] * retry_error_num # Include mock responses as possible side-effects - # Eg. return proper responses after some exceptions + # to simulate returning proper results after some exceptions session.request.side_effect = retry_errors + no_retry_errors + \ responses @@ -287,7 +325,7 @@ def make_request(self): timeout=80, proxy='http://slap/') # Override sleep time to speed up tests - client._sleep_time = lambda _: 0.001 + client._sleep_time = lambda _: 0.0001 return client.request('GET', self.valid_url, {}, None) def max_retries(self): @@ -331,10 +369,6 @@ def test_handle_request_error_retry(self, mock_retry, session): client = self.REQUEST_CLIENT() request_mock = mock_retry(retry_error_num=1, no_retry_error_num=1) - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error(request_mock.exceptions.SSLError()) - assert error.value.should_retry is False - with pytest.raises(stripe.error.APIConnectionError) as error: client._handle_request_error(request_mock.exceptions.Timeout()) assert error.value.should_retry @@ -344,6 +378,14 @@ def test_handle_request_error_retry(self, mock_retry, session): request_mock.exceptions.ConnectionError()) assert error.value.should_retry + def test_handle_request_error_no_retry(self, mock_retry, session): + client = self.REQUEST_CLIENT() + request_mock = mock_retry(retry_error_num=1, no_retry_error_num=1) + + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error(request_mock.exceptions.SSLError()) + assert error.value.should_retry is False + with pytest.raises(stripe.error.APIConnectionError) as error: client._handle_request_error( request_mock.exceptions.RequestException()) @@ -353,7 +395,7 @@ def test_handle_request_error_retry(self, mock_retry, session): client._handle_request_error(KeyError("")) assert error.value.should_retry is False - # Basic requests client behavior tested in TestRequestsClient + # Skip basic requests client behavior tested in TestRequestsClient def test_request(self, request_mock, mock_response, check_call): pass From 7c72d3b1caf96b2536821f9ce05d674e025ac9bb Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Fri, 7 Sep 2018 08:38:31 -0700 Subject: [PATCH 09/12] clean up --- tests/test_http_client.py | 64 ++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 31 deletions(-) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index 29e4a0122..ebcb66a17 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -55,8 +55,8 @@ def test_new_default_http_client_urllib2(self): class TestRetrySleepTimeDefaultHttpClient(StripeClientTestCase): from contextlib import contextmanager - def assert_sleep_times(self, client, until, expected): - assert len(expected) == until + def assert_sleep_times(self, client, expected): + until = len(expected) actual = list(map(lambda i: client._sleep_time(i+1), range(until))) assert expected == actual @@ -73,20 +73,20 @@ def test_sleep_time_exponential_back_off(self): client = stripe.http_client.new_default_http_client() client._add_jitter_time = lambda t: t with self.mock_max_delay(10): - self.assert_sleep_times(client, 5, [0.5, 1.0, 2.0, 4.0, 8.0]) + self.assert_sleep_times(client, [0.5, 1.0, 2.0, 4.0, 8.0]) def test_initial_delay_as_minimum(self): client = stripe.http_client.new_default_http_client() client._add_jitter_time = lambda t: t * 0.001 initial_delay = stripe.http_client.HTTPClient.INITIAL_DELAY - self.assert_sleep_times(client, 5, [initial_delay] * 5) + self.assert_sleep_times(client, [initial_delay] * 5) def test_maximum_delay(self): client = stripe.http_client.new_default_http_client() client._add_jitter_time = lambda t: t max_delay = stripe.http_client.HTTPClient.MAX_DELAY expected = [0.5, 1.0, max_delay, max_delay, max_delay] - self.assert_sleep_times(client, 5, expected) + self.assert_sleep_times(client, expected) def test_randomness_added(self): client = stripe.http_client.new_default_http_client() @@ -100,7 +100,7 @@ def test_randomness_added(self): base_value * 4, base_value * 8, base_value * 16] - self.assert_sleep_times(client, 5, expected) + self.assert_sleep_times(client, expected) class TestRetryConditionsDefaultHttpClient(StripeClientTestCase): @@ -287,8 +287,8 @@ def mock_retry(retry_error_num=0, no_retry_error_num=0, responses=[]): - # Mocking classes of exception we catch - # Any other exceptions with the same inheritance will work + # Mocking classes of exception we catch. Any group of exceptions + # with the same inheritance pattern will work request_root_error_class = Exception request_mock.exceptions.RequestException = request_root_error_class @@ -365,38 +365,40 @@ def test_retry_codes_until_exceeded(self, mock_retry, response, assert code == 409 check_call_numbers(self.max_retries()) - def test_handle_request_error_retry(self, mock_retry, session): + @pytest.fixture + def connection_error(self, session): client = self.REQUEST_CLIENT() - request_mock = mock_retry(retry_error_num=1, no_retry_error_num=1) - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error(request_mock.exceptions.Timeout()) - assert error.value.should_retry + def connection_error(given_exception): + with pytest.raises(stripe.error.APIConnectionError) as error: + client._handle_request_error(given_exception) + return error.value + return connection_error - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error( - request_mock.exceptions.ConnectionError()) - assert error.value.should_retry + def test_handle_request_error_should_retry(self, connection_error, + mock_retry): + request_mock = mock_retry() - def test_handle_request_error_no_retry(self, mock_retry, session): - client = self.REQUEST_CLIENT() - request_mock = mock_retry(retry_error_num=1, no_retry_error_num=1) + error = connection_error(request_mock.exceptions.Timeout()) + assert error.should_retry + + error = connection_error(request_mock.exceptions.ConnectionError()) + assert error.should_retry - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error(request_mock.exceptions.SSLError()) - assert error.value.should_retry is False + def test_handle_request_error_should_not_retry(self, connection_error, + mock_retry): + request_mock = mock_retry() - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error( - request_mock.exceptions.RequestException()) - assert error.value.should_retry is False + error = connection_error(request_mock.exceptions.SSLError()) + assert error.should_retry is False - with pytest.raises(stripe.error.APIConnectionError) as error: - client._handle_request_error(KeyError("")) - assert error.value.should_retry is False + error = connection_error(request_mock.exceptions.RequestException()) + assert error.should_retry is False - # Skip basic requests client behavior tested in TestRequestsClient + error = connection_error(KeyError("")) + assert error.should_retry is False + # Skip inherited basic requests client tests def test_request(self, request_mock, mock_response, check_call): pass From 34484eaa5b813812b275e6ace7547c5e752f0c2f Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Fri, 7 Sep 2018 10:21:54 -0700 Subject: [PATCH 10/12] move max retries to top-level place for config --- stripe/__init__.py | 1 + stripe/http_client.py | 9 ++++++--- tests/test_http_client.py | 20 ++++++++++++-------- 3 files changed, 19 insertions(+), 11 deletions(-) diff --git a/stripe/__init__.py b/stripe/__init__.py index c3d561b0a..c0f14086e 100644 --- a/stripe/__init__.py +++ b/stripe/__init__.py @@ -19,6 +19,7 @@ proxy = None default_http_client = None app_info = None +max_network_retries = 0 # Set to either 'debug' or 'info', controls console logging log = None diff --git a/stripe/http_client.py b/stripe/http_client.py index 56a1f30e1..f5e22cb5c 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -8,7 +8,7 @@ import time import random -from stripe import error, util, six +from stripe import error, util, six, max_network_retries # - Requests is the preferred HTTP library # - Google App Engine has urlfetch @@ -79,7 +79,6 @@ def new_default_http_client(*args, **kwargs): class HTTPClient(object): - MAX_RETRIES = 3 MAX_DELAY = 2 INITIAL_DELAY = 0.5 @@ -126,7 +125,11 @@ def _should_retry(self, response, api_connection_error, num_retries): should_retry = status_code == 409 else: should_retry = api_connection_error.should_retry - return should_retry and num_retries < HTTPClient.MAX_RETRIES + return should_retry and num_retries < self._max_network_retries() + + def _max_network_retries(self): + # Configured retries, isolated here for tests + return max_network_retries def _sleep(self, num_retries): time.sleep(self._sleep_time(num_retries)) diff --git a/tests/test_http_client.py b/tests/test_http_client.py index ebcb66a17..e79a712f1 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -121,24 +121,26 @@ def test_should_retry_on_codes(self): def test_should_retry_on_error(self, mocker): client = stripe.http_client.new_default_http_client() + client._max_network_retries = lambda: 1 api_connection_error = mocker.Mock() api_connection_error.should_retry = True - assert client._should_retry(None, api_connection_error, 1) is True + assert client._should_retry(None, api_connection_error, 0) is True api_connection_error.should_retry = False - assert client._should_retry(None, api_connection_error, 1) is False + assert client._should_retry(None, api_connection_error, 0) is False def test_should_retry_on_num_retries(self, mocker): client = stripe.http_client.new_default_http_client() + max_test_retries = 10 + client._max_network_retries = lambda: max_test_retries api_connection_error = mocker.Mock() api_connection_error.should_retry = True - max_retries = stripe.http_client.HTTPClient.MAX_RETRIES assert client._should_retry( - None, api_connection_error, max_retries + 1) is False + None, api_connection_error, max_test_retries + 1) is False assert client._should_retry( - (None, 409, None), None, max_retries + 1) is False + (None, 409, None), None, max_test_retries + 1) is False class ClientTestBase(object): @@ -320,17 +322,19 @@ def check_call_numbers(times): check_call(None, 'GET', valid_url, None, {}, times=times) return check_call_numbers + def max_retries(self): + return 3 + def make_request(self): client = self.REQUEST_CLIENT(verify_ssl_certs=True, timeout=80, proxy='http://slap/') # Override sleep time to speed up tests client._sleep_time = lambda _: 0.0001 + # Override configured max retries + client._max_network_retries = lambda: self.max_retries() return client.request('GET', self.valid_url, {}, None) - def max_retries(self): - return self.REQUEST_CLIENT.MAX_RETRIES - def test_retry_error_until_response(self, mock_retry, response, check_call_numbers): mock_retry(retry_error_num=1, responses=[response(code=202)]) From 73482edbd486d13c5376aa5f68384e119f07b035 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Fri, 7 Sep 2018 10:55:25 -0700 Subject: [PATCH 11/12] use explicit request_with_retries name --- stripe/api_requestor.py | 2 +- stripe/http_client.py | 15 +++++++-------- tests/test_api_requestor.py | 8 ++++---- tests/test_http_client.py | 28 ++++++++++++++++++++++------ 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/stripe/api_requestor.py b/stripe/api_requestor.py index ef89f7ad1..7a9fe1d54 100644 --- a/stripe/api_requestor.py +++ b/stripe/api_requestor.py @@ -282,7 +282,7 @@ def request_raw(self, method, url, params=None, supplied_headers=None): 'Post details', post_data=encoded_params, api_version=self.api_version) - rbody, rcode, rheaders = self._client.request( + rbody, rcode, rheaders = self._client.request_with_retries( method, abs_url, headers, post_data) util.log_info( diff --git a/stripe/http_client.py b/stripe/http_client.py index f5e22cb5c..43b426b19 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -94,14 +94,13 @@ def __init__(self, verify_ssl_certs=True, proxy=None): " ""https"" and/or ""http"" keys.") self._proxy = proxy.copy() if proxy else None - def request(self, method, url, headers, post_data=None): + def request_with_retries(self, method, url, headers, post_data=None): num_retries = 0 while True: try: num_retries += 1 - response = self._delegated_request(method, url, headers, - post_data) + response = self.request(method, url, headers, post_data) connection_error = None except error.APIConnectionError as e: connection_error = e @@ -115,7 +114,7 @@ def request(self, method, url, headers, post_data=None): else: raise connection_error - def _delegated_request(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): raise NotImplementedError( 'HTTPClient subclasses must implement `request`') @@ -168,7 +167,7 @@ def __init__(self, timeout=80, session=None, **kwargs): self._timeout = timeout self._session = session or requests.Session() - def _delegated_request(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): kwargs = {} if self._verify_ssl_certs: kwargs['verify'] = os.path.join( @@ -274,7 +273,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None, deadline=55): # to 55 seconds to allow for a slow Stripe self._deadline = deadline - def _delegated_request(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): try: result = urlfetch.fetch( url=url, @@ -340,7 +339,7 @@ def parse_headers(self, data): headers = email.message_from_string(raw_headers) return dict((k.lower(), v) for k, v in six.iteritems(dict(headers))) - def _delegated_request(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): b = util.io.BytesIO() rheaders = util.io.BytesIO() @@ -449,7 +448,7 @@ def __init__(self, verify_ssl_certs=True, proxy=None): proxy = urllib.request.ProxyHandler(self._proxy) self._opener = urllib.request.build_opener(proxy) - def _delegated_request(self, method, url, headers, post_data=None): + def request(self, method, url, headers, post_data=None): if six.PY3 and isinstance(post_data, six.string_types): post_data = post_data.encode('utf-8') diff --git a/tests/test_api_requestor.py b/tests/test_api_requestor.py index 580fb6e1d..ab1190004 100644 --- a/tests/test_api_requestor.py +++ b/tests/test_api_requestor.py @@ -221,7 +221,7 @@ def requestor(self, http_client): def mock_response(self, mocker, http_client): def mock_response(return_body, return_code, headers=None): print(return_code) - http_client.request = mocker.Mock( + http_client.request_with_retries = mocker.Mock( return_value=(return_body, return_code, headers or {})) return mock_response @@ -234,7 +234,7 @@ def check_call(method, abs_url=None, headers=None, if not headers: headers = APIHeaderMatcher(request_method=method) - http_client.request.assert_called_with( + http_client.request_with_retries.assert_called_with( method, abs_url, headers, post_data) return check_call @@ -583,12 +583,12 @@ def test_default_http_client_called(self, mocker): hc = mocker.Mock(stripe.http_client.HTTPClient) hc._verify_ssl_certs = True hc.name = 'mockclient' - hc.request = mocker.Mock(return_value=("{}", 200, {})) + hc.request_with_retries = mocker.Mock(return_value=("{}", 200, {})) stripe.default_http_client = hc stripe.Charge.list(limit=3) - hc.request.assert_called_with( + hc.request_with_retries.assert_called_with( 'get', 'https://api.stripe.com/v1/charges?limit=3', mocker.ANY, diff --git a/tests/test_http_client.py b/tests/test_http_client.py index e79a712f1..fd2b8bfe7 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -102,6 +102,16 @@ def test_randomness_added(self): base_value * 16] self.assert_sleep_times(client, expected) + def test_jitter_has_randomness_but_within_range(self): + client = stripe.http_client.new_default_http_client() + + jittered_ones = set( + map(lambda _: client._add_jitter_time(1), list(range(100))) + ) + + assert len(jittered_ones) > 1 + assert all(0.5 <= val <= 1 for val in jittered_ones) + class TestRetryConditionsDefaultHttpClient(StripeClientTestCase): @@ -154,7 +164,7 @@ def valid_url(self, path='/foo'): def make_request(self, method, url, headers, post_data): client = self.REQUEST_CLIENT(verify_ssl_certs=True) - return client.request(method, url, headers, post_data) + return client.request_with_retries(method, url, headers, post_data) @pytest.fixture def mock_response(self): @@ -260,7 +270,7 @@ def make_request(self, method, url, headers, post_data, timeout=80): client = self.REQUEST_CLIENT(verify_ssl_certs=True, timeout=timeout, proxy='http://slap/') - return client.request(method, url, headers, post_data) + return client.request_with_retries(method, url, headers, post_data) def test_timeout(self, request_mock, mock_response, check_call): headers = {'my-header': 'header val'} @@ -333,7 +343,7 @@ def make_request(self): client._sleep_time = lambda _: 0.0001 # Override configured max retries client._max_network_retries = lambda: self.max_retries() - return client.request('GET', self.valid_url, {}, None) + return client.request_with_retries('GET', self.valid_url, {}, None) def test_retry_error_until_response(self, mock_retry, response, check_call_numbers): @@ -395,12 +405,16 @@ def test_handle_request_error_should_not_retry(self, connection_error, error = connection_error(request_mock.exceptions.SSLError()) assert error.should_retry is False + assert 'not verify Stripe\'s SSL certificate' in error.user_message error = connection_error(request_mock.exceptions.RequestException()) assert error.should_retry is False - error = connection_error(KeyError("")) + # Mimic non-requests exception as not being children of Exception, + # See mock_retry for the exceptions setup + error = connection_error(BaseException("")) assert error.should_retry is False + assert 'configuration issue locally' in error.user_message # Skip inherited basic requests client tests def test_request(self, request_mock, mock_response, check_call): @@ -454,7 +468,8 @@ def make_request(self, method, url, headers, post_data, proxy=None): self.client = self.REQUEST_CLIENT(verify_ssl_certs=True, proxy=proxy) self.proxy = proxy - return self.client.request(method, url, headers, post_data) + return self.client.request_with_retries(method, url, headers, + post_data) @pytest.fixture def mock_response(self, mocker): @@ -529,7 +544,8 @@ def make_request(self, method, url, headers, post_data, proxy=None): self.client = self.REQUEST_CLIENT(verify_ssl_certs=True, proxy=proxy) self.proxy = proxy - return self.client.request(method, url, headers, post_data) + return self.client.request_with_retries(method, url, headers, + post_data) @pytest.fixture def curl_mock(self, mocker): From 5e002cd7cb86c52c2bd8c9ddb2fbd1facadd2703 Mon Sep 17 00:00:00 2001 From: Mick Jermsurawong Date: Fri, 7 Sep 2018 13:15:49 -0700 Subject: [PATCH 12/12] add logging and explain retry logic design in comment --- stripe/http_client.py | 20 ++++++++++++++------ tests/test_http_client.py | 3 ++- 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/stripe/http_client.py b/stripe/http_client.py index 43b426b19..504eec657 100644 --- a/stripe/http_client.py +++ b/stripe/http_client.py @@ -107,7 +107,15 @@ def request_with_retries(self, method, url, headers, post_data=None): response = None if self._should_retry(response, connection_error, num_retries): - self._sleep(num_retries) + if connection_error: + util.log_info("Encountered a retryable error %s" % + connection_error.user_message) + + sleep_time = self._sleep_time_seconds(num_retries) + util.log_info(("Initiating retry %i for request %s %s after " + "sleeping %.2f seconds." % + (num_retries, method, url, sleep_time))) + time.sleep(sleep_time) else: if response is not None: return response @@ -123,6 +131,10 @@ def _should_retry(self, response, api_connection_error, num_retries): _, status_code, _ = response should_retry = status_code == 409 else: + # We generally want to retry on timeout and connection + # exceptions, but defer this decision to underlying subclass + # implementations. They should evaluate the driver-specific + # errors worthy of retries, and set flag on the error returned. should_retry = api_connection_error.should_retry return should_retry and num_retries < self._max_network_retries() @@ -130,10 +142,7 @@ def _max_network_retries(self): # Configured retries, isolated here for tests return max_network_retries - def _sleep(self, num_retries): - time.sleep(self._sleep_time(num_retries)) - - def _sleep_time(self, num_retries): + def _sleep_time_seconds(self, num_retries): # Apply exponential backoff with initial_network_retry_delay on the # number of num_retries so far as inputs. # Do not allow the number to exceed max_network_retry_delay. @@ -145,7 +154,6 @@ def _sleep_time(self, num_retries): # But never sleep less than the base sleep seconds. sleep_seconds = max(HTTPClient.INITIAL_DELAY, sleep_seconds) - return sleep_seconds def _add_jitter_time(self, sleep_seconds): diff --git a/tests/test_http_client.py b/tests/test_http_client.py index fd2b8bfe7..f58acd434 100644 --- a/tests/test_http_client.py +++ b/tests/test_http_client.py @@ -57,7 +57,8 @@ class TestRetrySleepTimeDefaultHttpClient(StripeClientTestCase): def assert_sleep_times(self, client, expected): until = len(expected) - actual = list(map(lambda i: client._sleep_time(i+1), range(until))) + actual = list( + map(lambda i: client._sleep_time_seconds(i + 1), range(until))) assert expected == actual @contextmanager