pg.connect returns both err and client as nulls when PostgreSQL is down

[lmorandini]

I wrote an application using the connection pool, and it had gone well until PostgreSQL went down (actually, a network error), and, as far as i can tell, pg.connect returned a null client AND a null error, hence breaking my code (I expected err to be non-null in a case like this).
It has been easy to patch up my code, but I suppose this is not the intended pg.connect behavior.

[brianc]

Yeah that' sounds like a bug. Do you have steps to reproduce this?

[strk]

I think this is what I'm seeing here too: CartoDB/CartoDB-SQL-API#135

Adding debugging logs I get:

[2014-03-14 14:34:20.996] [INFO] console - Connection[3] created
[2014-03-14 14:34:20.996] [INFO] console - Connection[3].connect called
[2014-03-14 14:34:20.999] [INFO] console - pool.aquire returned err null and client [object Object]
...
[2014-03-14 14:34:21.543] [INFO] console - Connection[3].stream end
[2014-03-14 14:34:21.543] [INFO] console - query 'error' event cought: Error: Stream unexpectedly ended during query execution
[2014-03-14 14:34:21.543] [INFO] console - query 'end' event cought, this.error is Error: Stream unexpectedly ended during query execution
[2014-03-14 14:34:21.543] [INFO] console - sendResponse returned err: Error: Stream unexpectedly ended during query execution
...
[2014-03-14 14:34:27.598] [INFO] console - me.connect calling pg.connect
[2014-03-14 14:34:27.598] [INFO] console - pool.aquire returned err null and client [object Object]

Evidently the stream error did not trigger connection removal from the pool, which returned the same "bad" connection again on next .acquire call.

I also reported this in a comment to #534 but now I'm not sure anymore if this is the real reason for my problem (or I'm having both). Tested versions (all affected): 2.4.0, 2.6.2, 2.11.0+ (2716f95)

[strk]

Sorry, I'm obviously getting a client object from aquire, so not the same as this bug either :/
Maybe it's #458

[strk]

Installing a listener for "end" event on the client from lib/pool.js to destroy the client fixes my case.
Indeed "error" event in client is never raised on premature stream close, while "end" is called.

[strk]

This is the code snippet triggering "end" event for a client but no "error" event:

  con.once('end', function() {
    if(self.activeQuery) {
      var disconnectError = new Error('Stream unexpectedly ended during query execution');
      self.activeQuery.handleError(disconnectError);
      self.activeQuery = null;
    }
    self.emit('end');
  });

Should that be an error instead ?
Or is it ok to also catch "end" as an error that should trigger removal from pool ?

[strk]

forget it, I don't confirm that handling "end" fixes my case, must have been a lucky run

[strk]

Sorry for the confusion, the patch actually works, only when I re-tested it I added a syntax error which broke it. The working patch is here:

diff --git a/lib/pool.js b/lib/pool.js
index 9cf9aab..27aa84c 100644
--- a/lib/pool.js
+++ b/lib/pool.js
@@ -33,6 +33,13 @@ var pools = {
             pool.destroy(client);
           });

+          // Some errors trigger "end" rather than "error", see
+          // https://github.com/brianc/node-postgres/issues/418#issuecomment-37657396
+          client.on('end', function() {
+            pool.destroy(client);
+          });
+
+
           return cb(null, client);
         });
       },