[Tarantool-patches] [PATCH v2 2/2] lua: return getaddrinfo() errors

Roman Khabibov roman.habibov at tarantool.org
Mon Apr 6 05:08:02 MSK 2020



> On Mar 29, 2020, at 12:07, Sergey Ostanevich <sergos at tarantool.org> wrote:
> 
> Hi!
> 
> Thanks for the patch, please see below.
> 
> Sergos
> 
> On 12 мар 13:24, Roman Khabibov wrote:
>> Add getaddrinfo() errors into the several fuctions of socket. Now
>> getaddrinfo() can return a pair of values (nil and error message)
>> in case of error.
>> 
>> Closes #4138
>> 
>> @TarantoolBot document
>> Title: socket API changes
>> 
>> * socket.getaddrinfo()
>> 
>> Can return error message as second return value in case of
>> internal getaddrinfo() error.
> I really don't like the case 'can return'. Can we work this out to
> always return a pair of values?
Done.
>> 
>> Example:
>> tarantool> socket.getaddrinfo('non_exists_hostname', 3301)
>> ---
>> - null
>> - 'getaddrinfo: nodename nor servname provided, or not known'
>> ...
>> 
>> * socket.tcp_connect()
>> 
>> Can return socket.getaddrinfo() error message as second return
>> value.
>> 
>> Example:
>> tarantool> socket.tcp_connect('non_exists_hostname', 3301)
>> ---
>> - null
>> - 'getaddrinfo: nodename nor servname provided, or not known'
>> ...
>> ---
>> src/box/lua/net_box.lua   |  7 +++--
>> src/lua/socket.c          | 16 +++++++++-
>> src/lua/socket.lua        | 22 ++++++++++----
>> test/app/socket.result    | 63 ++++++++++++++++++++++++++++++++++++---
>> test/app/socket.test.lua  | 35 ++++++++++++++++++++--
>> test/box/net.box.result   | 31 +++++++++++++++++++
>> test/box/net.box.test.lua | 20 +++++++++++++
>> 7 files changed, 179 insertions(+), 15 deletions(-)
>> 
>> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
>> index 3f611c027..8068a8637 100644
>> --- a/src/box/lua/net_box.lua
>> +++ b/src/box/lua/net_box.lua
>> @@ -154,9 +154,12 @@ local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
>> local function establish_connection(host, port, timeout)
>>     local timeout = timeout or DEFAULT_CONNECT_TIMEOUT
>>     local begin = fiber.clock()
>> -    local s = socket.tcp_connect(host, port, timeout)
>> +    local s, err = socket.tcp_connect(host, port, timeout)
>>     if not s then
>> -        return nil, errno.strerror(errno())
>> +        if not err then
>> +            return nil, errno.strerror(errno())
>> +        end
>> +        return nil, err
>>     end
>>     local msg = s:read({chunk = IPROTO_GREETING_SIZE},
>>                         timeout - (fiber.clock() - begin))
>> diff --git a/src/lua/socket.c b/src/lua/socket.c
>> index e75a8802e..ba683ad3a 100644
>> --- a/src/lua/socket.c
>> +++ b/src/lua/socket.c
>> @@ -775,6 +775,18 @@ lbox_getaddrinfo_result_wrapper(struct lua_State *L)
>> 	return 1;
>> }
>> 
>> +/**
>> + * Wrap coio_getaddrinfo() and call it. Push returned values onto
>> + * @a L Lua stack.
>> + *
>> + * @param L Lua stack.
>> + *
>> + * @retval 1 Number of returned values by Lua function if
>> + * coio_getaddrinfo() success.
>> + * @retval 2 Number of returned values by Lua function if
>> + * coio_getaddrinfo() is failed (first is nil, second is error
>> + * message).
>> + */
>> static int
>> lbox_socket_getaddrinfo(struct lua_State *L)
>> {
>> @@ -817,7 +829,9 @@ lbox_socket_getaddrinfo(struct lua_State *L)
>> 
>> 	if (dns_res != 0) {
>> 		lua_pushnil(L);
>> -		return 1;
>> +		struct error *err = diag_get()->last;
>> +		lua_pushstring(L, err->errmsg);
>> +		return 2;
>> 	}
>> 
>> 	/* no results */
>> diff --git a/src/lua/socket.lua b/src/lua/socket.lua
>> index a334ad45b..ce6dab301 100644
>> --- a/src/lua/socket.lua
>> +++ b/src/lua/socket.lua
>> @@ -1041,9 +1041,12 @@ local function tcp_connect(host, port, timeout)
>>     end
>>     local timeout = timeout or TIMEOUT_INFINITY
>>     local stop = fiber.clock() + timeout
>> -    local dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
>> +    local dns, err = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
>>         protocol = 'tcp' })
>> -    if dns == nil or #dns == 0 then
>> +    if dns == nil then
>> +        return nil, err
> You explicitly put error here...
> 
>> +    end
>> +    if #dns == 0 then
>>         boxerrno(boxerrno.EINVAL)
>>         return nil
> ... and not here. Why do you keep using two versions of error passing,
> not one? I would agree you need boxerrno for backward compatibility, but
> to start moving towards {res, err} you have to introduce it everywhere.
@@ -1028,30 +1028,33 @@ local function tcp_connect(host, port, timeout)
         local s = socket_new('AF_UNIX', 'SOCK_STREAM', 0)
         if not s then
             -- Address family is not supported by the host
-            return nil
+            return nil, boxerrno.strerror()
         end
         if not socket_tcp_connect(s, host, port, timeout) then
             local save_errno = s._errno
             s:close()
             boxerrno(save_errno)
-            return nil
+            return nil, boxerrno.strerror()
         end
         boxerrno(0)
         return s
     end
     local timeout = timeout or TIMEOUT_INFINITY
     local stop = fiber.clock() + timeout
-    local dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
+    local dns, err = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
         protocol = 'tcp' })
-    if dns == nil or #dns == 0 then
+    if dns == nil then
+        return nil, err
+    end
+    if #dns == 0 then
         boxerrno(boxerrno.EINVAL)
-        return nil
+        return nil, boxerrno.strerror()
     end
     for i, remote in pairs(dns) do
         timeout = stop - fiber.clock()
         if timeout <= 0 then
             boxerrno(boxerrno.ETIMEDOUT)
-            return nil
+            return nil, boxerrno.strerror()
         end
         local s = socket_new(remote.family, remote.type, remote.protocol)
         if s then


>>     end
>> @@ -1360,8 +1363,12 @@ local function lsocket_tcp_connect(self, host, port)
>>     -- This function is broken by design
>>     local ga_opts = { family = 'AF_INET', type = 'SOCK_STREAM' }
>>     local timeout = deadline - fiber.clock()
>> -    local dns = getaddrinfo(host, port, timeout, ga_opts)
>> -    if dns == nil or #dns == 0 then
>> +    local dns, err = getaddrinfo(host, port, timeout, ga_opts)
>> +    if dns == nil then
>> +        self._errno = boxerrno.EINVAL
>> +        return nil, err
>> +    end
>> +    if #dns == 0 then
>>         self._errno = boxerrno.EINVAL
>>         return nil, socket_error(self)
>>     end
>> @@ -1547,9 +1554,12 @@ local function lsocket_connect(host, port)
>>     if host == nil or port == nil then
>>         error("Usage: luasocket.connect(host, port)")
>>     end
>> -    local s = tcp_connect(host, port)
>> +    local s, err = tcp_connect(host, port)
>>     if not s then
>> -        return nil, boxerrno.strerror()
>> +        if not err then
>> +            return nil, boxerrno.strerror()
>> +        end
>> +        return nil, err
>>     end
>>     setmetatable(s, lsocket_tcp_client_mt)
>>     return s
>> diff --git a/test/app/socket.result b/test/app/socket.result
>> index 9f0ea0a5d..3f26bf2b0 100644
>> --- a/test/app/socket.result
>> +++ b/test/app/socket.result
>> @@ -941,10 +941,20 @@ sc:close()
>> - true
>> ...
>> -- tcp_connect
>> --- test timeout
>> -socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
>> +-- Test timeout. In this test, tcp_connect can return the second
>> +-- output value from internal.getaddrinfo (usually on Mac OS, but
>> +-- theoretically it can happen on Linux too). Sometimes
>> +-- getaddrinfo() is timed out, sometimes connect. On Linux however
>> +-- getaddrinfo is fast enough to never give timeout error in
>> +-- the case. So, there are two sources of timeout errors that are
>> +-- reported differently. This difference has appeared after
>> +-- gh-4138 patch.
>> +s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
>> ---
>> -- null
>> +...
>> +s == nil
>> +---
>> +- true
>> ...
>> -- AF_INET
>> s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
>> @@ -1595,7 +1605,7 @@ fio.stat(path) == nil
>> { socket.tcp_connect('abrakadabra#123') == nil, errno.strerror() }
>> ---
>> - - true
>> -  - Invalid argument
>> +  - Input/output error
>> ...
>> -- wrong options for getaddrinfo
>> socket.getaddrinfo('host', 'port', { type = 'WRONG' }) == nil and errno() == errno.EINVAL
>> @@ -2914,3 +2924,48 @@ srv:close()
>> ---
>> - true
>> ...
>> +-- gh-4138 Check getaddrinfo() error from socket:connect() only.
>> +-- Error code and error message returned by getaddrinfo() depends
>> +-- on system's gai_strerror().
>> +test_run:cmd("setopt delimiter ';'")
>> +---
>> +- true
>> +...
>> +function check_err(err)
>> +    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Servname not supported for ai_socktype' or
>> +       err == 'getaddrinfo: Name or service not known' or
>> +       err == 'getaddrinfo: hostname nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Temporary failure in name resolution' or
>> +       err == 'getaddrinfo: Name could not be resolved at this time' then
>> +        return true
>> +    end
>> +    return false
>> +end;
>> +---
>> +...
>> +test_run:cmd("setopt delimiter ''");
>> +---
>> +- true
>> +...
>> +s, err = socket.getaddrinfo('non_exists_hostname', 3301)
>> +---
>> +...
>> +check_err(err)
>> +---
>> +- true
>> +...
>> +s, err = socket.connect('non_exists_hostname', 3301)
>> +---
>> +...
>> +check_err(err)
>> +---
>> +- true
>> +...
>> +s, err = socket.tcp_connect('non_exists_hostname', 3301)
>> +---
>> +...
>> +check_err(err)
>> +---
>> +- true
>> +...
>> diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
>> index d1fe7ada6..c455d3ddb 100644
>> --- a/test/app/socket.test.lua
>> +++ b/test/app/socket.test.lua
>> @@ -300,8 +300,16 @@ sc:close()
>> 
>> -- tcp_connect
>> 
>> --- test timeout
>> -socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
>> +-- Test timeout. In this test, tcp_connect can return the second
>> +-- output value from internal.getaddrinfo (usually on Mac OS, but
>> +-- theoretically it can happen on Linux too). Sometimes
>> +-- getaddrinfo() is timed out, sometimes connect. On Linux however
>> +-- getaddrinfo is fast enough to never give timeout error in
>> +-- the case. So, there are two sources of timeout errors that are
>> +-- reported differently. This difference has appeared after
>> +-- gh-4138 patch.
>> +s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
>> +s == nil
>> 
>> -- AF_INET
>> s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
>> @@ -988,3 +996,26 @@ s:receive('*a')
>> s:close()
>> srv:close()
>> 
>> +-- gh-4138 Check getaddrinfo() error from socket:connect() only.
>> +-- Error code and error message returned by getaddrinfo() depends
>> +-- on system's gai_strerror().
>> +test_run:cmd("setopt delimiter ';'")
>> +function check_err(err)
>> +    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Servname not supported for ai_socktype' or
>> +       err == 'getaddrinfo: Name or service not known' or
>> +       err == 'getaddrinfo: hostname nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Temporary failure in name resolution' or
>> +       err == 'getaddrinfo: Name could not be resolved at this time' then
>> +        return true
>> +    end
>> +    return false
>> +end;
> I really doubt that different error messages from the set above will appear 
> for the same error on different platforms. Could you please check for particular 
> output for each case you trigger below?
Look at that:
https://travis-ci.org/github/tarantool/tarantool/jobs/546115892#L3581 - Linux failed
https://travis-ci.org/github/tarantool/tarantool/jobs/546115893#L3100 - macOS isn’t


>> +test_run:cmd("setopt delimiter ''");
>> +
>> +s, err = socket.getaddrinfo('non_exists_hostname', 3301)
>> +check_err(err)
>> +s, err = socket.connect('non_exists_hostname', 3301)
>> +check_err(err)
>> +s, err = socket.tcp_connect('non_exists_hostname', 3301)
>> +check_err(err)
>> diff --git a/test/box/net.box.result b/test/box/net.box.result
>> index e3dabf7d9..3e1ce6b11 100644
>> --- a/test/box/net.box.result
>> +++ b/test/box/net.box.result
>> @@ -3927,6 +3927,37 @@ test_run:grep_log('default', '00000040:.*')
>> ---
>> - null
>> ...
>> +-- gh-4138 Check getaddrinfo() error from connect() only. Error
>> +-- code and error message returned by getaddrinfo() depends on
>> +-- system's gai_strerror().
>> +test_run:cmd("setopt delimiter ';'")
>> +---
>> +- true
>> +...
>> +function check_err(err)
>> +    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Servname not supported for ai_socktype' or
>> +       err == 'getaddrinfo: Name or service not known' or
>> +       err == 'getaddrinfo: hostname nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Temporary failure in name resolution' or
>> +       err == 'getaddrinfo: Name could not be resolved at this time' then
>> +            return true
>> +    end
>> +    return false
>> +end;
>> +---
>> +...
>> +test_run:cmd("setopt delimiter ''");
>> +---
>> +- true
>> +...
>> +s = remote.connect('non_exists_hostname:3301')
>> +---
>> +...
>> +check_err(s['error'])
>> +---
>> +- true
>> +...
>> box.cfg{log_level=log_level}
>> ---
>> ...
>> diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
>> index 8e65ff470..cad8754c3 100644
>> --- a/test/box/net.box.test.lua
>> +++ b/test/box/net.box.test.lua
>> @@ -1582,4 +1582,24 @@ test_run:wait_log('default', '00000030:.*', nil, 10)
>> -- we expect nothing below, so don't wait
>> test_run:grep_log('default', '00000040:.*')
>> 
>> +-- gh-4138 Check getaddrinfo() error from connect() only. Error
>> +-- code and error message returned by getaddrinfo() depends on
>> +-- system's gai_strerror().
>> +test_run:cmd("setopt delimiter ';'")
>> +function check_err(err)
>> +    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Servname not supported for ai_socktype' or
>> +       err == 'getaddrinfo: Name or service not known' or
>> +       err == 'getaddrinfo: hostname nor servname provided, or not known' or
>> +       err == 'getaddrinfo: Temporary failure in name resolution' or
>> +       err == 'getaddrinfo: Name could not be resolved at this time' then
>> +            return true
>> +    end
>> +    return false
>> +end;
>> +test_run:cmd("setopt delimiter ''");
>> +
>> +s = remote.connect('non_exists_hostname:3301')
>> +check_err(s['error'])
>> +
>> box.cfg{log_level=log_level}
>> -- 
>> 2.21.0 (Apple Git-122)
>> 

commit 57d8b556b98a8b6de4d40f6ae1c5f48498022962
Author: Roman Khabibov <roman.habibov at tarantool.org>
Date:   Thu Nov 21 14:37:54 2019 +0300

    lua: return getaddrinfo() errors
    
    Add getaddrinfo() errors into the several fuctions of socket. Now
    getaddrinfo() return a pair of values (nil and error message) in
    case of error.
    
    Closes #4138
    
    @TarantoolBot document
    Title: socket API changes
    
    * socket.getaddrinfo()
    
    Return error message as the second return value.
    
    Example:
    tarantool> socket.getaddrinfo('non_exists_hostname', 3301)
    ---
    - null
    - 'getaddrinfo: nodename nor servname provided, or not known'
    ...
    
    * socket.tcp_connect()
    
    Return error message as the second return value.
    
    Example:
    tarantool> socket.tcp_connect('non_exists_hostname', 3301)
    ---
    - null
    - 'getaddrinfo: nodename nor servname provided, or not known'
    ...
    
    * socket.bind()
    
    Return error message as the second return value.
    
    Example:
    tarantool> socket.tcp_connect('non_exists_hostname', 3301)
    ---
    - null
    - 'getaddrinfo: nodename nor servname provided, or not known'
    ...
    
    * socket.tcp_server()
    
    Return error message as the second return value.
    
    Example:
    tarantool> socket.tcp_connect('non_exists_hostname', 3301)
    ---
    - null
    - 'getaddrinfo: nodename nor servname provided, or not known'
    ...

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 3f611c027..b5d0dff4a 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -154,9 +154,9 @@ local function next_id(id) return band(id + 1, 0x7FFFFFFF) end
 local function establish_connection(host, port, timeout)
     local timeout = timeout or DEFAULT_CONNECT_TIMEOUT
     local begin = fiber.clock()
-    local s = socket.tcp_connect(host, port, timeout)
+    local s, err = socket.tcp_connect(host, port, timeout)
     if not s then
-        return nil, errno.strerror(errno())
+        return nil, err
     end
     local msg = s:read({chunk = IPROTO_GREETING_SIZE},
                         timeout - (fiber.clock() - begin))
diff --git a/src/lua/socket.c b/src/lua/socket.c
index e75a8802e..ba683ad3a 100644
--- a/src/lua/socket.c
+++ b/src/lua/socket.c
@@ -775,6 +775,18 @@ lbox_getaddrinfo_result_wrapper(struct lua_State *L)
 	return 1;
 }
 
+/**
+ * Wrap coio_getaddrinfo() and call it. Push returned values onto
+ * @a L Lua stack.
+ *
+ * @param L Lua stack.
+ *
+ * @retval 1 Number of returned values by Lua function if
+ * coio_getaddrinfo() success.
+ * @retval 2 Number of returned values by Lua function if
+ * coio_getaddrinfo() is failed (first is nil, second is error
+ * message).
+ */
 static int
 lbox_socket_getaddrinfo(struct lua_State *L)
 {
@@ -817,7 +829,9 @@ lbox_socket_getaddrinfo(struct lua_State *L)
 
 	if (dns_res != 0) {
 		lua_pushnil(L);
-		return 1;
+		struct error *err = diag_get()->last;
+		lua_pushstring(L, err->errmsg);
+		return 2;
 	}
 
 	/* no results */
diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index a334ad45b..57fdc6213 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -963,7 +963,7 @@ local function getaddrinfo(host, port, timeout, opts)
             local itype = get_ivalue(internal.SO_TYPE, opts.type)
             if itype == nil then
                 boxerrno(boxerrno.EINVAL)
-                return nil
+                return nil, boxerrno.strerror()
             end
             ga_opts.type = itype
         end
@@ -972,7 +972,7 @@ local function getaddrinfo(host, port, timeout, opts)
             local ifamily = get_ivalue(internal.DOMAIN, opts.family)
             if ifamily == nil then
                 boxerrno(boxerrno.EINVAL)
-                return nil
+                return nil, boxerrno.strerror()
             end
             ga_opts.family = ifamily
         end
@@ -980,7 +980,7 @@ local function getaddrinfo(host, port, timeout, opts)
         if opts.protocol ~= nil then
             local p = getprotobyname(opts.protocol)
             if p == nil then
-                return nil
+                return nil, boxerrno.strerror()
             end
             ga_opts.protocol = p
         end
@@ -990,7 +990,7 @@ local function getaddrinfo(host, port, timeout, opts)
                 get_iflags(internal.AI_FLAGS, opts.flags)
             if ga_opts.flags == nil then
                 boxerrno(boxerrno.EINVAL)
-                return nil
+                return nil, boxerrno.strerror()
             end
         end
 
@@ -1028,30 +1028,33 @@ local function tcp_connect(host, port, timeout)
         local s = socket_new('AF_UNIX', 'SOCK_STREAM', 0)
         if not s then
             -- Address family is not supported by the host
-            return nil
+            return nil, boxerrno.strerror()
         end
         if not socket_tcp_connect(s, host, port, timeout) then
             local save_errno = s._errno
             s:close()
             boxerrno(save_errno)
-            return nil
+            return nil, boxerrno.strerror()
         end
         boxerrno(0)
         return s
     end
     local timeout = timeout or TIMEOUT_INFINITY
     local stop = fiber.clock() + timeout
-    local dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
+    local dns, err = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
         protocol = 'tcp' })
-    if dns == nil or #dns == 0 then
+    if dns == nil then
+        return nil, err
+    end
+    if #dns == 0 then
         boxerrno(boxerrno.EINVAL)
-        return nil
+        return nil, boxerrno.strerror()
     end
     for i, remote in pairs(dns) do
         timeout = stop - fiber.clock()
         if timeout <= 0 then
             boxerrno(boxerrno.ETIMEDOUT)
-            return nil
+            return nil, boxerrno.strerror()
         end
         local s = socket_new(remote.family, remote.type, remote.protocol)
         if s then
@@ -1065,7 +1068,7 @@ local function tcp_connect(host, port, timeout)
         end
     end
     -- errno is set by socket_tcp_connect()
-    return nil
+    return nil, boxerrno.strerror()
 end
 
 local function tcp_server_handler(server, sc, from)
@@ -1160,15 +1163,15 @@ end
 
 local function tcp_server_bind(host, port, prepare, timeout)
     timeout = timeout and tonumber(timeout) or TIMEOUT_INFINITY
-    local dns
+    local dns, err
     if host == 'unix/' then
         dns = {{host = host, port = port, family = 'AF_UNIX', protocol = 0,
             type = 'SOCK_STREAM' }}
     else
-        dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
+        dns, err = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM',
             flags = 'AI_PASSIVE'})
         if dns == nil then
-            return nil
+            return nil, err
         end
     end
 
@@ -1185,14 +1188,14 @@ local function tcp_server_bind(host, port, prepare, timeout)
                 local save_errno = boxerrno()
                 socket_close(s)
                 boxerrno(save_errno)
-                return nil
+                return nil, boxerrno.strerror()
             end
             return s, addr
        end
     end
     -- DNS resolved successfully, but addresss family is not supported
     boxerrno(boxerrno.EAFNOSUPPORT)
-    return nil
+    return nil, boxerrno.strerror()
 end
 
 local function tcp_server(host, port, opts, timeout)
@@ -1213,7 +1216,8 @@ local function tcp_server(host, port, opts, timeout)
     server.name = server.name or 'server'
     local s, addr = tcp_server_bind(host, port, server.prepare, timeout)
     if not s then
-        return nil
+        -- addr is error message now.
+        return nil, addr
     end
     fiber.create(tcp_server_loop, server, s, addr)
     return s, addr
@@ -1360,8 +1364,12 @@ local function lsocket_tcp_connect(self, host, port)
     -- This function is broken by design
     local ga_opts = { family = 'AF_INET', type = 'SOCK_STREAM' }
     local timeout = deadline - fiber.clock()
-    local dns = getaddrinfo(host, port, timeout, ga_opts)
-    if dns == nil or #dns == 0 then
+    local dns, err = getaddrinfo(host, port, timeout, ga_opts)
+    if dns == nil then
+        self._errno = boxerrno.EINVAL
+        return nil, err
+    end
+    if #dns == 0 then
         self._errno = boxerrno.EINVAL
         return nil, socket_error(self)
     end
@@ -1547,9 +1555,9 @@ local function lsocket_connect(host, port)
     if host == nil or port == nil then
         error("Usage: luasocket.connect(host, port)")
     end
-    local s = tcp_connect(host, port)
+    local s, err = tcp_connect(host, port)
     if not s then
-        return nil, boxerrno.strerror()
+        return nil, err
     end
     setmetatable(s, lsocket_tcp_client_mt)
     return s
@@ -1560,9 +1568,9 @@ local function lsocket_bind(host, port, backlog)
         error("Usage: luasocket.bind(host, port [, backlog])")
     end
     local function prepare(s) return backlog end
-    local s = tcp_server_bind(host, port, prepare)
+    local s, err = tcp_server_bind(host, port, prepare)
     if not s then
-        return nil, boxerrno.strerror()
+        return nil, err
     end
     return setmetatable(s, lsocket_tcp_server_mt)
 end
diff --git a/test/app/socket.result b/test/app/socket.result
index 9f0ea0a5d..6f91b32ee 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -941,10 +941,20 @@ sc:close()
 - true
 ...
 -- tcp_connect
--- test timeout
-socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
+-- Test timeout. In this test, tcp_connect can return the second
+-- output value from internal.getaddrinfo (usually on Mac OS, but
+-- theoretically it can happen on Linux too). Sometimes
+-- getaddrinfo() is timed out, sometimes connect. On Linux however
+-- getaddrinfo is fast enough to never give timeout error in
+-- the case. So, there are two sources of timeout errors that are
+-- reported differently. This difference has appeared after
+-- gh-4138 patch.
+s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
 ---
-- null
+...
+s == nil
+---
+- true
 ...
 -- AF_INET
 s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
@@ -1595,7 +1605,7 @@ fio.stat(path) == nil
 { socket.tcp_connect('abrakadabra#123') == nil, errno.strerror() }
 ---
 - - true
-  - Invalid argument
+  - Input/output error
 ...
 -- wrong options for getaddrinfo
 socket.getaddrinfo('host', 'port', { type = 'WRONG' }) == nil and errno() == errno.EINVAL
@@ -2914,3 +2924,68 @@ srv:close()
 ---
 - true
 ...
+-- gh-4138 Check getaddrinfo() error from socket:connect() only.
+-- Error code and error message returned by getaddrinfo() depends
+-- on system's gai_strerror().
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+-- EAI_NONAME
+-- EAI_SERVICE
+-- EAI_NONAME
+-- EAI_NONAME
+-- EAI_AGAIN
+-- EAI_AGAIN
+function check_err(err)
+    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
+       err == 'getaddrinfo: Servname not supported for ai_socktype' or
+       err == 'getaddrinfo: Name or service not known' or
+       err == 'getaddrinfo: hostname nor servname provided, or not known' or
+       err == 'getaddrinfo: Temporary failure in name resolution' or
+       err == 'getaddrinfo: Name could not be resolved at this time' then
+        return true
+    end
+    return false
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+s, err = socket.getaddrinfo('non_exists_hostname', 3301)
+---
+...
+check_err(err)
+---
+- true
+...
+s, err = socket.connect('non_exists_hostname', 3301)
+---
+...
+check_err(err)
+---
+- true
+...
+s, err = socket.tcp_connect('non_exists_hostname', 3301)
+---
+...
+check_err(err)
+---
+- true
+...
+s, err = socket.bind('non_exists_hostname', 3301)
+---
+...
+check_err(err)
+---
+- true
+...
+s, err = socket.tcp_server('non_exists_hostname', 3301, function() end)
+---
+...
+check_err(err)
+---
+- true
+...
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index d1fe7ada6..9f5b3fea0 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -300,8 +300,16 @@ sc:close()
 
 -- tcp_connect
 
--- test timeout
-socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
+-- Test timeout. In this test, tcp_connect can return the second
+-- output value from internal.getaddrinfo (usually on Mac OS, but
+-- theoretically it can happen on Linux too). Sometimes
+-- getaddrinfo() is timed out, sometimes connect. On Linux however
+-- getaddrinfo is fast enough to never give timeout error in
+-- the case. So, there are two sources of timeout errors that are
+-- reported differently. This difference has appeared after
+-- gh-4138 patch.
+s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
+s == nil
 
 -- AF_INET
 s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
@@ -988,3 +996,36 @@ s:receive('*a')
 s:close()
 srv:close()
 
+-- gh-4138 Check getaddrinfo() error from socket:connect() only.
+-- Error code and error message returned by getaddrinfo() depends
+-- on system's gai_strerror().
+test_run:cmd("setopt delimiter ';'")
+function check_err(err)
+-- EAI_NONAME
+    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
+-- EAI_SERVICE
+       err == 'getaddrinfo: Servname not supported for ai_socktype' or
+-- EAI_NONAME
+       err == 'getaddrinfo: Name or service not known' or
+-- EAI_NONAME
+       err == 'getaddrinfo: hostname nor servname provided, or not known' or
+-- EAI_AGAIN
+       err == 'getaddrinfo: Temporary failure in name resolution' or
+-- EAI_AGAIN
+       err == 'getaddrinfo: Name could not be resolved at this time' then
+        return true
+    end
+    return false
+end;
+test_run:cmd("setopt delimiter ''");
+
+s, err = socket.getaddrinfo('non_exists_hostname', 3301)
+check_err(err)
+s, err = socket.connect('non_exists_hostname', 3301)
+check_err(err)
+s, err = socket.tcp_connect('non_exists_hostname', 3301)
+check_err(err)
+s, err = socket.bind('non_exists_hostname', 3301)
+check_err(err)
+s, err = socket.tcp_server('non_exists_hostname', 3301, function() end)
+check_err(err)
diff --git a/test/box/net.box.result b/test/box/net.box.result
index e3dabf7d9..c5b32123b 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3927,6 +3927,43 @@ test_run:grep_log('default', '00000040:.*')
 ---
 - null
 ...
+-- gh-4138 Check getaddrinfo() error from connect() only. Error
+-- code and error message returned by getaddrinfo() depends on
+-- system's gai_strerror().
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+-- EAI_NONAME
+-- EAI_SERVICE
+-- EAI_NONAME
+-- EAI_NONAME
+-- EAI_AGAIN
+-- EAI_AGAIN
+function check_err(err)
+    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
+       err == 'getaddrinfo: Servname not supported for ai_socktype' or
+       err == 'getaddrinfo: Name or service not known' or
+       err == 'getaddrinfo: hostname nor servname provided, or not known' or
+       err == 'getaddrinfo: Temporary failure in name resolution' or
+       err == 'getaddrinfo: Name could not be resolved at this time' then
+            return true
+    end
+    return false
+end;
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+s = remote.connect('non_exists_hostname:3301')
+---
+...
+check_err(s['error'])
+---
+- true
+...
 box.cfg{log_level=log_level}
 ---
 ...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..24b996be4 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1582,4 +1582,30 @@ test_run:wait_log('default', '00000030:.*', nil, 10)
 -- we expect nothing below, so don't wait
 test_run:grep_log('default', '00000040:.*')
 
+-- gh-4138 Check getaddrinfo() error from connect() only. Error
+-- code and error message returned by getaddrinfo() depends on
+-- system's gai_strerror().
+test_run:cmd("setopt delimiter ';'")
+function check_err(err)
+-- EAI_NONAME
+    if err == 'getaddrinfo: nodename nor servname provided, or not known' or
+-- EAI_SERVICE
+       err == 'getaddrinfo: Servname not supported for ai_socktype' or
+-- EAI_NONAME
+       err == 'getaddrinfo: Name or service not known' or
+-- EAI_NONAME
+       err == 'getaddrinfo: hostname nor servname provided, or not known' or
+-- EAI_AGAIN
+       err == 'getaddrinfo: Temporary failure in name resolution' or
+-- EAI_AGAIN
+       err == 'getaddrinfo: Name could not be resolved at this time' then
+            return true
+    end
+    return false
+end;
+test_run:cmd("setopt delimiter ''");
+
+s = remote.connect('non_exists_hostname:3301')
+check_err(s['error'])
+
 box.cfg{log_level=log_level}



More information about the Tarantool-patches mailing list