From b9d776df4848565f38c5ebe42d49030b3194a1a0 Mon Sep 17 00:00:00 2001
From: Brett Higgins <brett.higgins@gmail.com>
Date: Mon, 17 Jul 2023 10:50:57 -0400
Subject: [PATCH] Fixed failure to try next host after single-host connection
timeout
Also updated the default ``ClientTimeout`` params to include ``sock_connect``
so that this correct behavior happens by default.
---
CHANGES/7342.bugfix | 3 ++
CONTRIBUTORS.txt | 1 +
aiohttp/client.py | 2 +-
aiohttp/connector.py | 2 +-
docs/client_quickstart.rst | 3 +-
docs/client_reference.rst | 4 +-
tests/test_connector.py | 75 ++++++++++++++++++++++++++++++++++++++
7 files changed, 85 insertions(+), 5 deletions(-)
create mode 100644 CHANGES/7342.bugfix
diff --git a/CHANGES/7342.bugfix b/CHANGES/7342.bugfix
new file mode 100644
index 00000000000..fa411657e0c
--- /dev/null
+++ b/CHANGES/7342.bugfix
@@ -0,0 +1,3 @@
+Fixed failure to try next host after single-host connection timeout.
+Also updated the default ``ClientTimeout`` params to include ``sock_connect``
+so that this correct behavior happens by default.
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 0cb642bf466..25bf3ee7792 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -60,6 +60,7 @@ Boris Feld
Borys Vorona
Boyi Chen
Brett Cannon
+Brett Higgins
Brian Bouterse
Brian C. Lane
Brian Muller
diff --git a/aiohttp/client.py b/aiohttp/client.py
index 9050cc4120c..777722e2c27 100644
--- a/aiohttp/client.py
+++ b/aiohttp/client.py
@@ -157,7 +157,7 @@ class ClientTimeout:
# 5 Minute default read timeout
-DEFAULT_TIMEOUT: Final[ClientTimeout] = ClientTimeout(total=5 * 60)
+DEFAULT_TIMEOUT: Final[ClientTimeout] = ClientTimeout(total=5 * 60, sock_connect=30)
_RetType = TypeVar("_RetType")
diff --git a/aiohttp/connector.py b/aiohttp/connector.py
index 3d9f7b59f7b..dcaeb72b6a5 100644
--- a/aiohttp/connector.py
+++ b/aiohttp/connector.py
@@ -1131,7 +1131,7 @@ def drop_exception(fut: "asyncio.Future[List[Dict[str, Any]]]") -> None:
req=req,
client_error=client_error,
)
- except ClientConnectorError as exc:
+ except (ClientConnectorError, asyncio.TimeoutError) as exc:
last_exc = exc
continue
diff --git a/docs/client_quickstart.rst b/docs/client_quickstart.rst
index 92334a5f4b4..cb5a4f2ce3f 100644
--- a/docs/client_quickstart.rst
+++ b/docs/client_quickstart.rst
@@ -407,7 +407,8 @@ Timeouts
Timeout settings are stored in :class:`ClientTimeout` data structure.
By default *aiohttp* uses a *total* 300 seconds (5min) timeout, it means that the
-whole operation should finish in 5 minutes.
+whole operation should finish in 5 minutes. In order to allow time for DNS fallback,
+the default ``sock_connect`` timeout is 30 seconds.
The value could be overridden by *timeout* parameter for the session (specified in seconds)::
diff --git a/docs/client_reference.rst b/docs/client_reference.rst
index 407030dce14..464cc06c2c8 100644
--- a/docs/client_reference.rst
+++ b/docs/client_reference.rst
@@ -135,7 +135,7 @@ The client session supports the context manager protocol for self closing.
higher.
:param timeout: a :class:`ClientTimeout` settings structure, 300 seconds (5min)
- total timeout by default.
+ total timeout, 30 seconds socket connect timeout by default.
.. versionadded:: 3.3
@@ -905,7 +905,7 @@ certification chaining.
.. versionadded:: 3.7
:param timeout: a :class:`ClientTimeout` settings structure, 300 seconds (5min)
- total timeout by default.
+ total timeout, 30 seconds socket connect timeout by default.
:param loop: :ref:`event loop<asyncio-event-loop>`
used for processing HTTP requests.
diff --git a/tests/test_connector.py b/tests/test_connector.py
index 9ef71882f87..8a344a82953 100644
--- a/tests/test_connector.py
+++ b/tests/test_connector.py
@@ -669,6 +669,81 @@ def get_extra_info(param):
established_connection.close()
+async def test_tcp_connector_multiple_hosts_one_timeout(loop: Any) -> None:
+ conn = aiohttp.TCPConnector()
+
+ ip1 = "192.168.1.1"
+ ip2 = "192.168.1.2"
+ ips = [ip1, ip2]
+ ips_tried = []
+
+ req = ClientRequest(
+ "GET",
+ URL("https://mocked.host"),
+ loop=loop,
+ )
+
+ async def _resolve_host(host, port, traces=None):
+ return [
+ {
+ "hostname": host,
+ "host": ip,
+ "port": port,
+ "family": socket.AF_INET,
+ "proto": 0,
+ "flags": socket.AI_NUMERICHOST,
+ }
+ for ip in ips
+ ]
+
+ conn._resolve_host = _resolve_host
+
+ timeout_error = False
+ connected = False
+
+ async def create_connection(*args, **kwargs):
+ nonlocal timeout_error, connected
+
+ ip = args[1]
+
+ ips_tried.append(ip)
+
+ if ip == ip1:
+ timeout_error = True
+ raise asyncio.TimeoutError
+
+ if ip == ip2:
+ connected = True
+ tr = create_mocked_conn(loop)
+ pr = create_mocked_conn(loop)
+
+ def get_extra_info(param):
+ if param == "sslcontext":
+ return True
+
+ if param == "ssl_object":
+ s = create_mocked_conn(loop)
+ s.getpeercert.return_value = b"foo"
+ return s
+
+ assert False
+
+ tr.get_extra_info = get_extra_info
+ return tr, pr
+
+ assert False
+
+ conn._loop.create_connection = create_connection
+
+ established_connection = await conn.connect(req, [], ClientTimeout())
+ assert ips == ips_tried
+
+ assert timeout_error
+ assert connected
+
+ established_connection.close()
+
+
async def test_tcp_connector_resolve_host(loop: Any) -> None:
conn = aiohttp.TCPConnector(use_dns_cache=True)