Skip to content

Commit

Permalink
fix: backoff and jitter in new http wrapper (#418)
Browse files Browse the repository at this point in the history
This behavior was present in the old decorator, and is needed for some
providers.

Signed-off-by: Will Murphy <[email protected]>
  • Loading branch information
willmurphyscode authored Dec 8, 2023
1 parent 0190558 commit 63f56f1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 8 deletions.
14 changes: 9 additions & 5 deletions src/vunnel/utils/http.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import random
import time
from collections.abc import Callable
from typing import Any, Optional
Expand Down Expand Up @@ -42,9 +43,12 @@ def get( # noqa: PLR0913
"""
logger.debug(f"http GET {url}")
last_exception: Exception | None = None
sleep_interval = backoff_in_seconds
for attempt in range(retries + 1):
if last_exception:
time.sleep(backoff_in_seconds)
time.sleep(sleep_interval)
sleep_interval = backoff_in_seconds * 2**attempt + random.uniform(0, 1) # noqa: S311
# explanation of S311 disable: rng is not used cryptographically
try:
response = requests.get(url, timeout=timeout, **kwargs)
if status_handler:
Expand All @@ -56,17 +60,17 @@ def get( # noqa: PLR0913
last_exception = e
will_retry = ""
if attempt < retries:
will_retry = f" (will retry in {backoff_in_seconds} seconds) "
will_retry = f" (will retry in {int(backoff_in_seconds)} seconds) "
# HTTPError includes the attempted request, so don't include it redundantly here
logger.warning(f"attempt {attempt + 1} of {retries} failed:{will_retry}{e}")
logger.warning(f"attempt {attempt + 1} of {retries + 1} failed:{will_retry}{e}")
except Exception as e:
last_exception = e
will_retry = ""
if attempt < retries:
will_retry = f" (will retry in {backoff_in_seconds} seconds) "
will_retry = f" (will retry in {int(sleep_interval)} seconds) "
# this is an unexpected exception type, so include the attempted request in case the
# message from the unexpected exception doesn't.
logger.warning(f"attempt {attempt + 1} of {retries}{will_retry}: unexpected exception during GET {url}: {e}")
logger.warning(f"attempt {attempt + 1} of {retries + 1}{will_retry}: unexpected exception during GET {url}: {e}")
if last_exception:
logger.error(f"last retry of GET {url} failed with {last_exception}")
raise last_exception
Expand Down
24 changes: 21 additions & 3 deletions tests/unit/utils/test_http.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,20 @@ def test_raises_when_out_of_retries(self, mock_requests, mock_sleep, mock_logger
http.get("http://example.com/some-path", mock_logger, retries=2, backoff_in_seconds=3)
mock_logger.error.assert_called()

@patch("time.sleep")
@patch("requests.get")
def test_correct_number_of_retria(self, mock_requests, mock_sleep, mock_logger, error_response):
mock_requests.side_effect = [
error_response,
error_response,
error_response,
error_response,
error_response,
] # more than enough
with pytest.raises(requests.HTTPError):
http.get("http://example.com/some-path", mock_logger, retries=2, backoff_in_seconds=3)
assert len(mock_requests.call_args_list) == 3 # once for initial plus two retries

@patch("time.sleep")
@patch("requests.get")
def test_succeeds_if_retries_succeed(self, mock_requests, mock_sleep, mock_logger, error_response, success_response):
Expand All @@ -53,10 +67,14 @@ def test_timeout_is_passed_in(self, mock_requests, mock_logger):

@patch("time.sleep")
@patch("requests.get")
def test_sleeps_right_amount_between_retries(self, mock_requests, mock_sleep, mock_logger, error_response, success_response):
@patch("random.uniform")
def test_exponential_backoff_and_jitter(
self, mock_uniform_random, mock_requests, mock_sleep, mock_logger, error_response, success_response
):
mock_requests.side_effect = [error_response, error_response, error_response, success_response]
http.get("http://example.com/some-path", mock_logger, backoff_in_seconds=123, retries=3)
assert mock_sleep.call_args_list == [call(123), call(123), call(123)]
mock_uniform_random.side_effect = [0.5, 0.4, 0.1]
http.get("http://example.com/some-path", mock_logger, backoff_in_seconds=10, retries=3)
assert mock_sleep.call_args_list == [call(10), call(10 * 2 + 0.5), call(10 * 4 + 0.4)]

@patch("time.sleep")
@patch("requests.get")
Expand Down

0 comments on commit 63f56f1

Please sign in to comment.