diff --git a/CHANGES/4795.bugfix b/CHANGES/4795.bugfix new file mode 100644 index 00000000000..489214cc383 --- /dev/null +++ b/CHANGES/4795.bugfix @@ -0,0 +1 @@ +Fix resolver task is not awaited when connector is cancelled diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index e8bc9e11e79..4f612399b39 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -238,6 +238,7 @@ Sebastien Geffroy SeongSoo Cho Sergey Ninua Sergey Skripnick +Serhii Charykov Serhii Kostel Simon Kennedy Sin-Woo Bang diff --git a/aiohttp/connector.py b/aiohttp/connector.py index 10ad3311e01..4b4aeab5ce3 100644 --- a/aiohttp/connector.py +++ b/aiohttp/connector.py @@ -933,18 +933,27 @@ async def _create_direct_connection( sslcontext = self._get_ssl_context(req) fingerprint = self._get_fingerprint(req) + host = req.url.raw_host + assert host is not None + port = req.port + assert port is not None + host_resolved = asyncio.ensure_future(self._resolve_host( + host, + port, + traces=traces), loop=self._loop) try: # Cancelling this lookup should not cancel the underlying lookup # or else the cancel event will get broadcast to all the waiters # across all connections. - host = req.url.raw_host - assert host is not None - port = req.port - assert port is not None - hosts = await asyncio.shield(self._resolve_host( - host, - port, - traces=traces)) + hosts = await asyncio.shield(host_resolved) + except asyncio.CancelledError: + def drop_exception( + fut: 'asyncio.Future[List[Dict[str, Any]]]' + ) -> None: + with suppress(Exception, asyncio.CancelledError): + fut.result() + host_resolved.add_done_callback(drop_exception) + raise except OSError as exc: # in case of proxy it is not ClientProxyConnectionError # it is problem of resolving proxy ip itself diff --git a/tests/test_connector.py b/tests/test_connector.py index 139736955b8..4a513ed20da 100644 --- a/tests/test_connector.py +++ b/tests/test_connector.py @@ -738,6 +738,50 @@ async def test_tcp_connector_dns_throttle_requests_cancelled_when_close( await f +@pytest.fixture +def dns_response_error(loop): + async def coro(): + # simulates a network operation + await asyncio.sleep(0) + raise socket.gaierror(-3, 'Temporary failure in name resolution') + return coro + + +async def test_tcp_connector_cancel_dns_error_captured( + loop, + dns_response_error) -> None: + + exception_handler_called = False + + def exception_handler(loop, context): + nonlocal exception_handler_called + exception_handler_called = True + + loop.set_exception_handler(mock.Mock(side_effect=exception_handler)) + + with mock.patch('aiohttp.connector.DefaultResolver') as m_resolver: + req = ClientRequest( + method='GET', + url=URL('http://temporary-failure:80'), + loop=loop + ) + conn = aiohttp.TCPConnector( + use_dns_cache=False, + ) + m_resolver().resolve.return_value = dns_response_error() + f = loop.create_task( + conn._create_direct_connection(req, [], ClientTimeout(0)) + ) + + await asyncio.sleep(0) + f.cancel() + with pytest.raises(asyncio.CancelledError): + await f + + gc.collect() + assert exception_handler_called is False + + async def test_tcp_connector_dns_tracing(loop, dns_response) -> None: session = mock.Mock() trace_config_ctx = mock.Mock()