Repository: agronholm/anyio

Issue:
DNS timeouts have a traceback that is confusing for users.
### Discussed in https://github.com/agronholm/anyio/discussions/580

<div type='discussions-op-text'>

<sup>Originally posted by **tomchristie** June 26, 2023</sup>
Prompted by this user comment... https://github.com/encode/httpx/discussions/2167#discussioncomment-6276279

When we see DNS timeouts in `httpx` async users will end up with a traceback that looks like this...

```
▶ python3 ./tooltracker.py
url count: 14
Traceback (most recent call last):
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/anyio/_core/_sockets.py", line 189, in connect_tcp
    addr_obj = ip_address(remote_host)
               ^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/linuxbrew/.linuxbrew/opt/python@3.11/lib/python3.11/ipaddress.py", line 54, in ip_address
    raise ValueError(f'{address!r} does not appear to be an IPv4 or IPv6 address')
ValueError: 'api.github.com' does not appear to be an IPv4 or IPv6 address

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/httpcore/backends/asyncio.py", line 114, in connect_tcp
    stream: anyio.abc.ByteStream = await anyio.connect_tcp(
                                   ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/anyio/_core/_sockets.py", line 192, in connect_tcp
    gai_res = await getaddrinfo(
              ^^^^^^^^^^^^^^^^^^
asyncio.exceptions.CancelledError

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/httpcore/_exceptions.py", line 10, in map_exceptions
    yield
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/httpcore/backends/asyncio.py", line 113, in connect_tcp
    with anyio.fail_after(timeout):
  File "/home/nem/.cache/pypoetry/virtualenvs/tool-tracker-YFqSduNZ-py3.11/lib/python3.11/site-packages/anyio/_core/_tasks.py", line 119, in __exit__
    raise TimeoutError
TimeoutError

...
```

This looked confusing to me since it starts with the `ValueError: 'api.github.com' does not appear to be an IPv4 or IPv6 address` line.

Once I'd dug into the source for anyio 3.7 it made sense...

https://github.com/agronholm/anyio/blob/f7a880ffac4766efb39e6fb60fc28d944f5d2f65/src/anyio/_core/_sockets.py#L187-L194

...okay, is this an IP address? If not, raise an exception and then perform DNS lookup.

So the behaviour is valid and correct. But I'm wondering if there's any scope for a UX improvement here?

Restructuring the code so that the DNS lookup doesn't occur within an `except` block would result in a more obvious stacktrace for this fairly common exception case.

</div>

Discussion:

Patch:
diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst
index 9c22867..761be04 100644
--- a/docs/versionhistory.rst
+++ b/docs/versionhistory.rst
@@ -3,6 +3,11 @@ Version history
 
 This library adheres to `Semantic Versioning 2.0 <http://semver.org/>`_.
 
+**UNRELEASED**
+
+- Fixed a misleading ``ValueError`` in the context of DNS failures
+  (`#815 <https://github.com/agronholm/anyio/issues/815>`_; PR by @graingert)
+
 **4.6.2**
 
 - Fixed regression caused by (`#807 <https://github.com/agronholm/anyio/pull/807>`_)
diff --git a/src/anyio/_core/_sockets.py b/src/anyio/_core/_sockets.py
index 6070c64..fbfe258 100644
--- a/src/anyio/_core/_sockets.py
+++ b/src/anyio/_core/_sockets.py
@@ -186,6 +186,14 @@ async def connect_tcp(
     try:
         addr_obj = ip_address(remote_host)
     except ValueError:
+        addr_obj = None
+
+    if addr_obj is not None:
+        if isinstance(addr_obj, IPv6Address):
+            target_addrs = [(socket.AF_INET6, addr_obj.compressed)]
+        else:
+            target_addrs = [(socket.AF_INET, addr_obj.compressed)]
+    else:
         # getaddrinfo() will raise an exception if name resolution fails
         gai_res = await getaddrinfo(
             target_host, remote_port, family=family, type=socket.SOCK_STREAM
@@ -194,7 +202,7 @@ async def connect_tcp(
         # Organize the list so that the first address is an IPv6 address (if available)
         # and the second one is an IPv4 addresses. The rest can be in whatever order.
         v6_found = v4_found = False
-        target_addrs: list[tuple[socket.AddressFamily, str]] = []
+        target_addrs = []
         for af, *rest, sa in gai_res:
             if af == socket.AF_INET6 and not v6_found:
                 v6_found = True
@@ -204,11 +212,6 @@ async def connect_tcp(
                 target_addrs.insert(1, (af, sa[0]))
             else:
                 target_addrs.append((af, sa[0]))
-    else:
-        if isinstance(addr_obj, IPv6Address):
-            target_addrs = [(socket.AF_INET6, addr_obj.compressed)]
-        else:
-            target_addrs = [(socket.AF_INET, addr_obj.compressed)]
 
     oserrors: list[OSError] = []
     async with create_task_group() as tg:


Test patch:
diff --git a/tests/test_sockets.py b/tests/test_sockets.py
index 42937a3..0920f6e 100644
--- a/tests/test_sockets.py
+++ b/tests/test_sockets.py
@@ -1838,3 +1838,14 @@ async def test_getnameinfo() -> None:
     expected_result = socket.getnameinfo(("127.0.0.1", 6666), 0)
     result = await getnameinfo(("127.0.0.1", 6666))
     assert result == expected_result
+
+
+async def test_connect_tcp_getaddrinfo_context() -> None:
+    """
+    See https://github.com/agronholm/anyio/issues/815
+    """
+    with pytest.raises(socket.gaierror) as exc_info:
+        async with await connect_tcp("anyio.invalid", 6666):
+            pass
+
+    assert exc_info.value.__context__ is None