Make the ares resolver more consistent about errors.

Addresses #1459

docs/changes/1459.bugfix

+The c-ares resolver now raises exceptions much more consistently with
+the standard resolver. Types and errnos are more likely to match.

src/gevent/resolver/ares.py

@@ -8,12 +8,14 @@ import os
 from _socket import getaddrinfo as native_getaddrinfo
 from _socket import gethostbyname_ex as native_gethostbyname_ex
 from _socket import gaierror
+from _socket import herror
 from _socket import error
+from _socket import EAI_NONAME
 
-from gevent._compat import string_types
 from gevent._compat import text_type
 from gevent._compat import integer_types
 from gevent._compat import PY3
+from gevent._compat import MAC
 
 from gevent.hub import Waiter
 from gevent.hub import get_hub
@@ -75,7 +77,9 @@ class Resolver(AbstractResolver):
       the hosts file.
 
     - This implementation may raise ``gaierror(4)`` where the
-      system implementation would raise ``herror(1)``.
+      system implementation would raise ``herror(1)`` or vice versa,
+      with different error numbers. However, after 2020-04, this should be
+      much reduced.
 
     - The results for ``localhost`` may be different. In
       particular, some system resolvers will return more results
@@ -110,6 +114,10 @@ class Resolver(AbstractResolver):
        ``getaddrinfo`` is now implemented using the native c-ares function
        from c-ares 1.16 or newer.
 
+    .. versionchanged:: NEXT
+        Now ``herror`` and ``gaierror`` are raised more consistently with
+        the standard library resolver, and have more consistent errno values.
+
     .. _c-ares: http://c-ares.haxx.se
     """
 
@@ -160,6 +168,13 @@ class Resolver(AbstractResolver):
 
     _LOCAL_AND_BROADCAST_HOSTNAMES = _LOCAL_HOSTNAMES + (
         b'255.255.255.255',
+        b'<broadcast>',
+    )
+
+    EAI_NONAME_MSG = (
+        'nodename nor servname provided, or not known'
+        if MAC else
+        'Name or service not known'
     )
 
     def _hostname_to_bytes(self, hostname):
@@ -183,10 +198,16 @@ class Resolver(AbstractResolver):
                 ares.gethostbyname(waiter, hostname, family)
                 result = waiter.get()
                 if not result[-1]:
-                    raise gaierror(-5, 'No address associated with hostname')
+                    raise herror(EAI_NONAME, self.EAI_NONAME_MSG)
                 return result
-            except gaierror:
+            except herror as ex:
                 if ares is self.cares:
+                    if ex.args[0] == 1:
+                        # Somewhere along the line, the internal
+                        # implementation of gethostbyname_ex changed to invoke
+                        # getaddrinfo() as a first pass, much like we do for ``getnameinfo()``;
+                        # this means it raises a different error for not-found hosts.
+                        raise gaierror(EAI_NONAME, self.EAI_NONAME_MSG)
                     raise
                 # "self.cares is not ares" means channel was destroyed (because we were forked)
 
@@ -248,7 +269,7 @@ class Resolver(AbstractResolver):
         result = waiter.get()
 
         if not result:
-            raise gaierror(-5, 'No address associated with hostname')
+            raise gaierror(EAI_NONAME, self.EAI_NONAME_MSG)
 
         if fill_in_type_proto:
             # c-ares 1.16 DOES NOT fill in socktype or proto in the results,
@@ -318,7 +339,7 @@ class Resolver(AbstractResolver):
             ares = self.cares
             try:
                 return self._gethostbyaddr(ip_address)
-            except gaierror:
+            except herror:
                 if ares is self.cares:
                     raise
 
@@ -347,8 +368,8 @@ class Resolver(AbstractResolver):
             # requested, node or service will be NULL ". Python 2
             # allows that for the service, but Python 3 raises
             # an error. This is tested by test_socket in py 3.4
-            err = gaierror('nodename nor servname provided, or not known')
-            err.errno = 8
+            err = gaierror(EAI_NONAME, self.EAI_NONAME_MSG)
+            err.errno = EAI_NONAME
             raise err
 
         return node, service or '0'

src/gevent/resolver/cares.pyx

@@ -16,8 +16,11 @@ from cpython.mem cimport PyMem_Malloc
 from cpython.mem cimport PyMem_Free
 from libc.string cimport memset
 
+from gevent._compat import MAC
+
 import _socket
 from _socket import gaierror
+from _socket import herror
 
 
 __all__ = ['channel']
@@ -46,6 +49,31 @@ cdef extern from *:
 #ifdef HAVE_NETDB_H
 #include <netdb.h>
 #endif
+
+    #ifndef EAI_ADDRFAMILY
+    #define EAI_ADDRFAMILY -1
+    #endif
+
+    #ifndef EAI_BADHINTS
+    #define EAI_BADHINTS -2
+    #endif
+
+    #ifndef EAI_NODATA
+    #define EAI_NODATA -3
+    #endif
+
+    #ifndef EAI_OVERFLOW
+    #define EAI_OVERFLOW -4
+    #endif
+
+    #ifndef EAI_PROTOCOL
+    #define EAI_PROTOCOL -5
+    #endif
+
+    #ifndef EAI_SYSTEM
+    #define EAI_SYSTEM
+    #endif
+
     """
 
 cdef extern from "ares.h":
@@ -98,7 +126,7 @@ cdef int NI_NAMEREQD = _socket.NI_NAMEREQD
 cdef int NI_DGRAM = _socket.NI_DGRAM
 
 
-_ares_errors = dict([
+cdef dict _ares_errors = dict([
     (cares.ARES_SUCCESS, 'ARES_SUCCESS'),
 
     (cares.ARES_EADDRGETNETWORKPARAMS, 'ARES_EADDRGETNETWORKPARAMS'),
@@ -128,9 +156,67 @@ _ares_errors = dict([
     (cares.ARES_ETIMEOUT, 'ARES_ETIMEOUT'),
 ])
 
+cdef dict _ares_to_gai_system = {
+    cares.ARES_EBADFAMILY: cares.EAI_ADDRFAMILY,
+    cares.ARES_EBADFLAGS:  cares.EAI_BADFLAGS,
+    cares.ARES_EBADHINTS:  cares.EAI_BADHINTS,
+    cares.ARES_ENOMEM:     cares.EAI_MEMORY,
+    cares.ARES_ENONAME:    cares.EAI_NONAME,
+    cares.ARES_ENOTFOUND:  cares.EAI_NONAME,
+    cares.ARES_ENOTIMP:    cares.EAI_FAMILY,
+    # While EAI_NODATA ("No address associated with nodename") might
+    # seem to be the natural mapping, typical resolvers actually
+    # return EAI_NONAME in that same situation; I've yet to find EAI_NODATA
+    # in a test.
+    cares.ARES_ENODATA:    cares.EAI_NONAME,
+    # This one gets raised for unknown port/service names.
+    cares.ARES_ESERVICE:   cares.EAI_NONAME if MAC else cares.EAI_SERVICE,
+}
+
+cdef _gevent_gai_strerror(code):
+    cdef const char* err_str
+    cdef object result = None
+    cdef int system
+    try:
+        system = _ares_to_gai_system[code]
+    except KeyError:
+        err_str = cares.ares_strerror(code)
+        result = '%s: %s' % (_ares_errors.get(code) or code, _as_str(err_str))
+    else:
+        err_str = cares.gai_strerror(system)
+        result = _as_str(err_str)
+    return result
+
+cdef object _gevent_gaierror_from_status(int ares_status):
+    cdef object code = _ares_to_gai_system.get(ares_status, ares_status)
+    cdef object message = _gevent_gai_strerror(ares_status)
+    return gaierror(code, message)
+
+cdef dict _ares_to_host_system = {
+    cares.ARES_ENONAME:    cares.HOST_NOT_FOUND,
+    cares.ARES_ENOTFOUND:  cares.HOST_NOT_FOUND,
+    cares.ARES_ENODATA:    cares.NO_DATA,
+}
+
+cdef _gevent_herror_strerror(code):
+    cdef const char* err_str
+    cdef object result = None
+    cdef int system
+    try:
+        system = _ares_to_host_system[code]
+    except KeyError:
+        err_str = cares.ares_strerror(code)
+        result = '%s: %s' % (_ares_errors.get(code) or code, _as_str(err_str))
+    else:
+        err_str = cares.hstrerror(system)
+        result = _as_str(err_str)
+    return result
 
-cpdef strerror(code):
-    return '%s: %s' % (_ares_errors.get(code) or code, cares.ares_strerror(code))
+
+cdef object _gevent_herror_from_status(int ares_status):
+    cdef object code = _ares_to_host_system.get(ares_status, ares_status)
+    cdef object message = _gevent_herror_strerror(ares_status)
+    return herror(code, message)
 
 
 class InvalidIP(ValueError):
@@ -217,29 +303,6 @@ cdef list _parse_h_addr_list(hostent* host):
     return result
 
 
-cdef void gevent_ares_host_callback(void *arg, int status, int timeouts, hostent* host):
-    cdef channel channel
-    cdef object callback
-    channel, callback = <tuple>arg
-    Py_DECREF(<tuple>arg)
-    cdef object host_result
-    try:
-        if status or not host:
-            callback(Result(None, gaierror(status, strerror(status))))
-        else:
-            try:
-                host_result = ares_host_result(host.h_addrtype,
-                                               (_as_str(host.h_name),
-                                                _parse_h_aliases(host),
-                                                _parse_h_addr_list(host)))
-            except:
-                callback(Result(None, sys.exc_info()[1]))
-            else:
-                callback(Result(host_result))
-    except:
-        channel.loop.handle_error(callback, *sys.exc_info())
-
-
 cdef object _as_str(const char* val):
     if not val:
         return None
@@ -258,7 +321,7 @@ cdef void gevent_ares_nameinfo_callback(void *arg, int status, int timeouts, cha
     cdef object service
     try:
         if status:
-            callback(Result(None, gaierror(status, strerror(status))))
+            callback(Result(None, _gevent_gaierror_from_status(status)))
         else:
             node = _as_str(c_node)
             service = _as_str(c_service)
@@ -328,10 +391,10 @@ cdef class channel:
 
         cdef int result = cares.ares_library_init(cares.ARES_LIB_INIT_ALL)  # ARES_LIB_INIT_WIN32 -DUSE_WINSOCK?
         if result:
-            raise gaierror(result, strerror(result))
+            raise gaierror(result, _gevent_gai_strerror(result))
         result = cares.ares_init_options(&channel, &options, optmask)
         if result:
-            raise gaierror(result, strerror(result))
+            raise gaierror(result, _gevent_gai_strerror(result))
         self._timer = loop.timer(TIMEOUT, TIMEOUT)
         self._watchers = {}
         self.channel = channel
@@ -398,7 +461,7 @@ cdef class channel:
                 c_servers[length - 1].next = NULL
                 index = cares.ares_set_servers(self.channel, c_servers)
                 if index:
-                    raise ValueError(strerror(index))
+                    raise ValueError(_gevent_gai_strerror(index))
             finally:
                 PyMem_Free(c_servers)
 
@@ -449,6 +512,30 @@ cdef class channel:
             write_fd = cares.ARES_SOCKET_BAD
         cares.ares_process_fd(self.channel, read_fd, write_fd)
 
+    @staticmethod
+    cdef void _gethostbyname_or_byaddr_cb(void *arg, int status, int timeouts, hostent* host):
+        cdef channel channel
+        cdef object callback
+        channel, callback = <tuple>arg
+        Py_DECREF(<tuple>arg)
+        cdef object host_result
+        try:
+            if status or not host:
+                callback(Result(None, _gevent_herror_from_status(status)))
+            else:
+                try:
+                    host_result = ares_host_result(host.h_addrtype,
+                                                   (_as_str(host.h_name),
+                                                    _parse_h_aliases(host),
+                                                    _parse_h_addr_list(host)))
+                except:
+                    callback(Result(None, sys.exc_info()[1]))
+                else:
+                    callback(Result(host_result))
+        except:
+            channel.loop.handle_error(callback, *sys.exc_info())
+
+
     def gethostbyname(self, object callback, char* name, int family=AF_INET):
         if not self.channel:
             raise gaierror(cares.ARES_EDESTRUCTION, 'this ares channel has been destroyed')
@@ -456,7 +543,7 @@ cdef class channel:
         cdef object arg = (self, callback)
         Py_INCREF(arg)
         cares.ares_gethostbyname(self.channel, name, family,
-                                 <void*>gevent_ares_host_callback, <void*>arg)
+                                 <void*>channel._gethostbyname_or_byaddr_cb, <void*>arg)
 
     def gethostbyaddr(self, object callback, char* addr):
         if not self.channel:
@@ -475,7 +562,8 @@ cdef class channel:
             raise InvalidIP(repr(addr))
         cdef object arg = (self, callback)
         Py_INCREF(arg)
-        cares.ares_gethostbyaddr(self.channel, addr_packed, length, family, <void*>gevent_ares_host_callback, <void*>arg)
+        cares.ares_gethostbyaddr(self.channel, addr_packed, length, family,
+                                 <void*>channel._gethostbyname_or_byaddr_cb, <void*>arg)
 
     cpdef _getnameinfo(self, object callback, tuple sockaddr, int flags):
         if not self.channel:
@@ -488,8 +576,8 @@ cdef class channel:
         if not PyTuple_Check(sockaddr):
             raise TypeError('expected a tuple, got %r' % (sockaddr, ))
         PyArg_ParseTuple(sockaddr, "si|ii", &hostp, &port, &flowinfo, &scope_id)
-        if port < 0 or port > 65535:
-            raise gaierror(-8, 'Invalid value for port: %r' % port)
+        # if port < 0 or port > 65535:
+        #     raise gaierror(-8, 'Invalid value for port: %r' % port)
         cdef int length = _make_sockaddr(hostp, port, flowinfo, scope_id, &sa6)
         if length <= 0:
             raise InvalidIP(repr(hostp))
@@ -560,7 +648,7 @@ cdef class channel:
         addrs = []
         try:
             if status != cares.ARES_SUCCESS:
-                callback(Result(None, gaierror(status, strerror(status))))
+                callback(Result(None, _gevent_gaierror_from_status(status)))
                 return
             if result.cnames:
                 # These tend to come in pairs:

src/gevent/resolver/libcares.pxd

@@ -7,6 +7,34 @@ cdef extern from "ares.h":
     struct hostent:
         pass
 
+    # Errors from getaddrinfo
+    int EAI_ADDRFAMILY # The specified network host does not have
+                       # any network addresses in the requested address family (Linux)
+
+    int EAI_AGAIN     # temporary failure in name resolution
+    int EAI_BADFLAGS  # invalid value for ai_flags
+    int EAI_BADHINTS  # invalid value for hints (macOS only)
+    int EAI_FAIL      # non-recoverable failure in name resolution
+    int EAI_FAMILY    # ai_family not supported
+    int EAI_MEMORY    # memory allocation failure
+    int EAI_NODATA    # The specified network host exists, but does not have
+                      # any network addresses defined. (Linux)
+    int EAI_NONAME    # hostname or servname not provided, or not known
+    int EAI_OVERFLOW  # argument buffer overflow (macOS only)
+    int EAI_PROTOCOL  # resolved protocol is unknown (macOS only)
+    int EAI_SERVICE   # servname not supported for ai_socktype
+    int EAI_SOCKTYPE  # ai_socktype not supported
+    int EAI_SYSTEM    # system error returned in errno (macOS and Linux)
+
+    char* gai_strerror(int ecode)
+
+    # Errors from gethostbyname and gethostbyaddr
+    int HOST_NOT_FOUND
+    int TRY_AGAIN
+    int NO_RECOVERY
+    int NO_DATA
+    char* hstrerror(int err)
+
     struct ares_options:
         int flags
         void* sock_state_cb

src/gevent/testing/monkey_test.py

@@ -5,6 +5,9 @@ import os
 test_filename = sys.argv[1]
 del sys.argv[1]
 
+if test_filename == 'test_urllib2_localnet.py' and os.environ.get('APPVEYOR'):
+    os.environ['GEVENT_DEBUG'] = 'TRACE'
+
 print('Running with patch_all(): %s' % (test_filename,))
 
 from gevent import monkey

src/gevent/tests/test__ares_timeout.py

@@ -35,7 +35,7 @@ class TestTimeout(greentest.TestCase):
         r = Resolver(servers=[address[0]], timeout=0.001, tries=1,
                      udp_port=address[-1])
 
-        with self.assertRaisesRegex(socket.gaierror, "ARES_ETIMEOUT"):
+        with self.assertRaisesRegex(socket.herror, "ARES_ETIMEOUT"):
             r.gethostbyname('www.google.com')
 
 

src/gevent/tests/test__socket_dns.py

@@ -306,40 +306,43 @@ class TestCase(greentest.TestCase):
             return getattr(self, norm_name)(result)
         return result
 
-    def _normalize_result_gethostbyname_ex(self, result):
-        # Often the second and third part of the tuple (hostname, aliaslist, ipaddrlist)
-        # can be in different orders if we're hitting different servers,
-        # or using the native and ares resolvers due to load-balancing techniques.
-        # We sort them.
-        if not RESOLVER_NOT_SYSTEM or isinstance(result, BaseException):
-            return result
-        # result[1].sort() # we wind up discarding this
-
-        # On Py2 in test_russion_gethostbyname_ex, this
-        # is actually an integer, for some reason. In TestLocalhost.tets__ip6_localhost,
-        # the result isn't this long (maybe an error?).
-        try:
-            result[2].sort()
-        except AttributeError:
-            pass
-        except IndexError:
-            return result
-        # On some systems, a random alias is found in the aliaslist
-        # by the system resolver, but not by cares, and vice versa. We deem the aliaslist
-        # unimportant and discard it.
-        # On some systems (Travis CI), the ipaddrlist for 'localhost' can come back
-        # with two entries 127.0.0.1 (presumably two interfaces?) for c-ares
-        ips = result[2]
-        if ips == ['127.0.0.1', '127.0.0.1']:
-            ips = ['127.0.0.1']
-        # On some systems, the hostname can get caps
-        return (result[0].lower(), [], ips)
 
     NORMALIZE_GAI_IGNORE_CANONICAL_NAME = RESOLVER_ARES # It tends to return them even when not asked for
     if not RESOLVER_NOT_SYSTEM:
         def _normalize_result_getaddrinfo(self, result):
             return result
+        def _normalize_result_gethostbyname_ex(self, result):
+            return result
     else:
+        def _normalize_result_gethostbyname_ex(self, result):
+            # Often the second and third part of the tuple (hostname, aliaslist, ipaddrlist)
+            # can be in different orders if we're hitting different servers,
+            # or using the native and ares resolvers due to load-balancing techniques.
+            # We sort them.
+            if isinstance(result, BaseException):
+                return result
+            # result[1].sort() # we wind up discarding this
+
+            # On Py2 in test_russion_gethostbyname_ex, this
+            # is actually an integer, for some reason. In TestLocalhost.tets__ip6_localhost,
+            # the result isn't this long (maybe an error?).
+            try:
+                result[2].sort()
+            except AttributeError:
+                pass
+            except IndexError:
+                return result
+            # On some systems, a random alias is found in the aliaslist
+            # by the system resolver, but not by cares, and vice versa. We deem the aliaslist
+            # unimportant and discard it.
+            # On some systems (Travis CI), the ipaddrlist for 'localhost' can come back
+            # with two entries 127.0.0.1 (presumably two interfaces?) for c-ares
+            ips = result[2]
+            if ips == ['127.0.0.1', '127.0.0.1']:
+                ips = ['127.0.0.1']
+            # On some systems, the hostname can get caps
+            return (result[0].lower(), [], ips)
+
         def _normalize_result_getaddrinfo(self, result):
             # Result is a list
             # (family, socktype, proto, canonname, sockaddr)
@@ -387,13 +390,27 @@ class TestCase(greentest.TestCase):
             return (result[0], [], result[2])
         return result
 
-    def assertEqualResults(self, real_result, gevent_result, func):
-        errors = (socket.gaierror, socket.herror, TypeError)
-        if isinstance(real_result, errors) and isinstance(gevent_result, errors):
+    def _compare_exceptions(self, real_result, gevent_result):
+        msg = ('system:', repr(real_result), 'gevent:', repr(gevent_result))
+        self.assertIs(type(gevent_result), type(real_result), msg)
+        if isinstance(real_result, TypeError):
+            return
+
+        self.assertEqual(real_result.args, gevent_result.args, msg)
+        if hasattr(real_result, 'errno'):
+            self.assertEqual(real_result.errno, gevent_result.errno)
+
+    if RESOLVER_DNSPYTHON:
+        def _compare_exceptions(self, real_result, gevent_result):
             if type(real_result) is not type(gevent_result):
                 util.log('WARNING: error type mismatch: %r (gevent) != %r (stdlib)',
                          gevent_result, real_result,
                          color='warning')
+
+    def assertEqualResults(self, real_result, gevent_result, func):
+        errors = (socket.gaierror, socket.herror, TypeError)
+        if isinstance(real_result, errors) and isinstance(gevent_result, errors):
+            self._compare_exceptions(real_result, gevent_result)
             return
 
         real_result = self._normalize_result(real_result, func)
@@ -490,15 +507,13 @@ class TestLocalhost(TestCase):
 
 add(
     TestLocalhost, 'ip6-localhost',
-    skip=greentest.RUNNING_ON_TRAVIS,
-    skip_reason="ares fails here, for some reason, presumably a badly "
-    "configured /etc/hosts"
+    skip=RESOLVER_DNSPYTHON, # XXX: Fix these.
+    skip_reason="Can return gaierror(-2)"
 )
 add(
     TestLocalhost, 'localhost',
     skip=greentest.RUNNING_ON_TRAVIS,
-    skip_reason="Beginning Dec 1 2017, ares started returning ip6-localhost "
-    "instead of localhost"
+    skip_reason="Can return gaierror(-2)"
 )
 
 
@@ -519,9 +534,9 @@ class Test127001(TestCase):
 
 add(
     Test127001, '127.0.0.1',
-    skip=greentest.RUNNING_ON_TRAVIS,
-    skip_reason="Beginning Dec 1 2017, ares started returning ip6-localhost "
-    "instead of localhost"
+    # skip=RESOLVER_DNSPYTHON,
+    # skip_reason="Beginning Dec 1 2017, ares started returning ip6-localhost "
+    # "instead of localhost"
 )
 
 
@@ -529,7 +544,7 @@ add(
 class TestBroadcast(TestCase):
     switch_expected = False
 
-    if RESOLVER_NOT_SYSTEM:
+    if RESOLVER_DNSPYTHON:
         # ares and dnspython raises errors for broadcasthost/255.255.255.255
 
         @unittest.skip('ares raises errors for broadcasthost/255.255.255.255')

src/gevent/tests/test__socket_dns6.py

@@ -3,6 +3,7 @@
 from __future__ import print_function, absolute_import, division
 
 import socket
+import unittest
 
 import gevent.testing as greentest
 from gevent.tests.test__socket_dns import TestCase, add
@@ -51,6 +52,11 @@ class Test6(TestCase):
         def _run_test_getnameinfo(self, *_args):
             return (), 0, (), 0
 
+    def _run_test_gethostbyname(self, *_args):
+        raise unittest.SkipTest("gethostbyname[_ex] does not support IPV6")
+
+    _run_test_gethostbyname_ex = _run_test_gethostbyname
+
     def test_empty(self):
         self._test('getaddrinfo', self.host, 'http')