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

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 11dc98ee0a5ee63f93fa6fdc2f9419c598cb6e4b..2b501166b80f5ffad42de10e4af1d9daf31ee1d7 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -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');
 
 #########################################

diff --git a/src/interfaces/ecpg/test/expected/connect-test5.stderr b/src/interfaces/ecpg/test/expected/connect-test5.stderr
index a54df175fbf0db018cfb7a76ec13e0aeb1a69e23..4dbf2c0fc4626c9cb20343092fe8beecfc019dcd 100644 (file)
--- a/src/interfaces/ecpg/test/expected/connect-test5.stderr
+++ b/src/interfaces/ecpg/test/expected/connect-test5.stderr
@@ -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

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index 61a1fa6a14aad5e4317c4579a5d94e0ccc6fbe8c..a4a8b3ad7a54f1b3aee106087d598ae0efba0dfc 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -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;