Re: libpq ssl -> clear fallback looses error messages - Mailing list pgsql-hackers
From | Magnus Hagander |
---|---|
Subject | Re: libpq ssl -> clear fallback looses error messages |
Date | |
Msg-id | 48F11DBA.6010806@hagander.net Whole thread Raw |
In response to | Re: libpq ssl -> clear fallback looses error messages (Magnus Hagander <magnus@hagander.net>) |
Responses |
Re: libpq ssl -> clear fallback looses error messages
|
List | pgsql-hackers |
Magnus Hagander wrote: > Tom Lane wrote: >> Magnus Hagander <magnus@hagander.net> writes: >>> Tom Lane wrote: >>>> Maybe the answer is to not throw away the first error message? But >>>> presenting both messages could be confusing too. >>> Do we have the infrastructure to report more than one error? I thought >>> we didn't... >> I was thinking of merging the reports into a single PGresult. The >> tricky part is to figure out how to present it, and also when to throw >> away one of the two reports --- if one is obviously bogus you don't want >> to distract the user with it. > > Um, PGresult? These errors occur on connection, so it needs to go in PGconn. > > I guess the easy way would be to just append the error reports with a \n > between them. I think it would be good to keep both error messages in > *all* cases. If the second connection succeeds, you will not get the > error condition anyway, and thus not look at the error message from the > first one. Here's an ugly attempt towards this. Though I'm unsure if we can change the "const" on the PQerrorMessage parameter without messing with library versions and such? Another option could be to change all the calls that set the error message to *append* to the error message instead. Thoughts on that? //Magnus *** a/src/interfaces/libpq/fe-connect.c --- b/src/interfaces/libpq/fe-connect.c *************** *** 243,248 **** static char *pwdfMatchesString(char *buf, char *token); --- 243,249 ---- static char *PasswordFromFile(char *hostname, char *port, char *dbname, char *username); static void default_threadlock(int acquire); + static void save_error_messages(PGconn *conn); /* global variable because fe-auth.c needs to access it */ *************** *** 955,960 **** connectDBComplete(PGconn *conn) --- 956,975 ---- } } + /* + * save_error_message - save the current error message in a simple + * one element "stack" + */ + static void + save_error_messages(PGconn *conn) + { + if (conn->errorMessage.len > 0) + { + initPQExpBuffer(&conn->errorMessageSaved); + appendPQExpBufferStr(&conn->errorMessageSaved, conn->errorMessage.data); + } + } + /* ---------------- * PQconnectPoll * *************** *** 1447,1452 **** keep_going: /* We will come back to here until there is --- 1462,1469 ---- closesocket(conn->sock); conn->sock = -1; conn->status = CONNECTION_NEEDED; + save_error_messages(conn); + goto keep_going; } } *************** *** 1627,1632 **** keep_going: /* We will come back to here until there is --- 1644,1650 ---- closesocket(conn->sock); conn->sock = -1; conn->status = CONNECTION_NEEDED; + save_error_messages(conn); goto keep_going; } *************** *** 1646,1651 **** keep_going: /* We will come back to here until there is --- 1664,1670 ---- closesocket(conn->sock); conn->sock = -1; conn->status = CONNECTION_NEEDED; + save_error_messages(conn); goto keep_going; } #endif *************** *** 1947,1952 **** makeEmptyPGconn(void) --- 1966,1972 ---- conn->outBufSize = 16 * 1024; conn->outBuffer = (char *) malloc(conn->outBufSize); initPQExpBuffer(&conn->errorMessage); + conn->errorMessageSaved.data = NULL; initPQExpBuffer(&conn->workBuffer); if (conn->inBuffer == NULL || *************** *** 2023,2028 **** freePGconn(PGconn *conn) --- 2043,2050 ---- if (conn->outBuffer) free(conn->outBuffer); termPQExpBuffer(&conn->errorMessage); + if (conn->errorMessageSaved.data) + termPQExpBuffer(&conn->errorMessageSaved); termPQExpBuffer(&conn->workBuffer); free(conn); *************** *** 3538,3549 **** PQserverVersion(const PGconn *conn) } char * ! PQerrorMessage(const PGconn *conn) { if (!conn) return libpq_gettext("connection pointer is NULL\n"); ! return conn->errorMessage.data; } int --- 3560,3584 ---- } char * ! PQerrorMessage(PGconn *conn) { if (!conn) return libpq_gettext("connection pointer is NULL\n"); ! if (conn->errorMessageSaved.data) ! { ! /* ! * Merge the two error message strings together. ! * Stick the result in errorMessage, and make sure to ! * get rid of the saved one so we don't keep adding everything. ! */ ! appendPQExpBufferStr(&conn->errorMessageSaved, conn->errorMessage.data); ! ! resetPQExpBuffer(&conn->errorMessage); ! return conn->errorMessageSaved.data; ! } ! else ! return conn->errorMessage.data; } int *** a/src/interfaces/libpq/libpq-fe.h --- b/src/interfaces/libpq/libpq-fe.h *************** *** 287,293 **** extern const char *PQparameterStatus(const PGconn *conn, const char *paramName); extern int PQprotocolVersion(const PGconn *conn); extern int PQserverVersion(const PGconn *conn); ! extern char *PQerrorMessage(const PGconn *conn); extern int PQsocket(const PGconn *conn); extern int PQbackendPID(const PGconn *conn); extern int PQconnectionNeedsPassword(const PGconn *conn); --- 287,293 ---- const char *paramName); extern int PQprotocolVersion(const PGconn *conn); extern int PQserverVersion(const PGconn *conn); ! extern char *PQerrorMessage(PGconn *conn); extern int PQsocket(const PGconn *conn); extern int PQbackendPID(const PGconn *conn); extern int PQconnectionNeedsPassword(const PGconn *conn); *** a/src/interfaces/libpq/libpq-int.h --- b/src/interfaces/libpq/libpq-int.h *************** *** 402,407 **** struct pg_conn --- 402,408 ---- /* Buffer for current error message */ PQExpBufferData errorMessage; /* expansible string */ + PQExpBufferData errorMessageSaved; /* saved in some retried operations */ /* Buffer for receiving various parts of messages */ PQExpBufferData workBuffer; /* expansible string */
pgsql-hackers by date: