Skip to content

Commit

Permalink
Fixed failure to try next host after single-host connection timeout
Browse files Browse the repository at this point in the history
Also updated the default ``ClientTimeout`` params to include ``sock_connect``
so that this correct behavior happens by default.
  • Loading branch information
brettdh committed Jul 17, 2023
1 parent 7911f1e commit b9d776d
Show file tree
Hide file tree
Showing 7 changed files with 85 additions and 5 deletions.
3 changes: 3 additions & 0 deletions CHANGES/7342.bugfix
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ Boris Feld
Borys Vorona
Boyi Chen
Brett Cannon
Brett Higgins
Brian Bouterse
Brian C. Lane
Brian Muller
Expand Down
2 changes: 1 addition & 1 deletion aiohttp/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down
2 changes: 1 addition & 1 deletion aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion docs/client_quickstart.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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)::

Expand Down
4 changes: 2 additions & 2 deletions docs/client_reference.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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.
Expand Down
75 changes: 75 additions & 0 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down

0 comments on commit b9d776d

Please sign in to comment.