Throw on reconnect attempt

Clients are not reusable.  This changes the client to raise errors whenever you try to reconnect a client that's already been used.  They're cheap to create: just instantiate a new one (or use the pool) 😉.

Closes brianc#1352

Author: brianc

lib/client.js

@@ -34,6 +34,7 @@ var Client = function(config) {
   this._types = new TypeOverrides(c.types);
   this._ending = false;
   this._connecting = false;
+  this._connected = false;
   this._connectionError = false;
 
   this.connection = c.connection || new Connection({
@@ -54,6 +55,14 @@ util.inherits(Client, EventEmitter);
 Client.prototype.connect = function(callback) {
   var self = this;
   var con = this.connection;
+  if (this._connecting || this._connected) {
+    const err = new Error('Client has already been connected. You cannot reuse a client.')
+    if (callback) {
+      callback(err)
+      return undefined
+    }
+    return Promise.reject(err)
+  }
   this._connecting = true;
 
   if(this.host && this.host.indexOf('/') === 0) {
@@ -135,6 +144,7 @@ Client.prototype.connect = function(callback) {
   //after the connection initially becomes ready for queries
   con.once('readyForQuery', function() {
     self._connecting = false;
+    self._connected = true;
     self._attachListeners(con);
     con.removeListener('error', connectingErrorHandler);
     con.on('error', connectedErrorHandler)

lib/index.js

@@ -21,7 +21,7 @@ const poolFactory = (Client) => {
     for (var key in options) {
       config[key] = options[key];
     }
-    Pool.call(this, config);
+    return new Pool(config)
   };
 
   util.inherits(BoundPool, Pool);

lib/native/client.js

@@ -33,6 +33,7 @@ var Client = module.exports = function(config) {
 
   this._queryQueue = [];
   this._connected = false;
+  this._connecting = false;
 
   //keep these on the object for legacy reasons
   //for the time being. TODO: deprecate all this jazz
@@ -74,6 +75,13 @@ Client.prototype.connect = function(cb) {
     })
   }
 
+  if (this._connecting) {
+    process.nextTick(() => cb(new Error('Client has already been connected. You cannot reuse a client.')))
+    return result
+  }
+
+  this._connecting = true
+
   this.connectionParameters.getLibpqConnectionString(function(err, conString) {
     if(err) return onError(err);
     self.native.connect(conString, function(err) {

package.json

@@ -21,7 +21,7 @@
     "buffer-writer": "1.0.1",
     "packet-reader": "0.3.1",
     "pg-connection-string": "0.1.3",
-    "pg-pool": "1.*",
+    "pg-pool": "2.*",
     "pg-types": "1.*",
     "pgpass": "1.x",
     "semver": "4.3.2"

test/integration/client/api-tests.js

@@ -8,7 +8,7 @@ suite.test("pool callback behavior", done => {
   //test weird callback behavior with node-pool
   const pool = new pg.Pool();
   pool.connect(function(err) {
-    assert.isNull(err);
+    assert(!err);
     arguments[1].emit("drain");
     arguments[2]();
     pool.end(done);
@@ -52,7 +52,7 @@ suite.test("executing nested queries", function(done) {
   const pool = new pg.Pool();
   pool.connect(
     assert.calls(function(err, client, release) {
-      assert.isNull(err);
+      assert(!err);
       client.query(
         "select now as now from NOW()",
         assert.calls(function(err, result) {
@@ -91,7 +91,7 @@ suite.test("query errors are handled and do not bubble if callback is provded",
     const pool = new pg.Pool();
     pool.connect(
       assert.calls(function(err, client, release) {
-        assert.isNull(err);
+        assert(!err);
         client.query(
           "SELECT OISDJF FROM LEIWLISEJLSE",
           assert.calls(function(err, result) {
@@ -109,7 +109,7 @@ suite.test("callback is fired once and only once", function(done) {
   const pool = new pg.Pool()
   pool.connect(
     assert.calls(function(err, client, release) {
-      assert.isNull(err);
+      assert(!err);
       client.query("CREATE TEMP TABLE boom(name varchar(10))");
       var callCount = 0;
       client.query(
@@ -136,14 +136,14 @@ suite.test("can provide callback and config object", function(done) {
   const pool = new pg.Pool()
   pool.connect(
     assert.calls(function(err, client, release) {
-      assert.isNull(err);
+      assert(!err);
       client.query(
         {
           name: "boom",
           text: "select NOW()"
         },
         assert.calls(function(err, result) {
-          assert.isNull(err);
+          assert(!err);
           assert.equal(result.rows[0].now.getYear(), new Date().getYear());
           release();
           pool.end(done)
@@ -157,15 +157,15 @@ suite.test("can provide callback and config and parameters", function(done) {
   const pool = new pg.Pool()
   pool.connect(
     assert.calls(function(err, client, release) {
-      assert.isNull(err);
+      assert(!err);
       var config = {
         text: "select $1::text as val"
       };
       client.query(
         config,
         ["hi"],
         assert.calls(function(err, result) {
-          assert.isNull(err);
+          assert(!err);
           assert.equal(result.rows.length, 1);
           assert.equal(result.rows[0].val, "hi");
           release()
@@ -180,7 +180,7 @@ suite.test("null and undefined are both inserted as NULL", function(done) {
   const pool = new pg.Pool()
   pool.connect(
     assert.calls(function(err, client, release) {
-      assert.isNull(err);
+      assert(!err);
       client.query(
         "CREATE TEMP TABLE my_nulls(a varchar(1), b varchar(1), c integer, d integer, e date, f date)"
       );
@@ -191,7 +191,7 @@ suite.test("null and undefined are both inserted as NULL", function(done) {
       client.query(
         "SELECT * FROM my_nulls",
         assert.calls(function(err, result) {
-          assert.isNull(err);
+          assert(!err);
           assert.equal(result.rows.length, 1);
           assert.isNull(result.rows[0].a);
           assert.isNull(result.rows[0].b);