From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp63.i.mail.ru (smtp63.i.mail.ru [217.69.128.43])
(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 114A1446429
for <tarantool-patches@dev.tarantool.org>;
Sun, 29 Mar 2020 12:12:47 +0300 (MSK)
Date: Sun, 29 Mar 2020 12:12:51 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20200329091251.eihtejl6hrec4pon@tkn_work_nb>
References: <20200312102434.97300-1-roman.habibov@tarantool.org>
<20200312102434.97300-2-roman.habibov@tarantool.org>
<20200329085141.GC328@tarantool.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20200329085141.GC328@tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v2 1/2] coio/say: fix getaddrinfo
error handling on macOS
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>,
<mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
> > diff --git a/src/lib/core/say.c b/src/lib/core/say.c
> > index 64a637c58..8ad88ad57 100644
> > --- a/src/lib/core/say.c
> > +++ b/src/lib/core/say.c
> > @@ -459,14 +459,17 @@ static inline int
> > syslog_connect_unix(const char *path)
> > {
> > int fd = socket(PF_UNIX, SOCK_DGRAM, 0);
> > - if (fd < 0)
> > + if (fd < 0) {
> > + diag_set(SystemError, "socket");
> This error message gives nothing. Please, describe the error behind it
> using the strerror(errno)
| void
| SystemError::log() const
| {
| say_file_line(S_SYSERROR, file, line, strerror(saved_errno),
| "SystemError %s", errmsg);
| }
https://github.com/tarantool/tarantool/blob/eaa860885dc5708956094845e20f5bc81275ca26/src/lib/core/exception.cc#L153-L158
> > diff --git a/test/unit/coio.cc b/test/unit/coio.cc
> > index bb8bd7131..957c58ede 100644
> > --- a/test/unit/coio.cc
> > +++ b/test/unit/coio.cc
> > @@ -72,7 +72,7 @@ static void
> > test_getaddrinfo(void)
> > {
> > header();
> > - plan(1);
> > + plan(3);
> > const char *host = "127.0.0.1";
> > const char *port = "3333";
> > struct addrinfo *i;
> > @@ -81,6 +81,33 @@ test_getaddrinfo(void)
> > is(rc, 0, "getaddrinfo");
> > freeaddrinfo(i);
> >
> > + /*
> > + * gh-4138: Check getaddrinfo() retval and diagnostics
> > + * area.
> > + */
> > + rc = coio_getaddrinfo("non_exists_hostname", port, NULL, &i,
> > + 15768000000);
> > + isnt(rc, 0, "getaddrinfo retval");
> > + const char *errmsg = diag_get()->last->errmsg;
> > + const char *exp_errmsg_1 = "getaddrinfo: nodename nor servname provided"
> > + ", or not known";
> > + const char *exp_errmsg_2 = "getaddrinfo: Servname not supported for "
> > + "ai_socktype";
> > + const char *exp_errmsg_3 = "getaddrinfo: Name or service not known";
> > + const char *exp_errmsg_4 = "getaddrinfo: hostname nor servname provided"
> > + ", or not known";
> > + const char *exp_errmsg_5 = "getaddrinfo: Temporary failure in name "
> > + "resolution";
> > + const char *exp_errmsg_6 = "getaddrinfo: Name could not be resolved at "
> > + "this time";
> > + bool is_match_with_exp = strcmp(errmsg, exp_errmsg_1) == 0 ||
> > + strcmp(errmsg, exp_errmsg_2) == 0 ||
> > + strcmp(errmsg, exp_errmsg_3) == 0 ||
> > + strcmp(errmsg, exp_errmsg_4) == 0 ||
> > + strcmp(errmsg, exp_errmsg_5) == 0 ||
> > + strcmp(errmsg, exp_errmsg_6) == 0;
> > + is(is_match_with_exp, true, "getaddrinfo error message");
> > +
> Why did you made such a test - you're not sure which one will be
> triggered? Can you create a test that will check all possible errors?
I asked to check for certain errors rather than that some SystemError
with '^getaddrinfo: .*$' message raised. Sure, the return value of
getaddrinfo() depends on network conditions (I guess it depends of a DNS
server: whether it reports temporary or persistent error; but also
whether a network interface is up at all). gai_strerror() output
depends of OS.
I asked to comment messages with EAI_* constants that corresponds to
them, but it seems it was missed.
Sure, proper description is necessary here.
Nope, we unable to control network conditions and OS here and cannot
create a test case for each of those situations.