From 7c9e8fc3b27cc1799d0a288a05812c869df1313a Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Mon, 17 Aug 2020 12:50:51 +0200 Subject: [PATCH 01/13] Add connection retries --- httpx/__init__.py | 3 +- httpx/_client.py | 64 +++++++++++++- httpx/_config.py | 30 ++++++- httpx/_types.py | 4 +- httpx/_utils.py | 21 +++++ tests/client/test_retries.py | 160 +++++++++++++++++++++++++++++++++++ 6 files changed, 277 insertions(+), 5 deletions(-) create mode 100644 tests/client/test_retries.py diff --git a/httpx/__init__.py b/httpx/__init__.py index 3802439466..92eee528eb 100644 --- a/httpx/__init__.py +++ b/httpx/__init__.py @@ -2,7 +2,7 @@ from ._api import delete, get, head, options, patch, post, put, request, stream from ._auth import Auth, BasicAuth, DigestAuth from ._client import AsyncClient, Client -from ._config import Limits, PoolLimits, Proxy, Timeout, create_ssl_context +from ._config import Limits, Retries, PoolLimits, Proxy, Timeout, create_ssl_context from ._exceptions import ( CloseError, ConnectError, @@ -91,6 +91,7 @@ "Response", "ResponseClosed", "ResponseNotRead", + "Retries", "StatusCode", "stream", "StreamConsumed", diff --git a/httpx/_client.py b/httpx/_client.py index 53d52e5123..0d87b5490e 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -1,4 +1,5 @@ import functools +import time import typing from types import TracebackType @@ -8,10 +9,12 @@ from ._config import ( DEFAULT_LIMITS, DEFAULT_MAX_REDIRECTS, + DEFAULT_RETRIES, DEFAULT_TIMEOUT_CONFIG, UNSET, Limits, Proxy, + Retries, Timeout, UnsetType, create_ssl_context, @@ -19,6 +22,8 @@ from ._content_streams import ContentStream from ._exceptions import ( HTTPCORE_EXC_MAP, + ConnectError, + ConnectTimeout, InvalidURL, RemoteProtocolError, RequestBodyUnavailable, @@ -38,6 +43,7 @@ QueryParamTypes, RequestData, RequestFiles, + RetriesTypes, TimeoutTypes, URLTypes, VerifyTypes, @@ -45,9 +51,11 @@ from ._utils import ( NetRCInfo, URLPattern, + exponential_backoff, get_environment_proxies, get_logger, same_origin, + sleep, warn_deprecated, ) @@ -66,6 +74,7 @@ def __init__( cookies: CookieTypes = None, timeout: TimeoutTypes = DEFAULT_TIMEOUT_CONFIG, max_redirects: int = DEFAULT_MAX_REDIRECTS, + retries: RetriesTypes = DEFAULT_RETRIES, base_url: URLTypes = "", trust_env: bool = True, ): @@ -77,6 +86,7 @@ def __init__( self._cookies = Cookies(cookies) self._timeout = Timeout(timeout) self.max_redirects = max_redirects + self._retries = Retries(retries) self._trust_env = trust_env self._netrc = NetRCInfo() @@ -117,6 +127,14 @@ def timeout(self) -> Timeout: def timeout(self, timeout: TimeoutTypes) -> None: self._timeout = Timeout(timeout) + @property + def retries(self) -> Retries: + return self._retries + + @retries.setter + def retries(self, retries: RetriesTypes) -> None: + self._retries = Retries(retries) + @property def base_url(self) -> URL: """ @@ -453,6 +471,8 @@ class Client(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. + * **retries** - *(optional)* The maximum number of retries when trying to + establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. * **transport** - *(optional)* A transport class to use for sending requests @@ -478,6 +498,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, + retries: RetriesTypes = DEFAULT_RETRIES, base_url: URLTypes = "", transport: httpcore.SyncHTTPTransport = None, app: typing.Callable = None, @@ -490,6 +511,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, + retries=retries, base_url=base_url, trust_env=trust_env, ) @@ -735,7 +757,7 @@ def _send_handling_auth( auth_flow = auth.auth_flow(request) request = next(auth_flow) while True: - response = self._send_single_request(request, timeout) + response = self._send_handling_retries(request, timeout) if auth.requires_response_body: response.read() try: @@ -751,6 +773,22 @@ def _send_handling_auth( request = next_request history.append(response) + def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response: + attempts = self.retries.max_attempts + delays = exponential_backoff(factor=self.retries.backoff_factor) + + while True: + try: + return self._send_single_request(request, timeout) + except (ConnectError, ConnectTimeout): + if request.method in ("POST", "PATCH"): + raise + if attempts <= 0: + raise + attempts -= 1 + delay = next(delays) + time.sleep(delay) + def _send_single_request(self, request: Request, timeout: Timeout) -> Response: """ Sends a single request, without handling any redirections. @@ -1053,6 +1091,8 @@ class AsyncClient(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. + * **retries** - *(optional)* The maximum number of retries when trying to + establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. * **transport** - *(optional)* A transport class to use for sending requests @@ -1078,6 +1118,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, + retries: RetriesTypes = DEFAULT_RETRIES, base_url: URLTypes = "", transport: httpcore.AsyncHTTPTransport = None, app: typing.Callable = None, @@ -1090,6 +1131,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, + retries=retries, base_url=base_url, trust_env=trust_env, ) @@ -1337,7 +1379,7 @@ async def _send_handling_auth( auth_flow = auth.auth_flow(request) request = next(auth_flow) while True: - response = await self._send_single_request(request, timeout) + response = await self._send_handling_retries(request, timeout) if auth.requires_response_body: await response.aread() try: @@ -1353,6 +1395,24 @@ async def _send_handling_auth( request = next_request history.append(response) + async def _send_handling_retries( + self, request: Request, timeout: Timeout + ) -> Response: + attempts = self.retries.max_attempts + delays = exponential_backoff(factor=self.retries.backoff_factor) + + while True: + try: + return await self._send_single_request(request, timeout) + except (ConnectError, ConnectTimeout): + if request.method in ("POST", "PATCH"): + raise + if attempts <= 0: + raise + attempts -= 1 + delay = next(delays) + await sleep(delay) + async def _send_single_request( self, request: Request, timeout: Timeout, ) -> Response: diff --git a/httpx/_config.py b/httpx/_config.py index 4284a329f9..0cdaef3769 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -8,7 +8,14 @@ import certifi from ._models import URL, Headers -from ._types import CertTypes, HeaderTypes, TimeoutTypes, URLTypes, VerifyTypes +from ._types import ( + CertTypes, + HeaderTypes, + RetriesTypes, + TimeoutTypes, + URLTypes, + VerifyTypes, +) from ._utils import get_ca_bundle_from_env, get_logger, warn_deprecated DEFAULT_CIPHERS = ":".join( @@ -410,6 +417,27 @@ def __repr__(self) -> str: ) +class Retries: + def __init__(self, retries: RetriesTypes, *, backoff_factor: float = 0.2) -> None: + if isinstance(retries, Retries): + self.max_attempts = retries.max_attempts # type: int + self.backoff_factor = retries.backoff_factor # type: float + else: + self.max_attempts = retries + self.backoff_factor = backoff_factor + + def __eq__(self, other: typing.Any) -> bool: + return ( + isinstance(other, self.__class__) + and self.max_attempts == other.max_attempts + and self.backoff_factor == other.backoff_factor + ) + + def __repr__(self) -> str: + return f"Retries({self.max_attempts}, backoff_factor={self.backoff_factor})" + + DEFAULT_TIMEOUT_CONFIG = Timeout(timeout=5.0) DEFAULT_LIMITS = Limits(max_connections=100, max_keepalive_connections=20) DEFAULT_MAX_REDIRECTS = 20 +DEFAULT_RETRIES = Retries(0) diff --git a/httpx/_types.py b/httpx/_types.py index 437d601c90..181d1ca1d0 100644 --- a/httpx/_types.py +++ b/httpx/_types.py @@ -21,7 +21,7 @@ if TYPE_CHECKING: # pragma: no cover from ._auth import Auth # noqa: F401 - from ._config import Proxy, Timeout # noqa: F401 + from ._config import Proxy, Retries, Timeout # noqa: F401 from ._models import URL, Cookies, Headers, QueryParams, Request # noqa: F401 @@ -63,6 +63,8 @@ None, ] +RetriesTypes = Union[int, "Retries"] + RequestData = Union[dict, str, bytes, Iterator[bytes], AsyncIterator[bytes]] FileContent = Union[IO[str], IO[bytes], str, bytes] diff --git a/httpx/_utils.py b/httpx/_utils.py index 8080f63a46..703bb8279a 100644 --- a/httpx/_utils.py +++ b/httpx/_utils.py @@ -1,5 +1,7 @@ +import asyncio import codecs import collections +import itertools import logging import mimetypes import netrc @@ -14,6 +16,8 @@ from types import TracebackType from urllib.request import getproxies +import sniffio + from ._types import PrimitiveData if typing.TYPE_CHECKING: # pragma: no cover @@ -529,3 +533,20 @@ def __eq__(self, other: typing.Any) -> bool: def warn_deprecated(message: str) -> None: # pragma: nocover warnings.warn(message, DeprecationWarning, stacklevel=2) + + +async def sleep(seconds: float) -> None: + library = sniffio.current_async_library() + if library == "trio": + import trio + + await trio.sleep(seconds) + else: + assert library == "asyncio" + await asyncio.sleep(seconds) + + +def exponential_backoff(factor: float) -> typing.Iterator[float]: + yield 0 + for n in itertools.count(2): + yield factor * (2 ** (n - 2)) diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py new file mode 100644 index 0000000000..a09f01e0f6 --- /dev/null +++ b/tests/client/test_retries.py @@ -0,0 +1,160 @@ +import collections +import itertools +from typing import List, Tuple, Type, Dict, Mapping, Optional + +import pytest + +import httpcore +import httpx +from httpx._utils import exponential_backoff + + +def test_retries_config() -> None: + client = httpx.AsyncClient() + assert client.retries == httpx.Retries(0) + assert client.retries.max_attempts == 0 + + client = httpx.AsyncClient(retries=3) + assert client.retries == httpx.Retries(3) + assert client.retries.max_attempts == 3 + assert client.retries.backoff_factor == 0.2 + + client = httpx.AsyncClient(retries=httpx.Retries(3, backoff_factor=0.1)) + assert client.retries == httpx.Retries(3, backoff_factor=0.1) + assert client.retries.max_attempts == 3 + assert client.retries.backoff_factor == 0.1 + + +class AsyncMockTransport(httpcore.AsyncHTTPTransport): + _ENDPOINTS: Dict[bytes, Tuple[Type[Exception], bool]] = { + b"/connect_timeout": (httpcore.ConnectTimeout, True), + b"/connect_error": (httpcore.ConnectError, True), + b"/read_timeout": (httpcore.ReadTimeout, False), + b"/network_error": (httpcore.NetworkError, False), + } + + def __init__(self, succeed_after: int) -> None: + self._succeed_after = succeed_after + self._path_attempts: Dict[bytes, int] = collections.defaultdict(int) + + async def request( + self, + method: bytes, + url: Tuple[bytes, bytes, Optional[int], bytes], + headers: List[Tuple[bytes, bytes]] = None, + stream: httpcore.AsyncByteStream = None, + timeout: Mapping[str, Optional[float]] = None, + ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.AsyncByteStream]: + scheme, host, port, path = url + + if path not in self._ENDPOINTS: + stream = httpcore.PlainByteStream(b"") + return (b"HTTP/1.1", 200, b"OK", [], stream) + + exc_cls, is_retryable = self._ENDPOINTS[path] + + if not is_retryable: + raise exc_cls() + + if self._path_attempts[path] < self._succeed_after: + self._path_attempts[path] += 1 + raise exc_cls() + + stream = httpcore.PlainByteStream(b"") + return (b"HTTP/1.1", 200, b"OK", [], stream) + + +@pytest.mark.usefixtures("async_environment") +async def test_no_retries() -> None: + client = httpx.AsyncClient(transport=AsyncMockTransport(succeed_after=1)) + + response = await client.get("https://example.com") + assert response.status_code == 200 + + with pytest.raises(httpx.ConnectTimeout): + await client.get("https://example.com/connect_timeout") + + with pytest.raises(httpx.ConnectError): + await client.get("https://example.com/connect_error") + + with pytest.raises(httpx.ReadTimeout): + await client.get("https://example.com/read_timeout") + + with pytest.raises(httpx.NetworkError): + await client.get("https://example.com/network_error") + + +@pytest.mark.usefixtures("async_environment") +async def test_retries_enabled() -> None: + transport = AsyncMockTransport(succeed_after=3) + client = httpx.AsyncClient( + transport=transport, retries=httpx.Retries(3, backoff_factor=0.01) + ) + + response = await client.get("https://example.com") + assert response.status_code == 200 + + response = await client.get("https://example.com/connect_timeout") + assert response.status_code == 200 + + response = await client.get("https://example.com/connect_error") + assert response.status_code == 200 + + with pytest.raises(httpx.ReadTimeout): + await client.get("https://example.com/read_timeout") + + with pytest.raises(httpx.NetworkError): + await client.get("https://example.com/network_error") + + +@pytest.mark.usefixtures("async_environment") +async def test_retries_exceeded() -> None: + transport = AsyncMockTransport(succeed_after=2) + client = httpx.AsyncClient(transport=transport, retries=1) + with pytest.raises(httpx.ConnectTimeout): + await client.get("https://example.com/connect_timeout") + + +@pytest.mark.usefixtures("async_environment") +@pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) +async def test_retries_idempotent_methods(method: str) -> None: + transport = AsyncMockTransport(succeed_after=1) + client = httpx.AsyncClient(transport=transport, retries=1) + response = await client.request(method, "https://example.com/connect_timeout") + assert response.status_code == 200 + + +@pytest.mark.usefixtures("async_environment") +@pytest.mark.parametrize("method", ["POST", "PATCH"]) +async def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: + """ + Non-idempotent HTTP verbs should never be retried on. + """ + transport = AsyncMockTransport(succeed_after=1) + client = httpx.AsyncClient(transport=transport, retries=2) + + with pytest.raises(httpx.ConnectTimeout): + await client.request(method, "https://example.com/connect_timeout") + + +@pytest.mark.parametrize( + "factor, expected", + [ + (0.1, [0, 0.1, 0.2, 0.4, 0.8]), + (0.2, [0, 0.2, 0.4, 0.8, 1.6]), + (0.5, [0, 0.5, 1.0, 2.0, 4.0]), + ], +) +def test_exponential_backoff(factor: float, expected: List[int]) -> None: + delays = list(itertools.islice(exponential_backoff(factor), 5)) + assert delays == expected + + +@pytest.mark.usefixtures("async_environment") +async def test_retries_backoff() -> None: + retries = httpx.Retries(3, backoff_factor=0.05) + transport = AsyncMockTransport(succeed_after=3) + client = httpx.AsyncClient(transport=transport, retries=retries) + response = await client.get("https://example.com/connect_timeout") + assert response.status_code == 200 + assert response.elapsed.total_seconds() == pytest.approx(0 + 0.05 + 0.1, rel=0.1) From 66986fa189d7044a39f673d872cb9279e6413031 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Wed, 19 Aug 2020 09:58:46 +0200 Subject: [PATCH 02/13] Update tests --- httpx/_client.py | 1 + tests/client/test_retries.py | 97 ++++++++++++++++++++++-------------- 2 files changed, 61 insertions(+), 37 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index c3895b89b5..375c025c3a 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -127,6 +127,7 @@ def timeout(self) -> Timeout: def timeout(self, timeout: TimeoutTypes) -> None: self._timeout = Timeout(timeout) + @property def auth(self) -> typing.Optional[Auth]: """ Authentication class used when none is passed at the request-level. diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index a09f01e0f6..b405837628 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -1,6 +1,6 @@ import collections import itertools -from typing import List, Tuple, Type, Dict, Mapping, Optional +from typing import List, Tuple, Dict, Mapping, Optional import pytest @@ -26,16 +26,9 @@ def test_retries_config() -> None: class AsyncMockTransport(httpcore.AsyncHTTPTransport): - _ENDPOINTS: Dict[bytes, Tuple[Type[Exception], bool]] = { - b"/connect_timeout": (httpcore.ConnectTimeout, True), - b"/connect_error": (httpcore.ConnectError, True), - b"/read_timeout": (httpcore.ReadTimeout, False), - b"/network_error": (httpcore.NetworkError, False), - } - - def __init__(self, succeed_after: int) -> None: - self._succeed_after = succeed_after - self._path_attempts: Dict[bytes, int] = collections.defaultdict(int) + def __init__(self, num_failures: int) -> None: + self._num_failures = num_failures + self._failures_by_path: Dict[bytes, int] = collections.defaultdict(int) async def request( self, @@ -47,26 +40,37 @@ async def request( ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.AsyncByteStream]: scheme, host, port, path = url - if path not in self._ENDPOINTS: + exc, is_retryable = { + b"/": (None, False), + b"/connect_timeout": (httpcore.ConnectTimeout, True), + b"/connect_error": (httpcore.ConnectError, True), + b"/read_timeout": (httpcore.ReadTimeout, False), + b"/network_error": (httpcore.NetworkError, False), + }[path] + + if exc is None: stream = httpcore.PlainByteStream(b"") return (b"HTTP/1.1", 200, b"OK", [], stream) - exc_cls, is_retryable = self._ENDPOINTS[path] - if not is_retryable: - raise exc_cls() + raise exc + + if self._failures_by_path[path] >= self._num_failures: + stream = httpcore.PlainByteStream(b"") + return (b"HTTP/1.1", 200, b"OK", [], stream) - if self._path_attempts[path] < self._succeed_after: - self._path_attempts[path] += 1 - raise exc_cls() + self._failures_by_path[path] += 1 - stream = httpcore.PlainByteStream(b"") - return (b"HTTP/1.1", 200, b"OK", [], stream) + raise exc @pytest.mark.usefixtures("async_environment") async def test_no_retries() -> None: - client = httpx.AsyncClient(transport=AsyncMockTransport(succeed_after=1)) + """ + When no retries are configured, the default behavior is to not retry + and raise immediately any connection failures. + """ + client = httpx.AsyncClient(transport=AsyncMockTransport(num_failures=1)) response = await client.get("https://example.com") assert response.status_code == 200 @@ -77,19 +81,19 @@ async def test_no_retries() -> None: with pytest.raises(httpx.ConnectError): await client.get("https://example.com/connect_error") - with pytest.raises(httpx.ReadTimeout): - await client.get("https://example.com/read_timeout") - - with pytest.raises(httpx.NetworkError): - await client.get("https://example.com/network_error") - @pytest.mark.usefixtures("async_environment") async def test_retries_enabled() -> None: - transport = AsyncMockTransport(succeed_after=3) - client = httpx.AsyncClient( - transport=transport, retries=httpx.Retries(3, backoff_factor=0.01) + """ + When retries are enabled, connection failures are retried on. + """ + transport = AsyncMockTransport(num_failures=3) + retries = httpx.Retries( + 3, + # Small backoff to speed up this test. + backoff_factor=0.01, ) + client = httpx.AsyncClient(transport=transport, retries=retries) response = await client.get("https://example.com") assert response.status_code == 200 @@ -109,17 +113,30 @@ async def test_retries_enabled() -> None: @pytest.mark.usefixtures("async_environment") async def test_retries_exceeded() -> None: - transport = AsyncMockTransport(succeed_after=2) - client = httpx.AsyncClient(transport=transport, retries=1) + """ + When retries are enabled and connecting failures more than the configured number + of retries, connect exceptions are raised. + """ + transport = AsyncMockTransport(num_failures=2) + retries = 1 + client = httpx.AsyncClient(transport=transport, retries=retries) + with pytest.raises(httpx.ConnectTimeout): await client.get("https://example.com/connect_timeout") + with pytest.raises(httpx.ConnectError): + await client.get("https://example.com/connect_error") + @pytest.mark.usefixtures("async_environment") @pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) async def test_retries_idempotent_methods(method: str) -> None: - transport = AsyncMockTransport(succeed_after=1) - client = httpx.AsyncClient(transport=transport, retries=1) + """ + Client makes retries for all idempotent HTTP methods. + """ + transport = AsyncMockTransport(num_failures=1) + retries = 1 + client = httpx.AsyncClient(transport=transport, retries=retries) response = await client.request(method, "https://example.com/connect_timeout") assert response.status_code == 200 @@ -128,9 +145,9 @@ async def test_retries_idempotent_methods(method: str) -> None: @pytest.mark.parametrize("method", ["POST", "PATCH"]) async def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: """ - Non-idempotent HTTP verbs should never be retried on. + Client makes no retries for HTTP methods that are not idempotent. """ - transport = AsyncMockTransport(succeed_after=1) + transport = AsyncMockTransport(num_failures=1) client = httpx.AsyncClient(transport=transport, retries=2) with pytest.raises(httpx.ConnectTimeout): @@ -146,14 +163,20 @@ async def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: ], ) def test_exponential_backoff(factor: float, expected: List[int]) -> None: + """ + Exponential backoff helper works as expected. + """ delays = list(itertools.islice(exponential_backoff(factor), 5)) assert delays == expected @pytest.mark.usefixtures("async_environment") async def test_retries_backoff() -> None: + """ + Exponential backoff is applied when retrying. + """ retries = httpx.Retries(3, backoff_factor=0.05) - transport = AsyncMockTransport(succeed_after=3) + transport = AsyncMockTransport(num_failures=3) client = httpx.AsyncClient(transport=transport, retries=retries) response = await client.get("https://example.com/connect_timeout") assert response.status_code == 200 From 612c64f2910bc40d89e9ddf2beee2eb6c3c57f18 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Wed, 19 Aug 2020 09:59:36 +0200 Subject: [PATCH 03/13] Lint --- httpx/__init__.py | 2 +- tests/client/test_retries.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/httpx/__init__.py b/httpx/__init__.py index 92eee528eb..39dd19c205 100644 --- a/httpx/__init__.py +++ b/httpx/__init__.py @@ -2,7 +2,7 @@ from ._api import delete, get, head, options, patch, post, put, request, stream from ._auth import Auth, BasicAuth, DigestAuth from ._client import AsyncClient, Client -from ._config import Limits, Retries, PoolLimits, Proxy, Timeout, create_ssl_context +from ._config import Limits, PoolLimits, Proxy, Retries, Timeout, create_ssl_context from ._exceptions import ( CloseError, ConnectError, diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index b405837628..9df69a0783 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -1,10 +1,10 @@ import collections import itertools -from typing import List, Tuple, Dict, Mapping, Optional +from typing import Dict, List, Mapping, Optional, Tuple +import httpcore import pytest -import httpcore import httpx from httpx._utils import exponential_backoff From 61b148a02398069b2c5481c2d985f5b3266caca0 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sat, 29 Aug 2020 21:07:43 +0200 Subject: [PATCH 04/13] Rework towards Client(connect_retries=) --- httpx/__init__.py | 3 +- httpx/_client.py | 45 ++++++++++--------------- httpx/_config.py | 32 ++---------------- httpx/_types.py | 4 +-- tests/client/test_retries.py | 64 ++++++++++++------------------------ 5 files changed, 44 insertions(+), 104 deletions(-) diff --git a/httpx/__init__.py b/httpx/__init__.py index 39dd19c205..3802439466 100644 --- a/httpx/__init__.py +++ b/httpx/__init__.py @@ -2,7 +2,7 @@ from ._api import delete, get, head, options, patch, post, put, request, stream from ._auth import Auth, BasicAuth, DigestAuth from ._client import AsyncClient, Client -from ._config import Limits, PoolLimits, Proxy, Retries, Timeout, create_ssl_context +from ._config import Limits, PoolLimits, Proxy, Timeout, create_ssl_context from ._exceptions import ( CloseError, ConnectError, @@ -91,7 +91,6 @@ "Response", "ResponseClosed", "ResponseNotRead", - "Retries", "StatusCode", "stream", "StreamConsumed", diff --git a/httpx/_client.py b/httpx/_client.py index dd697ec93e..eee48e88a7 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -7,14 +7,14 @@ from ._auth import Auth, BasicAuth, FunctionAuth from ._config import ( + CONNECT_RETRIES_BACKOFF_FACTOR, + DEFAULT_CONNECT_RETRIES, DEFAULT_LIMITS, DEFAULT_MAX_REDIRECTS, - DEFAULT_RETRIES, DEFAULT_TIMEOUT_CONFIG, UNSET, Limits, Proxy, - Retries, Timeout, UnsetType, create_ssl_context, @@ -43,7 +43,6 @@ QueryParamTypes, RequestData, RequestFiles, - RetriesTypes, TimeoutTypes, URLTypes, VerifyTypes, @@ -74,7 +73,7 @@ def __init__( cookies: CookieTypes = None, timeout: TimeoutTypes = DEFAULT_TIMEOUT_CONFIG, max_redirects: int = DEFAULT_MAX_REDIRECTS, - retries: RetriesTypes = DEFAULT_RETRIES, + connect_retries: int = DEFAULT_CONNECT_RETRIES, base_url: URLTypes = "", trust_env: bool = True, ): @@ -86,7 +85,7 @@ def __init__( self._cookies = Cookies(cookies) self._timeout = Timeout(timeout) self.max_redirects = max_redirects - self._retries = Retries(retries) + self.connect_retries = connect_retries self._trust_env = trust_env self._netrc = NetRCInfo() @@ -142,14 +141,6 @@ def auth(self) -> typing.Optional[Auth]: def auth(self, auth: AuthTypes) -> None: self._auth = self._build_auth(auth) - @property - def retries(self) -> Retries: - return self._retries - - @retries.setter - def retries(self, retries: RetriesTypes) -> None: - self._retries = Retries(retries) - @property def base_url(self) -> URL: """ @@ -502,7 +493,7 @@ class Client(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. - * **retries** - *(optional)* The maximum number of retries when trying to + * **connect_retries** - *(optional)* The maximum number of retries when trying to establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. @@ -529,7 +520,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, - retries: RetriesTypes = DEFAULT_RETRIES, + connect_retries: int = DEFAULT_CONNECT_RETRIES, base_url: URLTypes = "", transport: httpcore.SyncHTTPTransport = None, app: typing.Callable = None, @@ -542,7 +533,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, - retries=retries, + connect_retries=connect_retries, base_url=base_url, trust_env=trust_env, ) @@ -805,8 +796,8 @@ def _send_handling_auth( history.append(response) def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response: - attempts = self.retries.max_attempts - delays = exponential_backoff(factor=self.retries.backoff_factor) + connect_retries_left = self.connect_retries + delays = exponential_backoff(factor=CONNECT_RETRIES_BACKOFF_FACTOR) while True: try: @@ -814,9 +805,9 @@ def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response except (ConnectError, ConnectTimeout): if request.method in ("POST", "PATCH"): raise - if attempts <= 0: + if connect_retries_left <= 0: raise - attempts -= 1 + connect_retries_left -= 1 delay = next(delays) time.sleep(delay) @@ -1129,7 +1120,7 @@ class AsyncClient(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. - * **retries** - *(optional)* The maximum number of retries when trying to + * **connect_retries** - *(optional)* The maximum number of retries when trying to establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. @@ -1156,7 +1147,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, - retries: RetriesTypes = DEFAULT_RETRIES, + connect_retries: int = DEFAULT_CONNECT_RETRIES, base_url: URLTypes = "", transport: httpcore.AsyncHTTPTransport = None, app: typing.Callable = None, @@ -1169,7 +1160,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, - retries=retries, + connect_retries=connect_retries, base_url=base_url, trust_env=trust_env, ) @@ -1436,8 +1427,8 @@ async def _send_handling_auth( async def _send_handling_retries( self, request: Request, timeout: Timeout ) -> Response: - attempts = self.retries.max_attempts - delays = exponential_backoff(factor=self.retries.backoff_factor) + connect_retries_left = self.connect_retries + delays = exponential_backoff(factor=CONNECT_RETRIES_BACKOFF_FACTOR) while True: try: @@ -1445,9 +1436,9 @@ async def _send_handling_retries( except (ConnectError, ConnectTimeout): if request.method in ("POST", "PATCH"): raise - if attempts <= 0: + if connect_retries_left <= 0: raise - attempts -= 1 + connect_retries_left -= 1 delay = next(delays) await sleep(delay) diff --git a/httpx/_config.py b/httpx/_config.py index 3d788aef47..836d633bdf 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -8,14 +8,7 @@ import certifi from ._models import URL, Headers -from ._types import ( - CertTypes, - HeaderTypes, - RetriesTypes, - TimeoutTypes, - URLTypes, - VerifyTypes, -) +from ._types import CertTypes, HeaderTypes, TimeoutTypes, URLTypes, VerifyTypes from ._utils import get_ca_bundle_from_env, get_logger, warn_deprecated DEFAULT_CIPHERS = ":".join( @@ -417,27 +410,8 @@ def __repr__(self) -> str: ) -class Retries: - def __init__(self, retries: RetriesTypes, *, backoff_factor: float = 0.2) -> None: - if isinstance(retries, Retries): - self.max_attempts = retries.max_attempts # type: int - self.backoff_factor = retries.backoff_factor # type: float - else: - self.max_attempts = retries - self.backoff_factor = backoff_factor - - def __eq__(self, other: typing.Any) -> bool: - return ( - isinstance(other, self.__class__) - and self.max_attempts == other.max_attempts - and self.backoff_factor == other.backoff_factor - ) - - def __repr__(self) -> str: - return f"Retries({self.max_attempts}, backoff_factor={self.backoff_factor})" - - DEFAULT_TIMEOUT_CONFIG = Timeout(timeout=5.0) DEFAULT_LIMITS = Limits(max_connections=100, max_keepalive_connections=20) DEFAULT_MAX_REDIRECTS = 20 -DEFAULT_RETRIES = Retries(0) +DEFAULT_CONNECT_RETRIES = 0 +CONNECT_RETRIES_BACKOFF_FACTOR = 0.5 # 0s, 0.5s, 1s, 2s, 4s, etc. diff --git a/httpx/_types.py b/httpx/_types.py index cc3cb84179..3a90ee42e7 100644 --- a/httpx/_types.py +++ b/httpx/_types.py @@ -21,7 +21,7 @@ if TYPE_CHECKING: # pragma: no cover from ._auth import Auth # noqa: F401 - from ._config import Proxy, Retries, Timeout # noqa: F401 + from ._config import Proxy, Timeout # noqa: F401 from ._models import URL, Cookies, Headers, QueryParams, Request # noqa: F401 @@ -63,8 +63,6 @@ None, ] -RetriesTypes = Union[int, "Retries"] - RequestData = Union[dict, str, bytes, Iterator[bytes], AsyncIterator[bytes]] FileContent = Union[IO[str], IO[bytes], str, bytes] diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index 9df69a0783..66a41b6b45 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -11,18 +11,13 @@ def test_retries_config() -> None: client = httpx.AsyncClient() - assert client.retries == httpx.Retries(0) - assert client.retries.max_attempts == 0 + assert client.connect_retries == 0 - client = httpx.AsyncClient(retries=3) - assert client.retries == httpx.Retries(3) - assert client.retries.max_attempts == 3 - assert client.retries.backoff_factor == 0.2 + client = httpx.AsyncClient(connect_retries=3) + assert client.connect_retries == 3 - client = httpx.AsyncClient(retries=httpx.Retries(3, backoff_factor=0.1)) - assert client.retries == httpx.Retries(3, backoff_factor=0.1) - assert client.retries.max_attempts == 3 - assert client.retries.backoff_factor == 0.1 + client.connect_retries = 1 + assert client.connect_retries == 1 class AsyncMockTransport(httpcore.AsyncHTTPTransport): @@ -65,10 +60,9 @@ async def request( @pytest.mark.usefixtures("async_environment") -async def test_no_retries() -> None: +async def test_no_connect_retries() -> None: """ - When no retries are configured, the default behavior is to not retry - and raise immediately any connection failures. + By default, connection failures are not retried on. """ client = httpx.AsyncClient(transport=AsyncMockTransport(num_failures=1)) @@ -83,26 +77,25 @@ async def test_no_retries() -> None: @pytest.mark.usefixtures("async_environment") -async def test_retries_enabled() -> None: +async def test_connect_retries_enabled() -> None: """ - When retries are enabled, connection failures are retried on. + When connect retries are enabled, connection failures are retried on with + a fixed exponential backoff. """ transport = AsyncMockTransport(num_failures=3) - retries = httpx.Retries( - 3, - # Small backoff to speed up this test. - backoff_factor=0.01, - ) - client = httpx.AsyncClient(transport=transport, retries=retries) + client = httpx.AsyncClient(transport=transport, connect_retries=3) + expected_elapsed_time = pytest.approx(0 + 0.5 + 1, rel=0.1) response = await client.get("https://example.com") assert response.status_code == 200 response = await client.get("https://example.com/connect_timeout") assert response.status_code == 200 + assert response.elapsed.total_seconds() == expected_elapsed_time response = await client.get("https://example.com/connect_error") assert response.status_code == 200 + assert response.elapsed.total_seconds() == expected_elapsed_time with pytest.raises(httpx.ReadTimeout): await client.get("https://example.com/read_timeout") @@ -112,14 +105,13 @@ async def test_retries_enabled() -> None: @pytest.mark.usefixtures("async_environment") -async def test_retries_exceeded() -> None: +async def test_connect_retries_exceeded() -> None: """ When retries are enabled and connecting failures more than the configured number of retries, connect exceptions are raised. """ transport = AsyncMockTransport(num_failures=2) - retries = 1 - client = httpx.AsyncClient(transport=transport, retries=retries) + client = httpx.AsyncClient(transport=transport, connect_retries=1) with pytest.raises(httpx.ConnectTimeout): await client.get("https://example.com/connect_timeout") @@ -130,25 +122,24 @@ async def test_retries_exceeded() -> None: @pytest.mark.usefixtures("async_environment") @pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) -async def test_retries_idempotent_methods(method: str) -> None: +async def test_connect_retries_idempotent_methods(method: str) -> None: """ - Client makes retries for all idempotent HTTP methods. + Client can retry on idempotent HTTP methods. """ transport = AsyncMockTransport(num_failures=1) - retries = 1 - client = httpx.AsyncClient(transport=transport, retries=retries) + client = httpx.AsyncClient(transport=transport, connect_retries=1) response = await client.request(method, "https://example.com/connect_timeout") assert response.status_code == 200 @pytest.mark.usefixtures("async_environment") @pytest.mark.parametrize("method", ["POST", "PATCH"]) -async def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: +async def test_connect_retries_disabled_on_non_idempotent_methods(method: str) -> None: """ Client makes no retries for HTTP methods that are not idempotent. """ transport = AsyncMockTransport(num_failures=1) - client = httpx.AsyncClient(transport=transport, retries=2) + client = httpx.AsyncClient(transport=transport, connect_retries=2) with pytest.raises(httpx.ConnectTimeout): await client.request(method, "https://example.com/connect_timeout") @@ -168,16 +159,3 @@ def test_exponential_backoff(factor: float, expected: List[int]) -> None: """ delays = list(itertools.islice(exponential_backoff(factor), 5)) assert delays == expected - - -@pytest.mark.usefixtures("async_environment") -async def test_retries_backoff() -> None: - """ - Exponential backoff is applied when retrying. - """ - retries = httpx.Retries(3, backoff_factor=0.05) - transport = AsyncMockTransport(num_failures=3) - client = httpx.AsyncClient(transport=transport, retries=retries) - response = await client.get("https://example.com/connect_timeout") - assert response.status_code == 200 - assert response.elapsed.total_seconds() == pytest.approx(0 + 0.05 + 0.1, rel=0.1) From 6dc659bb033ac8cb28a0501e1f518c29b45cebb9 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sat, 29 Aug 2020 21:12:30 +0200 Subject: [PATCH 05/13] Prefer "method not in IDEMPOTENT_METHODS" --- httpx/_client.py | 5 +++-- httpx/_config.py | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index eee48e88a7..41ee07faa7 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -12,6 +12,7 @@ DEFAULT_LIMITS, DEFAULT_MAX_REDIRECTS, DEFAULT_TIMEOUT_CONFIG, + IDEMPOTENT_METHODS, UNSET, Limits, Proxy, @@ -803,7 +804,7 @@ def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response try: return self._send_single_request(request, timeout) except (ConnectError, ConnectTimeout): - if request.method in ("POST", "PATCH"): + if request.method not in IDEMPOTENT_METHODS: raise if connect_retries_left <= 0: raise @@ -1434,7 +1435,7 @@ async def _send_handling_retries( try: return await self._send_single_request(request, timeout) except (ConnectError, ConnectTimeout): - if request.method in ("POST", "PATCH"): + if request.method not in IDEMPOTENT_METHODS: raise if connect_retries_left <= 0: raise diff --git a/httpx/_config.py b/httpx/_config.py index 836d633bdf..34a7e3f664 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -30,6 +30,7 @@ ] ) +IDEMPOTENT_METHODS = {"GET", "HEAD", "PUT", "DELETE", "OPTIONS", "TRACE"} logger = get_logger(__name__) From 8ccdac39e19c14c1ba72523eb48629cdd14c61e6 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sat, 29 Aug 2020 21:33:34 +0200 Subject: [PATCH 06/13] Prefer httpx.Client(), ensure test coverage --- tests/client/test_retries.py | 128 +++++++++++++++++++++++------------ 1 file changed, 86 insertions(+), 42 deletions(-) diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index 66a41b6b45..8767f48e13 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -10,29 +10,25 @@ def test_retries_config() -> None: - client = httpx.AsyncClient() + client = httpx.Client() assert client.connect_retries == 0 - client = httpx.AsyncClient(connect_retries=3) + client = httpx.Client(connect_retries=3) assert client.connect_retries == 3 client.connect_retries = 1 assert client.connect_retries == 1 -class AsyncMockTransport(httpcore.AsyncHTTPTransport): +class BaseMockTransport: def __init__(self, num_failures: int) -> None: self._num_failures = num_failures - self._failures_by_path: Dict[bytes, int] = collections.defaultdict(int) + self._attempts_by_path: Dict[bytes, int] = collections.defaultdict(int) - async def request( + def _request( self, - method: bytes, url: Tuple[bytes, bytes, Optional[int], bytes], - headers: List[Tuple[bytes, bytes]] = None, - stream: httpcore.AsyncByteStream = None, - timeout: Mapping[str, Optional[float]] = None, - ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.AsyncByteStream]: + ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.PlainByteStream]: scheme, host, port, path = url exc, is_retryable = { @@ -50,99 +46,147 @@ async def request( if not is_retryable: raise exc - if self._failures_by_path[path] >= self._num_failures: + if self._attempts_by_path[path] >= self._num_failures: + self._attempts_by_path.clear() stream = httpcore.PlainByteStream(b"") return (b"HTTP/1.1", 200, b"OK", [], stream) - self._failures_by_path[path] += 1 + self._attempts_by_path[path] += 1 raise exc -@pytest.mark.usefixtures("async_environment") -async def test_no_connect_retries() -> None: +class MockTransport(BaseMockTransport, httpcore.SyncHTTPTransport): + def __init__(self, num_failures: int) -> None: + super().__init__(num_failures) + + def request( + self, + method: bytes, + url: Tuple[bytes, bytes, Optional[int], bytes], + headers: List[Tuple[bytes, bytes]] = None, + stream: httpcore.SyncByteStream = None, + timeout: Mapping[str, Optional[float]] = None, + ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.SyncByteStream]: + return self._request(url) + + +class AsyncMockTransport(BaseMockTransport, httpcore.AsyncHTTPTransport): + def __init__(self, num_failures: int) -> None: + super().__init__(num_failures) + + async def request( + self, + method: bytes, + url: Tuple[bytes, bytes, Optional[int], bytes], + headers: List[Tuple[bytes, bytes]] = None, + stream: httpcore.AsyncByteStream = None, + timeout: Mapping[str, Optional[float]] = None, + ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.AsyncByteStream]: + return self._request(url) + + +def test_no_connect_retries() -> None: """ By default, connection failures are not retried on. """ - client = httpx.AsyncClient(transport=AsyncMockTransport(num_failures=1)) + transport = MockTransport(num_failures=1) + client = httpx.Client(transport=transport) - response = await client.get("https://example.com") + response = client.get("https://example.com") assert response.status_code == 200 with pytest.raises(httpx.ConnectTimeout): - await client.get("https://example.com/connect_timeout") + client.get("https://example.com/connect_timeout") with pytest.raises(httpx.ConnectError): - await client.get("https://example.com/connect_error") + client.get("https://example.com/connect_error") -@pytest.mark.usefixtures("async_environment") -async def test_connect_retries_enabled() -> None: +def test_connect_retries_enabled() -> None: """ When connect retries are enabled, connection failures are retried on with a fixed exponential backoff. """ - transport = AsyncMockTransport(num_failures=3) - client = httpx.AsyncClient(transport=transport, connect_retries=3) + transport = MockTransport(num_failures=3) + client = httpx.Client(transport=transport, connect_retries=3) expected_elapsed_time = pytest.approx(0 + 0.5 + 1, rel=0.1) - response = await client.get("https://example.com") + response = client.get("https://example.com") assert response.status_code == 200 - response = await client.get("https://example.com/connect_timeout") + response = client.get("https://example.com/connect_timeout") assert response.status_code == 200 assert response.elapsed.total_seconds() == expected_elapsed_time - response = await client.get("https://example.com/connect_error") + response = client.get("https://example.com/connect_error") assert response.status_code == 200 assert response.elapsed.total_seconds() == expected_elapsed_time with pytest.raises(httpx.ReadTimeout): - await client.get("https://example.com/read_timeout") + client.get("https://example.com/read_timeout") with pytest.raises(httpx.NetworkError): - await client.get("https://example.com/network_error") + client.get("https://example.com/network_error") @pytest.mark.usefixtures("async_environment") -async def test_connect_retries_exceeded() -> None: +async def test_connect_retries_enabled_async() -> None: + # For test coverage purposes. + transport = AsyncMockTransport(num_failures=3) + client = httpx.AsyncClient(transport=transport, connect_retries=3) + expected_elapsed_time = pytest.approx(0 + 0.5 + 1, rel=0.1) + + # Connect exceptions are retried on with a backoff. + response = await client.get("https://example.com/connect_timeout") + assert response.status_code == 200 + assert response.elapsed.total_seconds() == expected_elapsed_time + + # Non-connect errors are not retried on. + with pytest.raises(httpx.ReadTimeout): + await client.get("https://example.com/read_timeout") + + # Non-idempotent methods are not retried on. + with pytest.raises(httpx.ConnectTimeout): + await client.post("https://example.com/connect_timeout") + + +def test_connect_retries_exceeded() -> None: """ When retries are enabled and connecting failures more than the configured number of retries, connect exceptions are raised. """ - transport = AsyncMockTransport(num_failures=2) - client = httpx.AsyncClient(transport=transport, connect_retries=1) + transport = MockTransport(num_failures=2) + client = httpx.Client(transport=transport, connect_retries=1) with pytest.raises(httpx.ConnectTimeout): - await client.get("https://example.com/connect_timeout") + client.get("https://example.com/connect_timeout") with pytest.raises(httpx.ConnectError): - await client.get("https://example.com/connect_error") + client.get("https://example.com/connect_error") -@pytest.mark.usefixtures("async_environment") @pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) -async def test_connect_retries_idempotent_methods(method: str) -> None: +def test_connect_retries_idempotent_methods(method: str) -> None: """ Client can retry on idempotent HTTP methods. """ - transport = AsyncMockTransport(num_failures=1) - client = httpx.AsyncClient(transport=transport, connect_retries=1) - response = await client.request(method, "https://example.com/connect_timeout") + transport = MockTransport(num_failures=1) + client = httpx.Client(transport=transport, connect_retries=1) + response = client.request(method, "https://example.com/connect_timeout") assert response.status_code == 200 -@pytest.mark.usefixtures("async_environment") @pytest.mark.parametrize("method", ["POST", "PATCH"]) -async def test_connect_retries_disabled_on_non_idempotent_methods(method: str) -> None: +def test_connect_retries_disabled_on_non_idempotent_methods(method: str) -> None: """ Client makes no retries for HTTP methods that are not idempotent. """ - transport = AsyncMockTransport(num_failures=1) - client = httpx.AsyncClient(transport=transport, connect_retries=2) + transport = MockTransport(num_failures=1) + client = httpx.Client(transport=transport, connect_retries=2) with pytest.raises(httpx.ConnectTimeout): - await client.request(method, "https://example.com/connect_timeout") + client.request(method, "https://example.com/connect_timeout") @pytest.mark.parametrize( From 8c56ad273be325e50636e764f16dbeec7f0c6c32 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Sat, 29 Aug 2020 21:35:20 +0200 Subject: [PATCH 07/13] Tweak url destructuring --- tests/client/test_retries.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index 8767f48e13..8fb224b96c 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -29,7 +29,7 @@ def _request( self, url: Tuple[bytes, bytes, Optional[int], bytes], ) -> Tuple[bytes, int, bytes, List[Tuple[bytes, bytes]], httpcore.PlainByteStream]: - scheme, host, port, path = url + _, _, _, path = url exc, is_retryable = { b"/": (None, False), From e7fe54756da111cddeca1dcb3371ae3a72f7e65e Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Tue, 1 Sep 2020 22:54:14 +0200 Subject: [PATCH 08/13] Rename connect_retries as retries --- httpx/_client.py | 36 ++++++++++++++++++------------------ httpx/_config.py | 4 ++-- tests/client/test_retries.py | 34 +++++++++++++++++----------------- 3 files changed, 37 insertions(+), 37 deletions(-) diff --git a/httpx/_client.py b/httpx/_client.py index 41ee07faa7..244ef735d5 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -7,12 +7,12 @@ from ._auth import Auth, BasicAuth, FunctionAuth from ._config import ( - CONNECT_RETRIES_BACKOFF_FACTOR, - DEFAULT_CONNECT_RETRIES, DEFAULT_LIMITS, DEFAULT_MAX_REDIRECTS, + DEFAULT_RETRIES, DEFAULT_TIMEOUT_CONFIG, IDEMPOTENT_METHODS, + RETRIES_BACKOFF_FACTOR, UNSET, Limits, Proxy, @@ -74,7 +74,7 @@ def __init__( cookies: CookieTypes = None, timeout: TimeoutTypes = DEFAULT_TIMEOUT_CONFIG, max_redirects: int = DEFAULT_MAX_REDIRECTS, - connect_retries: int = DEFAULT_CONNECT_RETRIES, + retries: int = DEFAULT_RETRIES, base_url: URLTypes = "", trust_env: bool = True, ): @@ -86,7 +86,7 @@ def __init__( self._cookies = Cookies(cookies) self._timeout = Timeout(timeout) self.max_redirects = max_redirects - self.connect_retries = connect_retries + self.retries = retries self._trust_env = trust_env self._netrc = NetRCInfo() @@ -494,7 +494,7 @@ class Client(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. - * **connect_retries** - *(optional)* The maximum number of retries when trying to + * **retries** - *(optional)* The maximum number of retries when trying to establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. @@ -521,7 +521,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, - connect_retries: int = DEFAULT_CONNECT_RETRIES, + retries: int = DEFAULT_RETRIES, base_url: URLTypes = "", transport: httpcore.SyncHTTPTransport = None, app: typing.Callable = None, @@ -534,7 +534,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, - connect_retries=connect_retries, + retries=retries, base_url=base_url, trust_env=trust_env, ) @@ -797,8 +797,8 @@ def _send_handling_auth( history.append(response) def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response: - connect_retries_left = self.connect_retries - delays = exponential_backoff(factor=CONNECT_RETRIES_BACKOFF_FACTOR) + retries_left = self.retries + delays = exponential_backoff(factor=RETRIES_BACKOFF_FACTOR) while True: try: @@ -806,9 +806,9 @@ def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response except (ConnectError, ConnectTimeout): if request.method not in IDEMPOTENT_METHODS: raise - if connect_retries_left <= 0: + if retries_left <= 0: raise - connect_retries_left -= 1 + retries_left -= 1 delay = next(delays) time.sleep(delay) @@ -1121,7 +1121,7 @@ class AsyncClient(BaseClient): * **limits** - *(optional)* The limits configuration to use. * **max_redirects** - *(optional)* The maximum number of redirect responses that should be followed. - * **connect_retries** - *(optional)* The maximum number of retries when trying to + * **retries** - *(optional)* The maximum number of retries when trying to establish a connection. * **base_url** - *(optional)* A URL to use as the base when building request URLs. @@ -1148,7 +1148,7 @@ def __init__( limits: Limits = DEFAULT_LIMITS, pool_limits: Limits = None, max_redirects: int = DEFAULT_MAX_REDIRECTS, - connect_retries: int = DEFAULT_CONNECT_RETRIES, + retries: int = DEFAULT_RETRIES, base_url: URLTypes = "", transport: httpcore.AsyncHTTPTransport = None, app: typing.Callable = None, @@ -1161,7 +1161,7 @@ def __init__( cookies=cookies, timeout=timeout, max_redirects=max_redirects, - connect_retries=connect_retries, + retries=retries, base_url=base_url, trust_env=trust_env, ) @@ -1428,8 +1428,8 @@ async def _send_handling_auth( async def _send_handling_retries( self, request: Request, timeout: Timeout ) -> Response: - connect_retries_left = self.connect_retries - delays = exponential_backoff(factor=CONNECT_RETRIES_BACKOFF_FACTOR) + retries_left = self.retries + delays = exponential_backoff(factor=RETRIES_BACKOFF_FACTOR) while True: try: @@ -1437,9 +1437,9 @@ async def _send_handling_retries( except (ConnectError, ConnectTimeout): if request.method not in IDEMPOTENT_METHODS: raise - if connect_retries_left <= 0: + if retries_left <= 0: raise - connect_retries_left -= 1 + retries_left -= 1 delay = next(delays) await sleep(delay) diff --git a/httpx/_config.py b/httpx/_config.py index 34a7e3f664..e2047f50ef 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -414,5 +414,5 @@ def __repr__(self) -> str: DEFAULT_TIMEOUT_CONFIG = Timeout(timeout=5.0) DEFAULT_LIMITS = Limits(max_connections=100, max_keepalive_connections=20) DEFAULT_MAX_REDIRECTS = 20 -DEFAULT_CONNECT_RETRIES = 0 -CONNECT_RETRIES_BACKOFF_FACTOR = 0.5 # 0s, 0.5s, 1s, 2s, 4s, etc. +DEFAULT_RETRIES = 0 +RETRIES_BACKOFF_FACTOR = 0.5 # 0s, 0.5s, 1s, 2s, 4s, etc. diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index 8fb224b96c..193bbeb4fd 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -11,13 +11,13 @@ def test_retries_config() -> None: client = httpx.Client() - assert client.connect_retries == 0 + assert client.retries == 0 - client = httpx.Client(connect_retries=3) - assert client.connect_retries == 3 + client = httpx.Client(retries=3) + assert client.retries == 3 - client.connect_retries = 1 - assert client.connect_retries == 1 + client.retries = 1 + assert client.retries == 1 class BaseMockTransport: @@ -86,7 +86,7 @@ async def request( return self._request(url) -def test_no_connect_retries() -> None: +def test_no_retries() -> None: """ By default, connection failures are not retried on. """ @@ -103,13 +103,13 @@ def test_no_connect_retries() -> None: client.get("https://example.com/connect_error") -def test_connect_retries_enabled() -> None: +def test_retries_enabled() -> None: """ - When connect retries are enabled, connection failures are retried on with + When retries are enabled, connection failures are retried on with a fixed exponential backoff. """ transport = MockTransport(num_failures=3) - client = httpx.Client(transport=transport, connect_retries=3) + client = httpx.Client(transport=transport, retries=3) expected_elapsed_time = pytest.approx(0 + 0.5 + 1, rel=0.1) response = client.get("https://example.com") @@ -131,10 +131,10 @@ def test_connect_retries_enabled() -> None: @pytest.mark.usefixtures("async_environment") -async def test_connect_retries_enabled_async() -> None: +async def test_retries_enabled_async() -> None: # For test coverage purposes. transport = AsyncMockTransport(num_failures=3) - client = httpx.AsyncClient(transport=transport, connect_retries=3) + client = httpx.AsyncClient(transport=transport, retries=3) expected_elapsed_time = pytest.approx(0 + 0.5 + 1, rel=0.1) # Connect exceptions are retried on with a backoff. @@ -151,13 +151,13 @@ async def test_connect_retries_enabled_async() -> None: await client.post("https://example.com/connect_timeout") -def test_connect_retries_exceeded() -> None: +def test_retries_exceeded() -> None: """ When retries are enabled and connecting failures more than the configured number of retries, connect exceptions are raised. """ transport = MockTransport(num_failures=2) - client = httpx.Client(transport=transport, connect_retries=1) + client = httpx.Client(transport=transport, retries=1) with pytest.raises(httpx.ConnectTimeout): client.get("https://example.com/connect_timeout") @@ -167,23 +167,23 @@ def test_connect_retries_exceeded() -> None: @pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) -def test_connect_retries_idempotent_methods(method: str) -> None: +def test_retries_idempotent_methods(method: str) -> None: """ Client can retry on idempotent HTTP methods. """ transport = MockTransport(num_failures=1) - client = httpx.Client(transport=transport, connect_retries=1) + client = httpx.Client(transport=transport, retries=1) response = client.request(method, "https://example.com/connect_timeout") assert response.status_code == 200 @pytest.mark.parametrize("method", ["POST", "PATCH"]) -def test_connect_retries_disabled_on_non_idempotent_methods(method: str) -> None: +def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: """ Client makes no retries for HTTP methods that are not idempotent. """ transport = MockTransport(num_failures=1) - client = httpx.Client(transport=transport, connect_retries=2) + client = httpx.Client(transport=transport, retries=2) with pytest.raises(httpx.ConnectTimeout): client.request(method, "https://example.com/connect_timeout") From 24e974dabac341e6c89bbc19ab5fb23aba6e5577 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Tue, 1 Sep 2020 23:12:27 +0200 Subject: [PATCH 09/13] Add docs --- docs/advanced.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/advanced.md b/docs/advanced.md index e43ecca6df..877cd9a503 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -626,6 +626,24 @@ For instance this request sends 2 files, `foo.png` and `bar.png` in one request >>> r = httpx.post("https://httpbin.org/post", files=files) ``` +## Retries + +Client instances can retry on errors that occur while establishing new connections. + +Retries are disabled by default. They can be enabled by passing the maximum number of retries as `Client(retries=)`. For example... + +```pycon +>>> with httpx.Client(retries=3) as client: +... # If a connect error occurs, we will retry up to 3 times before failing. +... response = client.get("https://unstableserver.com/") +``` + +Note that: + +* HTTPX issues a first retry without waiting (transient errors are often resolved immediately), then issues retries at exponentially increasing time intervals (0.5s, 1s, 2s, 4s, etc). +* Retries are always disabled for non-idempotent methods, such as `POST` or `PATCH`. +* The built-in retry functionality only applies to failures _while establishing new connections_ (effectively `ConnectError` and `ConnectTimeout` exceptions). In particular, errors while interacting with existing connections (such as unexpected server-side connection closures, or read/write timeouts) will not be retried on. + ## Customizing authentication When issuing requests or instantiating a client, the `auth` argument can be used to pass an authentication scheme to use. The `auth` argument may be one of the following... From 7c93bd7624ef086ca3ad3a7e7877655727887ee4 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Fri, 4 Sep 2020 23:00:19 +0200 Subject: [PATCH 10/13] No need to care about idempotent methods on connect --- docs/advanced.md | 1 - httpx/_client.py | 5 ----- httpx/_config.py | 2 -- tests/client/test_retries.py | 24 +++++------------------- 4 files changed, 5 insertions(+), 27 deletions(-) diff --git a/docs/advanced.md b/docs/advanced.md index 2ef9464b23..88dc377c51 100644 --- a/docs/advanced.md +++ b/docs/advanced.md @@ -641,7 +641,6 @@ Retries are disabled by default. They can be enabled by passing the maximum numb Note that: * HTTPX issues a first retry without waiting (transient errors are often resolved immediately), then issues retries at exponentially increasing time intervals (0.5s, 1s, 2s, 4s, etc). -* Retries are always disabled for non-idempotent methods, such as `POST` or `PATCH`. * The built-in retry functionality only applies to failures _while establishing new connections_ (effectively `ConnectError` and `ConnectTimeout` exceptions). In particular, errors while interacting with existing connections (such as unexpected server-side connection closures, or read/write timeouts) will not be retried on. ## Customizing authentication diff --git a/httpx/_client.py b/httpx/_client.py index 439b54700d..698a577295 100644 --- a/httpx/_client.py +++ b/httpx/_client.py @@ -13,7 +13,6 @@ DEFAULT_MAX_REDIRECTS, DEFAULT_RETRIES, DEFAULT_TIMEOUT_CONFIG, - IDEMPOTENT_METHODS, RETRIES_BACKOFF_FACTOR, UNSET, Limits, @@ -828,8 +827,6 @@ def _send_handling_retries(self, request: Request, timeout: Timeout) -> Response try: return self._send_single_request(request, timeout) except (ConnectError, ConnectTimeout): - if request.method not in IDEMPOTENT_METHODS: - raise if retries_left <= 0: raise retries_left -= 1 @@ -1471,8 +1468,6 @@ async def _send_handling_retries( try: return await self._send_single_request(request, timeout) except (ConnectError, ConnectTimeout): - if request.method not in IDEMPOTENT_METHODS: - raise if retries_left <= 0: raise retries_left -= 1 diff --git a/httpx/_config.py b/httpx/_config.py index e2047f50ef..958d20b5d6 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -30,8 +30,6 @@ ] ) -IDEMPOTENT_METHODS = {"GET", "HEAD", "PUT", "DELETE", "OPTIONS", "TRACE"} - logger = get_logger(__name__) diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index 193bbeb4fd..ce8f21ba02 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -146,10 +146,6 @@ async def test_retries_enabled_async() -> None: with pytest.raises(httpx.ReadTimeout): await client.get("https://example.com/read_timeout") - # Non-idempotent methods are not retried on. - with pytest.raises(httpx.ConnectTimeout): - await client.post("https://example.com/connect_timeout") - def test_retries_exceeded() -> None: """ @@ -166,10 +162,12 @@ def test_retries_exceeded() -> None: client.get("https://example.com/connect_error") -@pytest.mark.parametrize("method", ["HEAD", "GET", "PUT", "DELETE", "OPTIONS", "TRACE"]) -def test_retries_idempotent_methods(method: str) -> None: +@pytest.mark.parametrize( + "method", ["HEAD", "GET", "POST", "PUT", "PATCH", "DELETE", "OPTIONS", "TRACE"] +) +def test_retries_methods(method: str) -> None: """ - Client can retry on idempotent HTTP methods. + Client retries on all HTTP methods. """ transport = MockTransport(num_failures=1) client = httpx.Client(transport=transport, retries=1) @@ -177,18 +175,6 @@ def test_retries_idempotent_methods(method: str) -> None: assert response.status_code == 200 -@pytest.mark.parametrize("method", ["POST", "PATCH"]) -def test_retries_disabled_on_non_idempotent_methods(method: str) -> None: - """ - Client makes no retries for HTTP methods that are not idempotent. - """ - transport = MockTransport(num_failures=1) - client = httpx.Client(transport=transport, retries=2) - - with pytest.raises(httpx.ConnectTimeout): - client.request(method, "https://example.com/connect_timeout") - - @pytest.mark.parametrize( "factor, expected", [ From a1413dfb00b04135033ba3a9b222214ac4edd36f Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Fri, 4 Sep 2020 23:02:19 +0200 Subject: [PATCH 11/13] Anticipate curio support in HTTPCore --- httpx/_utils.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/httpx/_utils.py b/httpx/_utils.py index 703bb8279a..0b47e108d9 100644 --- a/httpx/_utils.py +++ b/httpx/_utils.py @@ -541,6 +541,10 @@ async def sleep(seconds: float) -> None: import trio await trio.sleep(seconds) + elif library == "curio": # pragma: no cover + import curio + + await curio.sleep(seconds) else: assert library == "asyncio" await asyncio.sleep(seconds) From a25aae360e098612bd6e0703354f67eeaffa4b21 Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Fri, 4 Sep 2020 23:23:43 +0200 Subject: [PATCH 12/13] Remove unrelated newline change --- httpx/_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/httpx/_config.py b/httpx/_config.py index 958d20b5d6..5bedff3874 100644 --- a/httpx/_config.py +++ b/httpx/_config.py @@ -30,6 +30,7 @@ ] ) + logger = get_logger(__name__) From d6c236bef37e6f0a3293991689817363a453558c Mon Sep 17 00:00:00 2001 From: florimondmanca Date: Fri, 4 Sep 2020 23:59:08 +0200 Subject: [PATCH 13/13] Move exponential_backoff test to test_utils.py --- tests/client/test_retries.py | 18 ------------------ tests/test_utils.py | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/tests/client/test_retries.py b/tests/client/test_retries.py index ce8f21ba02..065b32c121 100644 --- a/tests/client/test_retries.py +++ b/tests/client/test_retries.py @@ -1,12 +1,10 @@ import collections -import itertools from typing import Dict, List, Mapping, Optional, Tuple import httpcore import pytest import httpx -from httpx._utils import exponential_backoff def test_retries_config() -> None: @@ -173,19 +171,3 @@ def test_retries_methods(method: str) -> None: client = httpx.Client(transport=transport, retries=1) response = client.request(method, "https://example.com/connect_timeout") assert response.status_code == 200 - - -@pytest.mark.parametrize( - "factor, expected", - [ - (0.1, [0, 0.1, 0.2, 0.4, 0.8]), - (0.2, [0, 0.2, 0.4, 0.8, 1.6]), - (0.5, [0, 0.5, 1.0, 2.0, 4.0]), - ], -) -def test_exponential_backoff(factor: float, expected: List[int]) -> None: - """ - Exponential backoff helper works as expected. - """ - delays = list(itertools.islice(exponential_backoff(factor), 5)) - assert delays == expected diff --git a/tests/test_utils.py b/tests/test_utils.py index d5dfb5819b..cb58300061 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -1,6 +1,8 @@ import asyncio +import itertools import os import random +from typing import List import pytest @@ -9,6 +11,7 @@ ElapsedTimer, NetRCInfo, URLPattern, + exponential_backoff, get_ca_bundle_from_env, get_environment_proxies, guess_json_utf, @@ -270,3 +273,16 @@ def test_pattern_priority(): URLPattern("http://"), URLPattern("all://"), ] + + +@pytest.mark.parametrize( + "factor, expected", + [ + (0.1, [0, 0.1, 0.2, 0.4, 0.8]), + (0.2, [0, 0.2, 0.4, 0.8, 1.6]), + (0.5, [0, 0.5, 1.0, 2.0, 4.0]), + ], +) +def test_exponential_backoff(factor: float, expected: List[int]) -> None: + delays = list(itertools.islice(exponential_backoff(factor), 5)) + assert delays == expected