Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Sergey Ostanevich <sergos@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v2 1/2] coio/say: fix getaddrinfo error handling on macOS
Date: Sun, 29 Mar 2020 12:12:51 +0300	[thread overview]
Message-ID: <20200329091251.eihtejl6hrec4pon@tkn_work_nb> (raw)
In-Reply-To: <20200329085141.GC328@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.

  reply	other threads:[~2020-03-29  9:12 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-12 10:24 [Tarantool-patches] [PATCH v2 0/2] Return getaddrinfo() errors Roman Khabibov
2020-03-12 10:24 ` [Tarantool-patches] [PATCH v2 1/2] coio/say: fix getaddrinfo error handling on macOS Roman Khabibov
2020-03-29  8:51   ` Sergey Ostanevich
2020-03-29  9:12     ` Alexander Turenko [this message]
2020-04-06  2:07     ` Roman Khabibov
2020-04-17  9:37       ` Sergey Ostanevich
2020-04-24 11:32         ` Roman Khabibov
2020-06-23 13:27         ` Roman Khabibov
2020-07-23 14:12           ` Sergey Ostanevich
2020-07-28  9:26             ` Roman Khabibov
2020-04-26 17:20   ` Vladislav Shpilevoy
2020-03-12 10:24 ` [Tarantool-patches] [PATCH v2 2/2] lua: return getaddrinfo() errors Roman Khabibov
2020-03-29  9:07   ` Sergey Ostanevich
2020-03-29  9:25     ` Alexander Turenko
2020-04-06  2:08     ` Roman Khabibov
2020-04-16 10:27       ` Sergey Ostanevich
2020-04-24 11:42         ` Roman Khabibov
2020-04-24 17:18           ` Sergey Ostanevich
2020-04-26  3:16             ` Roman Khabibov
2020-04-26 15:46               ` Alexander Turenko
2020-04-26 16:26                 ` Roman Khabibov
2020-07-13  9:51                   ` sergos
2020-07-28 14:08 ` [Tarantool-patches] [PATCH v2 0/2] Return " Kirill Yukhin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200329091251.eihtejl6hrec4pon@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=sergos@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v2 1/2] coio/say: fix getaddrinfo error handling on macOS' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox