redundant error messages
A few client tools duplicate error messages already provided by libpq,
such as
pg_rewind: fatal: could not connect to server: could not connect to
server: No such file or directory
pg_basebackup: error: could not connect to server: could not connect to
server: No such file or directory
psql: error: could not connect to server: could not connect to server:
No such file or directory
The psql case is actually a regression introduced in PG12, but the other
two appear to be ancient.
Other client tools provide a different error message so in aggregate it
looks like this:
createdb: error: could not connect to database template1: could not
connect to server: No such file or directory
The attached patch removes the redundant message from the client tools.
I suppose it's a bit dubious because there is no guarantee what the
level of detail the message supplied by libpq has. But I think these
few cases are not particularly hard to keep in sync.
Attachments:
0001-Fix-redundant-error-messages-in-client-tools.patchtext/plain; charset=UTF-8; name=0001-Fix-redundant-error-messages-in-client-tools.patch; x-mac-creator=0; x-mac-type=0Download
From a2c1e26ceff05ec6ebbdb9cf74a3171ebf969911 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 5 Nov 2020 13:24:47 +0100
Subject: [PATCH] Fix redundant error messages in client tools
A few client tools duplicate error messages already provided by libpq.
---
src/bin/pg_basebackup/streamutil.c | 5 ++---
src/bin/pg_rewind/pg_rewind.c | 3 +--
src/bin/psql/startup.c | 2 +-
3 files changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c
index be653ebb2d..5c536eb5fd 100644
--- a/src/bin/pg_basebackup/streamutil.c
+++ b/src/bin/pg_basebackup/streamutil.c
@@ -183,7 +183,7 @@ GetConnection(void)
*/
if (!tmpconn)
{
- pg_log_error("could not connect to server");
+ pg_log_error("out of memory");
exit(1);
}
@@ -200,8 +200,7 @@ GetConnection(void)
if (PQstatus(tmpconn) != CONNECTION_OK)
{
- pg_log_error("could not connect to server: %s",
- PQerrorMessage(tmpconn));
+ pg_log_error("%s", PQerrorMessage(tmpconn));
PQfinish(tmpconn);
free(values);
free(keywords);
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 421a45ef5b..52e3fc40e8 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -282,8 +282,7 @@ main(int argc, char **argv)
conn = PQconnectdb(connstr_source);
if (PQstatus(conn) == CONNECTION_BAD)
- pg_fatal("could not connect to server: %s",
- PQerrorMessage(conn));
+ pg_fatal("%s", PQerrorMessage(conn));
if (showprogress)
pg_log_info("connected to server");
diff --git a/src/bin/psql/startup.c b/src/bin/psql/startup.c
index e8d35a108f..586fcb3366 100644
--- a/src/bin/psql/startup.c
+++ b/src/bin/psql/startup.c
@@ -295,7 +295,7 @@ main(int argc, char *argv[])
if (PQstatus(pset.db) == CONNECTION_BAD)
{
- pg_log_error("could not connect to server: %s", PQerrorMessage(pset.db));
+ pg_log_error("%s", PQerrorMessage(pset.db));
PQfinish(pset.db);
exit(EXIT_BADCONN);
}
--
2.28.0
On Thu, 5 Nov 2020 at 09:27, Peter Eisentraut <
peter.eisentraut@enterprisedb.com> wrote:
A few client tools duplicate error messages already provided by libpq,
such aspg_rewind: fatal: could not connect to server: could not connect to
server: No such file or directoryGood catch!
Other client tools provide a different error message so in aggregate it
looks like this:createdb: error: could not connect to database template1: could not
connect to server: No such file or directoryIs the database name important for this message? You should inform which
database you want to connect for all client tools except pg_dumpall. Hence,
you
already know which database has the connection problem. IMO the pg_dumpall
message should inform the database name. My suggestion is:
if (fail_on_error)
{
pg_log_error("database \"%s\": %s",
dbname, PQerrorMessage(conn));
exit_nicely(1);
}
and remove the redundant 'could not connect to database %s' from
scripts/common.c.
--
Euler Taveira http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, 5 Nov 2020 at 08:34, Euler Taveira <euler.taveira@2ndquadrant.com>
wrote:
Is the database name important for this message? You should inform which
database you want to connect for all client tools except pg_dumpall.
Hence, you
already know which database has the connection problem. IMO the pg_dumpall
message should inform the database name. My suggestion is:
In principle, the client knows the database name. In practice, if it's
coming from PGDATABASE or via a service configuration, one may be confused
about the database; having the error message be explicit will avoid many
problems. I can easily imagine that "unable to connect to database" would
be mystifying, whereas "unable to connect to database foo" would elicit the
response, "wait, I'm trying to connect to what now?" leading much more
quickly to a resolution.
On 2020-Nov-05, Isaac Morland wrote:
In principle, the client knows the database name. In practice, if it's
coming from PGDATABASE or via a service configuration, one may be confused
about the database; having the error message be explicit will avoid many
problems. I can easily imagine that "unable to connect to database" would
be mystifying, whereas "unable to connect to database foo" would elicit the
response, "wait, I'm trying to connect to what now?" leading much more
quickly to a resolution.
Also consider cases like running something via cron, where the person
reading the error output does not necessarily know what command is being
run: it might be hidden inside a script. It's often very helpful to
have object names in error messages, even if for the normal usage it
seems that the object being operated on is very obvious by just looking
at the command.
On 2020-11-05 13:27, Peter Eisentraut wrote:
A few client tools duplicate error messages already provided by libpq,
such aspg_rewind: fatal: could not connect to server: could not connect to
server: No such file or directorypg_basebackup: error: could not connect to server: could not connect to
server: No such file or directorypsql: error: could not connect to server: could not connect to server:
No such file or directoryThe psql case is actually a regression introduced in PG12, but the other
two appear to be ancient.
I have committed fixes for these.