Re: libpq ssl -> clear fallback looses error messages

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 */