Commit 52a10224 authored by Tom Lane's avatar Tom Lane

Uniformly identify the target host in libpq connection failure reports.

Prefix "could not connect to host-or-socket-path:" to all connection
failure cases that occur after the socket() call, and remove the
ad-hoc server identity data that was appended to a few of these
messages.  This should produce much more intelligible error reports
in multiple-target-host situations, especially for error cases that
are off the beaten track to any degree (because none of those provided
any server identity info).

As an example of the change, formerly a connection attempt with a bad
port number such as "psql -p 12345 -h localhost,/tmp" might produce

psql: error: could not connect to server: Connection refused
        Is the server running on host "localhost" (::1) and accepting
        TCP/IP connections on port 12345?
could not connect to server: Connection refused
        Is the server running on host "localhost" (127.0.0.1) and accepting
        TCP/IP connections on port 12345?
could not connect to server: No such file or directory
        Is the server running locally and accepting
        connections on Unix domain socket "/tmp/.s.PGSQL.12345"?

Now it looks like

psql: error: could not connect to host "localhost" (::1), port 12345: Connection refused
        Is the server running on that host and accepting TCP/IP connections?
could not connect to host "localhost" (127.0.0.1), port 12345: Connection refused
        Is the server running on that host and accepting TCP/IP connections?
could not connect to socket "/tmp/.s.PGSQL.12345": No such file or directory
        Is the server running locally and accepting connections on that socket?

This requires adjusting a couple of regression tests to allow for
variation in the contents of a connection failure message.

Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
parent 800d93f3
......@@ -3460,7 +3460,7 @@ foreach my $db (sort keys %create_sql)
command_fails_like(
[ 'pg_dump', '-p', "$port", 'qqq' ],
qr/\Qpg_dump: error: connection to database "qqq" failed: FATAL: database "qqq" does not exist\E/,
qr/pg_dump: error: connection to database "qqq" failed: could not connect to .*: FATAL: database "qqq" does not exist/,
'connecting to a non-existent database');
#########################################
......
......@@ -36,7 +36,7 @@
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ECPGconnect: opening database <DEFAULT> on <DEFAULT> port <DEFAULT> for user regress_ecpg_user2
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ECPGconnect: could not open database: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: ECPGconnect: could not open database: could not connect: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection main closed
......@@ -73,7 +73,7 @@
[NO_PID]: sqlca: code: -220, state: 08003
[NO_PID]: ECPGconnect: opening database <DEFAULT> on <DEFAULT> port <DEFAULT> for user regress_ecpg_user2
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ECPGconnect: could not open database: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: ECPGconnect: could not open database: could not connect: FATAL: database "regress_ecpg_user2" does not exist
[NO_PID]: sqlca: code: 0, state: 00000
[NO_PID]: ecpg_finish: connection main closed
......
......@@ -1669,15 +1669,17 @@ getHostaddr(PGconn *conn, char *host_addr, int host_addr_len)
}
/* ----------
* connectFailureMessage -
* create a friendly error message on connection failure.
* emitCouldNotConnect -
* Speculatively append "could not connect to ...: " to conn->errorMessage
* once we've identified the current connection target address. This ensures
* that any subsequent error message will be properly attributed to the
* server we couldn't connect to. conn->raddr must be valid, and the result
* of getHostaddr() must be supplied.
* ----------
*/
static void
connectFailureMessage(PGconn *conn, int errorno)
emitCouldNotConnect(PGconn *conn, const char *host_addr)
{
char sebuf[PG_STRERROR_R_BUFLEN];
#ifdef HAVE_UNIX_SOCKETS
if (IS_AF_UNIX(conn->raddr.addr.ss_family))
{
......@@ -1688,25 +1690,15 @@ connectFailureMessage(PGconn *conn, int errorno)
service, sizeof(service),
NI_NUMERICSERV);
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running locally and accepting\n"
"\tconnections on Unix domain socket \"%s\"?\n"),
SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)),
libpq_gettext("could not connect to socket \"%s\": "),
service);
}
else
#endif /* HAVE_UNIX_SOCKETS */
{
char host_addr[NI_MAXHOST];
const char *displayed_host;
const char *displayed_port;
/*
* Optionally display the network address with the hostname. This is
* useful to distinguish between IPv4 and IPv6 connections.
*/
getHostaddr(conn, host_addr, NI_MAXHOST);
/* To which host and port were we actually connecting? */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
displayed_host = conn->connhost[conn->whichhost].hostaddr;
......@@ -1722,26 +1714,46 @@ connectFailureMessage(PGconn *conn, int errorno)
* looked-up IP address.
*/
if (conn->connhost[conn->whichhost].type != CHT_HOST_ADDRESS &&
strlen(host_addr) > 0 &&
host_addr[0] &&
strcmp(displayed_host, host_addr) != 0)
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running on host \"%s\" (%s) and accepting\n"
"\tTCP/IP connections on port %s?\n"),
SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)),
libpq_gettext("could not connect to host \"%s\" (%s), port %s: "),
displayed_host, host_addr,
displayed_port);
else
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not connect to server: %s\n"
"\tIs the server running on host \"%s\" and accepting\n"
"\tTCP/IP connections on port %s?\n"),
SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)),
libpq_gettext("could not connect to host \"%s\", port %s: "),
displayed_host,
displayed_port);
}
}
/* ----------
* connectFailureMessage -
* create a friendly error message on connection failure,
* using the given errno value. Use this for error cases that
* imply that there's no server there.
* ----------
*/
static void
connectFailureMessage(PGconn *conn, int errorno)
{
char sebuf[PG_STRERROR_R_BUFLEN];
appendPQExpBuffer(&conn->errorMessage,
"%s\n",
SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)));
#ifdef HAVE_UNIX_SOCKETS
if (IS_AF_UNIX(conn->raddr.addr.ss_family))
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("\tIs the server running locally and accepting connections on that socket?\n"));
else
#endif
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("\tIs the server running on that host and accepting TCP/IP connections?\n"));
}
/*
* Should we use keepalives? Returns 1 if yes, 0 if no, and -1 if
* conn->keepalives is set to a value which is not parseable as an
......@@ -2476,30 +2488,30 @@ keep_going: /* We will come back to here until there is
goto keep_going;
}
/* Remember current address for possible error msg */
/* Remember current address for possible use later */
memcpy(&conn->raddr.addr, addr_cur->ai_addr,
addr_cur->ai_addrlen);
conn->raddr.salen = addr_cur->ai_addrlen;
/* set connip */
/*
* Set connip, too. Note we purposely ignore strdup
* failure; not a big problem if it fails.
*/
if (conn->connip != NULL)
{
free(conn->connip);
conn->connip = NULL;
}
getHostaddr(conn, host_addr, NI_MAXHOST);
if (strlen(host_addr) > 0)
if (host_addr[0])
conn->connip = strdup(host_addr);
/*
* purposely ignore strdup failure; not a big problem if
* it fails anyway.
*/
/* Try to create the socket */
conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0);
if (conn->sock == PGINVALID_SOCKET)
{
int errorno = SOCK_ERRNO;
/*
* Silently ignore socket() failure if we have more
* addresses to try; this reduces useless chatter in
......@@ -2512,12 +2524,20 @@ keep_going: /* We will come back to here until there is
conn->try_next_addr = true;
goto keep_going;
}
emitCouldNotConnect(conn, host_addr);
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not create socket: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)));
goto error_return;
}
/*
* Once we've identified a target address, all errors
* except the preceding socket()-failure case should be
* prefixed with "could not connect to <target>: ".
*/
emitCouldNotConnect(conn, host_addr);
/*
* Select socket options: no delay of outgoing data for
* TCP sockets, nonblock mode, close-on-exec. Try the
......@@ -3608,9 +3628,6 @@ keep_going: /* We will come back to here until there is
}
case CONNECTION_CHECK_WRITABLE:
{
const char *displayed_host;
const char *displayed_port;
conn->status = CONNECTION_OK;
if (!PQconsumeInput(conn))
goto error_return;
......@@ -3634,19 +3651,8 @@ keep_going: /* We will come back to here until there is
PQclear(res);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
displayed_host = conn->connhost[conn->whichhost].hostaddr;
else
displayed_host = conn->connhost[conn->whichhost].host;
displayed_port = conn->connhost[conn->whichhost].port;
if (displayed_port == NULL || displayed_port[0] == '\0')
displayed_port = DEF_PGPORT_STR;
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("could not make a writable "
"connection to server "
"\"%s:%s\"\n"),
displayed_host, displayed_port);
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("session is read-only\n"));
/* Close connection politely. */
conn->status = CONNECTION_OK;
......@@ -3679,17 +3685,8 @@ keep_going: /* We will come back to here until there is
PQclear(res);
/* Append error report to conn->errorMessage. */
if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS)
displayed_host = conn->connhost[conn->whichhost].hostaddr;
else
displayed_host = conn->connhost[conn->whichhost].host;
displayed_port = conn->connhost[conn->whichhost].port;
if (displayed_port == NULL || displayed_port[0] == '\0')
displayed_port = DEF_PGPORT_STR;
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("test \"SHOW transaction_read_only\" failed "
"on server \"%s:%s\"\n"),
displayed_host, displayed_port);
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("test \"SHOW transaction_read_only\" failed\n"));
/* Close connection politely. */
conn->status = CONNECTION_OK;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment