Skip to content

Commit

Permalink
Fix resolver task is not awaited when connector is cancelled (#4795)
Browse files Browse the repository at this point in the history
* Add test to reproduce the issue

"Task exception was never retrieved"
on connector cancellation and dns resolver
returns error

* Fix connector launching resolver task is not awaited

in case of CancelledError

* Add myself to contributors

* Add file to CHANGES

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
  • Loading branch information
moznuy and asvetlov committed Oct 16, 2020
1 parent b0bcf02 commit 4d71db9
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGES/4795.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix resolver task is not awaited when connector is cancelled
1 change: 1 addition & 0 deletions CONTRIBUTORS.txt
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ Sebastien Geffroy
SeongSoo Cho
Sergey Ninua
Sergey Skripnick
Serhii Charykov
Serhii Kostel
Simon Kennedy
Sin-Woo Bang
Expand Down
25 changes: 17 additions & 8 deletions aiohttp/connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
44 changes: 44 additions & 0 deletions tests/test_connector.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 4d71db9

Please sign in to comment.