database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-06-29 10:41:53 +03:00
Code Activity

In libpq, always append new error messages to conn->errorMessage.

Browse Source 
Previously, we had an undisciplined mish-mash of printfPQExpBuffer and
appendPQExpBuffer calls to report errors within libpq.  This commit
establishes a uniform rule that appendPQExpBuffer[Str] should be used.
conn->errorMessage is reset only at the start of an application request,
and then accumulates messages till we're done.  We can remove no less
than three different ad-hoc mechanisms that were used to get the effect
of concatenation of error messages within a sequence of operations.

Although this makes things quite a bit cleaner conceptually, the main
reason to do it is to make the world safer for the multiple-target-host
feature that was added awhile back.  Previously, there were many cases
in which an error occurring during an individual host connection attempt
would wipe out the record of what had happened during previous attempts.
(The reporting is still inadequate, in that it can be hard to tell which
host got the failure, but that seems like a matter for a separate commit.)

Currently, lo_import and lo_export contain exceptions to the "never
use printfPQExpBuffer" rule.  If we changed them, we'd risk reporting
an incidental lo_close failure before the actual read or write
failure, which would be confusing, not least because lo_close happened
after the main failure.  We could improve this by inventing an
internal version of lo_close that doesn't reset the errorMessage; but
we'd also need a version of PQfn() that does that, and it didn't quite
seem worth the trouble for now.

Discussion: https://postgr.es/m/BN6PR05MB3492948E4FD76C156E747E8BC9160@BN6PR05MB3492.namprd05.prod.outlook.com
This commit is contained in:
Tom Lane
2021-01-11 13:12:09 -05:00
parent ce6a71fa53
commit ffa2e46701
14 changed files with 699 additions and 774 deletions
Download Patch File Download Diff File Expand all files Collapse all files

54
src/interfaces/libpq/fe-misc.c
View File

@ -379,8 +379,8 @@ pqCheckOutBufferSpace(size_t bytes_needed, PGconn *conn)
}
/* realloc failed. Probably out of memory */
printfPQExpBuffer(&conn->errorMessage,
"cannot allocate memory for output buffer\n");
appendPQExpBufferStr(&conn->errorMessage,
"cannot allocate memory for output buffer\n");
return EOF;
}
@ -473,8 +473,8 @@ pqCheckInBufferSpace(size_t bytes_needed, PGconn *conn)
}
/* realloc failed. Probably out of memory */
printfPQExpBuffer(&conn->errorMessage,
"cannot allocate memory for input buffer\n");
appendPQExpBufferStr(&conn->errorMessage,
"cannot allocate memory for input buffer\n");
return EOF;
}
@ -619,8 +619,8 @@ pqReadData(PGconn *conn)
if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("connection not open\n"));
return -1;
}
@ -798,10 +798,10 @@ retry4:
* means the connection has been closed. Cope.
*/
definitelyEOF:
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("server closed the connection unexpectedly\n"
"\tThis probably means the server terminated abnormally\n"
"\tbefore or while processing the request.\n"));
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("server closed the connection unexpectedly\n"
"\tThis probably means the server terminated abnormally\n"
"\tbefore or while processing the request.\n"));
/* Come here if lower-level code already set a suitable errorMessage */
definitelyFailed:
@ -836,6 +836,7 @@ pqSendSome(PGconn *conn, int len)
{
char *ptr = conn->outBuffer;
int remaining = conn->outCount;
int oldmsglen = conn->errorMessage.len;
int result = 0;
/*
@ -862,13 +863,10 @@ pqSendSome(PGconn *conn, int len)
if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("connection not open\n"));
conn->write_failed = true;
/* Transfer error message to conn->write_err_msg, if possible */
/* Insert error message into conn->write_err_msg, if possible */
/* (strdup failure is OK, we'll cope later) */
conn->write_err_msg = strdup(conn->errorMessage.data);
resetPQExpBuffer(&conn->errorMessage);
conn->write_err_msg = strdup(libpq_gettext("connection not open\n"));
/* Discard queued data; no chance it'll ever be sent */
conn->outCount = 0;
return 0;
@ -915,14 +913,16 @@ pqSendSome(PGconn *conn, int len)
* Transfer error message to conn->write_err_msg, if
* possible (strdup failure is OK, we'll cope later).
*
* Note: this assumes that pqsecure_write and its children
* will overwrite not append to conn->errorMessage. If
* that's ever changed, we could remember the length of
* conn->errorMessage at entry to this routine, and then
* save and delete just what was appended.
* We only want to transfer whatever has been appended to
* conn->errorMessage since we entered this routine.
*/
conn->write_err_msg = strdup(conn->errorMessage.data);
resetPQExpBuffer(&conn->errorMessage);
if (!PQExpBufferBroken(&conn->errorMessage))
{
conn->write_err_msg = strdup(conn->errorMessage.data +
oldmsglen);
conn->errorMessage.len = oldmsglen;
conn->errorMessage.data[oldmsglen] = '\0';
}
/* Discard queued data; no chance it'll ever be sent */
conn->outCount = 0;
@ -1056,8 +1056,8 @@ pqWaitTimed(int forRead, int forWrite, PGconn *conn, time_t finish_time)
if (result == 0)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("timeout expired\n"));
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("timeout expired\n"));
return 1;
}
@ -1101,8 +1101,8 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
return -1;
if (conn->sock == PGINVALID_SOCKET)
{
printfPQExpBuffer(&conn->errorMessage,
libpq_gettext("invalid socket\n"));
appendPQExpBufferStr(&conn->errorMessage,
libpq_gettext("invalid socket\n"));
return -1;
}
@ -1124,7 +1124,7 @@ pqSocketCheck(PGconn *conn, int forRead, int forWrite, time_t end_time)
{
char sebuf[PG_STRERROR_R_BUFLEN];
printfPQExpBuffer(&conn->errorMessage,
appendPQExpBuffer(&conn->errorMessage,
libpq_gettext("select() failed: %s\n"),
SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
}