Improve implementation of respect_retry_after_header
MODIFY
CHANGES.rst
CHANGES.rst
Changes
=======
+dev (master)
+------------
+
+* Propagate Retry-After header settings to subsequent retries. (Pull #1607)
+
+* Fix edge case where Retry-After header was still respected even when
+ explicitly opted out of. (Pull #1607)
+
+
1.25.3 (2019-05-23)
-------------------

MODIFY
CONTRIBUTORS.txt
CONTRIBUTORS.txt
@@ -275,5 +275,8 @@ In chronological order:
* Katsuhiko YOSHIDA <https://github.com/kyoshidajp>
* Remove Authorization header regardless of case when redirecting to cross-site
+* James Meickle <https://permadeath.com/>
+ * Improve handling of Retry-After header
+
* [Your name or handle] <[email or website]>
* [Brief summary of your changes]

MODIFY
src/urllib3/util/retry.py
src/urllib3/util/retry.py
@@ -210,6 +210,7 @@ class Retry(object):
raise_on_status=self.raise_on_status,
history=self.history,
remove_headers_on_redirect=self.remove_headers_on_redirect,
+ respect_retry_after_header=self.respect_retry_after_header,
)
params.update(kw)
return type(self)(**params)
@@ -294,7 +295,7 @@ class Retry(object):
this method will return immediately.
"""
- if response:
+ if self.respect_retry_after_header and response:
slept = self.sleep_for_retry(response)
if slept:
return

MODIFY
test/test_retry.py
test/test_retry.py
+import datetime
+import mock
import pytest
+import time
from urllib3.response import HTTPResponse
+from urllib3.packages import six
from urllib3.packages.six.moves import xrange
from urllib3.util.retry import Retry, RequestHistory
from urllib3.exceptions import (
ConnectTimeoutError,
+ InvalidHeader,
MaxRetryError,
ReadTimeoutError,
ResponseError,
@@ -271,3 +276,68 @@ class TestRetry(object):
retry = Retry(remove_headers_on_redirect=["X-API-Secret"])
assert list(retry.remove_headers_on_redirect) == ["x-api-secret"]
+
+ @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")]) # \xb2 = ^2
+ def test_parse_retry_after_invalid(self, value):
+ retry = Retry()
+ with pytest.raises(InvalidHeader):
+ retry.parse_retry_after(value)
+
+ @pytest.mark.parametrize(
+ "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)]
+ )
+ def test_parse_retry_after(self, value, expected):
+ retry = Retry()
+ assert retry.parse_retry_after(value) == expected
+
+ @pytest.mark.parametrize("respect_retry_after_header", [True, False])
+ def test_respect_retry_after_header_propagated(self, respect_retry_after_header):
+
+ retry = Retry(respect_retry_after_header=respect_retry_after_header)
+ new_retry = retry.new()
+ assert new_retry.respect_retry_after_header == respect_retry_after_header
+
+ @pytest.mark.parametrize(
+ "retry_after_header,respect_retry_after_header,sleep_duration",
+ [
+ ("3600", True, 3600),
+ ("3600", False, None),
+ # Will sleep due to header is 1 hour in future
+ ("Mon, 3 Jun 2019 12:00:00 UTC", True, 3600),
+ # Won't sleep due to not respecting header
+ ("Mon, 3 Jun 2019 12:00:00 UTC", False, None),
+ # Won't sleep due to current time reached
+ ("Mon, 3 Jun 2019 11:00:00 UTC", True, None),
+ # Won't sleep due to current time reached + not respecting header
+ ("Mon, 3 Jun 2019 11:00:00 UTC", False, None),
+ ],
+ )
+ def test_respect_retry_after_header_sleep(
+ self, retry_after_header, respect_retry_after_header, sleep_duration
+ ):
+ retry = Retry(respect_retry_after_header=respect_retry_after_header)
+
+ # Date header syntax can specify an absolute date; compare this to the
+ # time in the parametrized inputs above.
+ current_time = mock.MagicMock(
+ return_value=time.mktime(
+ datetime.datetime(year=2019, month=6, day=3, hour=11).timetuple()
+ )
+ )
+
+ with mock.patch("time.sleep") as sleep_mock, mock.patch(
+ "time.time", current_time
+ ):
+ # for the default behavior, it must be in RETRY_AFTER_STATUS_CODES
+ response = HTTPResponse(
+ status=503, headers={"Retry-After": retry_after_header}
+ )
+
+ retry.sleep(response)
+
+ # The expected behavior is that we'll only sleep if respecting
+ # this header (since we won't have any backoff sleep attempts)
+ if respect_retry_after_header and sleep_duration is not None:
+ sleep_mock.assert_called_with(sleep_duration)
+ else:
+ sleep_mock.assert_not_called()

MODIFY
test/test_util.py
test/test_util.py
@@ -13,7 +13,6 @@ import pytest
from urllib3 import add_stderr_logger, disable_warnings
from urllib3.util.request import make_headers, rewind_body, _FAILEDTELL
from urllib3.util.response import assert_header_parsing
-from urllib3.util.retry import Retry
from urllib3.util.timeout import Timeout
from urllib3.util.url import get_host, parse_url, split_first, Url
from urllib3.util.ssl_ import (
@@ -27,7 +26,6 @@ from urllib3.exceptions import (
TimeoutStateError,
InsecureRequestWarning,
SNIMissingWarning,
- InvalidHeader,
UnrewindableBodyError,
)
from urllib3.util.connection import allowed_gai_family, _has_ipv6
@@ -782,19 +780,6 @@ class TestUtil(object):
with patch("urllib3.util.connection.HAS_IPV6", False):
assert allowed_gai_family() == socket.AF_INET
- @pytest.mark.parametrize("value", ["-1", "+1", "1.0", six.u("\xb2")]) # \xb2 = ^2
- def test_parse_retry_after_invalid(self, value):
- retry = Retry()
- with pytest.raises(InvalidHeader):
- retry.parse_retry_after(value)
-
- @pytest.mark.parametrize(
- "value, expected", [("0", 0), ("1000", 1000), ("\t42 ", 42)]
- )
- def test_parse_retry_after(self, value, expected):
- retry = Retry()
- assert retry.parse_retry_after(value) == expected
-
@pytest.mark.parametrize("headers", [b"foo", None, object])
def test_assert_header_parsing_throws_typeerror_with_non_headers(self, headers):
with pytest.raises(TypeError):

Improve implementation of respect_retry_after_header (#1607)