From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp16.mail.ru (smtp16.mail.ru [94.100.176.153]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 875494696C4 for ; Mon, 6 Apr 2020 05:08:04 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <20200329090718.GD328@tarantool.org> Date: Mon, 6 Apr 2020 05:08:02 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <20200312102434.97300-1-roman.habibov@tarantool.org> <20200312102434.97300-3-roman.habibov@tarantool.org> <20200329090718.GD328@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] lua: return getaddrinfo() errors List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sergey Ostanevich Cc: tarantool-patches@dev.tarantool.org > On Mar 29, 2020, at 12:07, Sergey Ostanevich = wrote: >=20 > Hi! >=20 > Thanks for the patch, please see below. >=20 > Sergos >=20 > On 12 =D0=BC=D0=B0=D1=80 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. >>=20 >> Closes #4138 >>=20 >> @TarantoolBot document >> Title: socket API changes >>=20 >> * socket.getaddrinfo() >>=20 >> 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. >>=20 >> Example: >> tarantool> socket.getaddrinfo('non_exists_hostname', 3301) >> --- >> - null >> - 'getaddrinfo: nodename nor servname provided, or not known' >> ... >>=20 >> * socket.tcp_connect() >>=20 >> Can return socket.getaddrinfo() error message as second return >> value. >>=20 >> 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(-) >>=20 >> 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 =3D timeout or DEFAULT_CONNECT_TIMEOUT >> local begin =3D fiber.clock() >> - local s =3D socket.tcp_connect(host, port, timeout) >> + local s, err =3D 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 =3D s:read({chunk =3D 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; >> } >>=20 >> +/** >> + * 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) >>=20 >> if (dns_res !=3D 0) { >> lua_pushnil(L); >> - return 1; >> + struct error *err =3D diag_get()->last; >> + lua_pushstring(L, err->errmsg); >> + return 2; >> } >>=20 >> /* 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 =3D timeout or TIMEOUT_INFINITY >> local stop =3D fiber.clock() + timeout >> - local dns =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', >> + local dns, err =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', >> protocol =3D 'tcp' }) >> - if dns =3D=3D nil or #dns =3D=3D 0 then >> + if dns =3D=3D nil then >> + return nil, err > You explicitly put error here... >=20 >> + end >> + if #dns =3D=3D 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 =3D 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 =3D s._errno s:close() boxerrno(save_errno) - return nil + return nil, boxerrno.strerror() end boxerrno(0) return s end local timeout =3D timeout or TIMEOUT_INFINITY local stop =3D fiber.clock() + timeout - local dns =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', + local dns, err =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', protocol =3D 'tcp' }) - if dns =3D=3D nil or #dns =3D=3D 0 then + if dns =3D=3D nil then + return nil, err + end + if #dns =3D=3D 0 then boxerrno(boxerrno.EINVAL) - return nil + return nil, boxerrno.strerror() end for i, remote in pairs(dns) do timeout =3D stop - fiber.clock() if timeout <=3D 0 then boxerrno(boxerrno.ETIMEDOUT) - return nil + return nil, boxerrno.strerror() end local s =3D 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 =3D { family =3D 'AF_INET', type =3D 'SOCK_STREAM' = } >> local timeout =3D deadline - fiber.clock() >> - local dns =3D getaddrinfo(host, port, timeout, ga_opts) >> - if dns =3D=3D nil or #dns =3D=3D 0 then >> + local dns, err =3D getaddrinfo(host, port, timeout, ga_opts) >> + if dns =3D=3D nil then >> + self._errno =3D boxerrno.EINVAL >> + return nil, err >> + end >> + if #dns =3D=3D 0 then >> self._errno =3D boxerrno.EINVAL >> return nil, socket_error(self) >> end >> @@ -1547,9 +1554,12 @@ local function lsocket_connect(host, port) >> if host =3D=3D nil or port =3D=3D nil then >> error("Usage: luasocket.connect(host, port)") >> end >> - local s =3D tcp_connect(host, port) >> + local s, err =3D 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 =3D socket.tcp_connect('127.0.0.1', 80, 0.00000000001) >> --- >> -- null >> +... >> +s =3D=3D nil >> +--- >> +- true >> ... >> -- AF_INET >> s =3D socket('AF_INET', 'SOCK_STREAM', 'tcp') >> @@ -1595,7 +1605,7 @@ fio.stat(path) =3D=3D nil >> { socket.tcp_connect('abrakadabra#123') =3D=3D nil, errno.strerror() = } >> --- >> - - true >> - - Invalid argument >> + - Input/output error >> ... >> -- wrong options for getaddrinfo >> socket.getaddrinfo('host', 'port', { type =3D 'WRONG' }) =3D=3D nil = and errno() =3D=3D 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Servname not supported for = ai_socktype' or >> + err =3D=3D 'getaddrinfo: Name or service not known' or >> + err =3D=3D 'getaddrinfo: hostname nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Temporary failure in name = resolution' or >> + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then >> + return true >> + end >> + return false >> +end; >> +--- >> +... >> +test_run:cmd("setopt delimiter ''"); >> +--- >> +- true >> +... >> +s, err =3D socket.getaddrinfo('non_exists_hostname', 3301) >> +--- >> +... >> +check_err(err) >> +--- >> +- true >> +... >> +s, err =3D socket.connect('non_exists_hostname', 3301) >> +--- >> +... >> +check_err(err) >> +--- >> +- true >> +... >> +s, err =3D 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() >>=20 >> -- tcp_connect >>=20 >> --- 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 =3D socket.tcp_connect('127.0.0.1', 80, 0.00000000001) >> +s =3D=3D nil >>=20 >> -- AF_INET >> s =3D socket('AF_INET', 'SOCK_STREAM', 'tcp') >> @@ -988,3 +996,26 @@ s:receive('*a') >> s:close() >> srv:close() >>=20 >> +-- 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Servname not supported for = ai_socktype' or >> + err =3D=3D 'getaddrinfo: Name or service not known' or >> + err =3D=3D 'getaddrinfo: hostname nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Temporary failure in name = resolution' or >> + err =3D=3D '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=20 > for the same error on different platforms. Could you please check for = particular=20 > 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=E2=80=99t >> +test_run:cmd("setopt delimiter ''"); >> + >> +s, err =3D socket.getaddrinfo('non_exists_hostname', 3301) >> +check_err(err) >> +s, err =3D socket.connect('non_exists_hostname', 3301) >> +check_err(err) >> +s, err =3D 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Servname not supported for = ai_socktype' or >> + err =3D=3D 'getaddrinfo: Name or service not known' or >> + err =3D=3D 'getaddrinfo: hostname nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Temporary failure in name = resolution' or >> + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then >> + return true >> + end >> + return false >> +end; >> +--- >> +... >> +test_run:cmd("setopt delimiter ''"); >> +--- >> +- true >> +... >> +s =3D remote.connect('non_exists_hostname:3301') >> +--- >> +... >> +check_err(s['error']) >> +--- >> +- true >> +... >> box.cfg{log_level=3Dlog_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:.*') >>=20 >> +-- 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Servname not supported for = ai_socktype' or >> + err =3D=3D 'getaddrinfo: Name or service not known' or >> + err =3D=3D 'getaddrinfo: hostname nor servname provided, or = not known' or >> + err =3D=3D 'getaddrinfo: Temporary failure in name = resolution' or >> + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then >> + return true >> + end >> + return false >> +end; >> +test_run:cmd("setopt delimiter ''"); >> + >> +s =3D remote.connect('non_exists_hostname:3301') >> +check_err(s['error']) >> + >> box.cfg{log_level=3Dlog_level} >> --=20 >> 2.21.0 (Apple Git-122) >>=20 commit 57d8b556b98a8b6de4d40f6ae1c5f48498022962 Author: Roman Khabibov Date: Thu Nov 21 14:37:54 2019 +0300 lua: return getaddrinfo() errors =20 Add getaddrinfo() errors into the several fuctions of socket. Now getaddrinfo() return a pair of values (nil and error message) in case of error. =20 Closes #4138 =20 @TarantoolBot document Title: socket API changes =20 * socket.getaddrinfo() =20 Return error message as the second return value. =20 Example: tarantool> socket.getaddrinfo('non_exists_hostname', 3301) --- - null - 'getaddrinfo: nodename nor servname provided, or not known' ... =20 * socket.tcp_connect() =20 Return error message as the second return value. =20 Example: tarantool> socket.tcp_connect('non_exists_hostname', 3301) --- - null - 'getaddrinfo: nodename nor servname provided, or not known' ... =20 * socket.bind() =20 Return error message as the second return value. =20 Example: tarantool> socket.tcp_connect('non_exists_hostname', 3301) --- - null - 'getaddrinfo: nodename nor servname provided, or not known' ... =20 * socket.tcp_server() =20 Return error message as the second return value. =20 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 =3D timeout or DEFAULT_CONNECT_TIMEOUT local begin =3D fiber.clock() - local s =3D socket.tcp_connect(host, port, timeout) + local s, err =3D socket.tcp_connect(host, port, timeout) if not s then - return nil, errno.strerror(errno()) + return nil, err end local msg =3D s:read({chunk =3D 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; } =20 +/** + * 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) =20 if (dns_res !=3D 0) { lua_pushnil(L); - return 1; + struct error *err =3D diag_get()->last; + lua_pushstring(L, err->errmsg); + return 2; } =20 /* 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 =3D get_ivalue(internal.SO_TYPE, opts.type) if itype =3D=3D nil then boxerrno(boxerrno.EINVAL) - return nil + return nil, boxerrno.strerror() end ga_opts.type =3D itype end @@ -972,7 +972,7 @@ local function getaddrinfo(host, port, timeout, = opts) local ifamily =3D get_ivalue(internal.DOMAIN, opts.family) if ifamily =3D=3D nil then boxerrno(boxerrno.EINVAL) - return nil + return nil, boxerrno.strerror() end ga_opts.family =3D ifamily end @@ -980,7 +980,7 @@ local function getaddrinfo(host, port, timeout, = opts) if opts.protocol ~=3D nil then local p =3D getprotobyname(opts.protocol) if p =3D=3D nil then - return nil + return nil, boxerrno.strerror() end ga_opts.protocol =3D p end @@ -990,7 +990,7 @@ local function getaddrinfo(host, port, timeout, = opts) get_iflags(internal.AI_FLAGS, opts.flags) if ga_opts.flags =3D=3D nil then boxerrno(boxerrno.EINVAL) - return nil + return nil, boxerrno.strerror() end end =20 @@ -1028,30 +1028,33 @@ local function tcp_connect(host, port, timeout) local s =3D 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 =3D s._errno s:close() boxerrno(save_errno) - return nil + return nil, boxerrno.strerror() end boxerrno(0) return s end local timeout =3D timeout or TIMEOUT_INFINITY local stop =3D fiber.clock() + timeout - local dns =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', + local dns, err =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', protocol =3D 'tcp' }) - if dns =3D=3D nil or #dns =3D=3D 0 then + if dns =3D=3D nil then + return nil, err + end + if #dns =3D=3D 0 then boxerrno(boxerrno.EINVAL) - return nil + return nil, boxerrno.strerror() end for i, remote in pairs(dns) do timeout =3D stop - fiber.clock() if timeout <=3D 0 then boxerrno(boxerrno.ETIMEDOUT) - return nil + return nil, boxerrno.strerror() end local s =3D 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 =20 local function tcp_server_handler(server, sc, from) @@ -1160,15 +1163,15 @@ end =20 local function tcp_server_bind(host, port, prepare, timeout) timeout =3D timeout and tonumber(timeout) or TIMEOUT_INFINITY - local dns + local dns, err if host =3D=3D 'unix/' then dns =3D {{host =3D host, port =3D port, family =3D 'AF_UNIX', = protocol =3D 0, type =3D 'SOCK_STREAM' }} else - dns =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', + dns, err =3D getaddrinfo(host, port, timeout, { type =3D = 'SOCK_STREAM', flags =3D 'AI_PASSIVE'}) if dns =3D=3D nil then - return nil + return nil, err end end =20 @@ -1185,14 +1188,14 @@ local function tcp_server_bind(host, port, = prepare, timeout) local save_errno =3D 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 =20 local function tcp_server(host, port, opts, timeout) @@ -1213,7 +1216,8 @@ local function tcp_server(host, port, opts, = timeout) server.name =3D server.name or 'server' local s, addr =3D 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 =3D { family =3D 'AF_INET', type =3D 'SOCK_STREAM' } local timeout =3D deadline - fiber.clock() - local dns =3D getaddrinfo(host, port, timeout, ga_opts) - if dns =3D=3D nil or #dns =3D=3D 0 then + local dns, err =3D getaddrinfo(host, port, timeout, ga_opts) + if dns =3D=3D nil then + self._errno =3D boxerrno.EINVAL + return nil, err + end + if #dns =3D=3D 0 then self._errno =3D boxerrno.EINVAL return nil, socket_error(self) end @@ -1547,9 +1555,9 @@ local function lsocket_connect(host, port) if host =3D=3D nil or port =3D=3D nil then error("Usage: luasocket.connect(host, port)") end - local s =3D tcp_connect(host, port) + local s, err =3D 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 =3D tcp_server_bind(host, port, prepare) + local s, err =3D 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 =3D socket.tcp_connect('127.0.0.1', 80, 0.00000000001) --- -- null +... +s =3D=3D nil +--- +- true ... -- AF_INET s =3D socket('AF_INET', 'SOCK_STREAM', 'tcp') @@ -1595,7 +1605,7 @@ fio.stat(path) =3D=3D nil { socket.tcp_connect('abrakadabra#123') =3D=3D nil, errno.strerror() } --- - - true - - Invalid argument + - Input/output error ... -- wrong options for getaddrinfo socket.getaddrinfo('host', 'port', { type =3D 'WRONG' }) =3D=3D nil and = errno() =3D=3D 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or not = known' or + err =3D=3D 'getaddrinfo: Servname not supported for ai_socktype' = or + err =3D=3D 'getaddrinfo: Name or service not known' or + err =3D=3D 'getaddrinfo: hostname nor servname provided, or not = known' or + err =3D=3D 'getaddrinfo: Temporary failure in name resolution' = or + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then + return true + end + return false +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +s, err =3D socket.getaddrinfo('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err =3D socket.connect('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err =3D socket.tcp_connect('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err =3D socket.bind('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err =3D 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() =20 -- tcp_connect =20 --- 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 =3D socket.tcp_connect('127.0.0.1', 80, 0.00000000001) +s =3D=3D nil =20 -- AF_INET s =3D socket('AF_INET', 'SOCK_STREAM', 'tcp') @@ -988,3 +996,36 @@ s:receive('*a') s:close() srv:close() =20 +-- 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or not = known' or +-- EAI_SERVICE + err =3D=3D 'getaddrinfo: Servname not supported for ai_socktype' = or +-- EAI_NONAME + err =3D=3D 'getaddrinfo: Name or service not known' or +-- EAI_NONAME + err =3D=3D 'getaddrinfo: hostname nor servname provided, or not = known' or +-- EAI_AGAIN + err =3D=3D 'getaddrinfo: Temporary failure in name resolution' = or +-- EAI_AGAIN + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then + return true + end + return false +end; +test_run:cmd("setopt delimiter ''"); + +s, err =3D socket.getaddrinfo('non_exists_hostname', 3301) +check_err(err) +s, err =3D socket.connect('non_exists_hostname', 3301) +check_err(err) +s, err =3D socket.tcp_connect('non_exists_hostname', 3301) +check_err(err) +s, err =3D socket.bind('non_exists_hostname', 3301) +check_err(err) +s, err =3D 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or not = known' or + err =3D=3D 'getaddrinfo: Servname not supported for ai_socktype' = or + err =3D=3D 'getaddrinfo: Name or service not known' or + err =3D=3D 'getaddrinfo: hostname nor servname provided, or not = known' or + err =3D=3D 'getaddrinfo: Temporary failure in name resolution' = or + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then + return true + end + return false +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +s =3D remote.connect('non_exists_hostname:3301') +--- +... +check_err(s['error']) +--- +- true +... box.cfg{log_level=3Dlog_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:.*') =20 +-- 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 =3D=3D 'getaddrinfo: nodename nor servname provided, or not = known' or +-- EAI_SERVICE + err =3D=3D 'getaddrinfo: Servname not supported for ai_socktype' = or +-- EAI_NONAME + err =3D=3D 'getaddrinfo: Name or service not known' or +-- EAI_NONAME + err =3D=3D 'getaddrinfo: hostname nor servname provided, or not = known' or +-- EAI_AGAIN + err =3D=3D 'getaddrinfo: Temporary failure in name resolution' = or +-- EAI_AGAIN + err =3D=3D 'getaddrinfo: Name could not be resolved at this = time' then + return true + end + return false +end; +test_run:cmd("setopt delimiter ''"); + +s =3D remote.connect('non_exists_hostname:3301') +check_err(s['error']) + box.cfg{log_level=3Dlog_level}