Do not return broken clients to the pool

.travis.yml

@@ -10,7 +10,10 @@ env:
 node_js:
   - lts/dubnium
   - lts/erbium
-  - 13
+  # node 13.7 seems to have changed behavior of async iterators exiting early on streams
+  # if 13.8 still has this problem when it comes down I'll talk to the node team about the change
+  # in the mean time...peg to 13.6 
+  - 13.6
 
 addons:
   postgresql: "10"

packages/pg-pool/index.js

@@ -25,6 +25,10 @@ class PendingItem {
   }
 }
 
+function throwOnDoubleRelease () {
+  throw new Error('Release called on client which has already been released to the pool.')
+}
+
 function promisify (Promise, callback) {
   if (callback) {
     return { callback: callback, result: undefined }
@@ -244,7 +248,7 @@ class Pool extends EventEmitter {
 
     client.release = (err) => {
       if (released) {
-        throw new Error('Release called on client which has already been released to the pool.')
+        throwOnDoubleRelease()
       }
 
       released = true
@@ -280,7 +284,8 @@ class Pool extends EventEmitter {
   _release (client, idleListener, err) {
     client.on('error', idleListener)
 
-    if (err || this.ending) {
+    // TODO(bmc): expose a proper, public interface _queryable and _ending
+    if (err || this.ending || !client._queryable || client._ending) {
       this._remove(client)
       this._pulseQueue()
       return

packages/pg-pool/test/releasing-clients.js

@@ -0,0 +1,54 @@
+const Pool = require('../')
+
+const expect = require('expect.js')
+const net = require('net')
+
+describe('releasing clients', () => {
+  it('removes a client which cannot be queried', async () => {
+    // make a pool w/ only 1 client
+    const pool = new Pool({ max: 1 })
+    expect(pool.totalCount).to.eql(0)
+    const client = await pool.connect()
+    expect(pool.totalCount).to.eql(1)
+    expect(pool.idleCount).to.eql(0)
+    // reach into the client and sever its connection
+    client.connection.end()
+
+    // wait for the client to error out
+    const err = await new Promise((resolve) => client.once('error', resolve))
+    expect(err).to.be.ok()
+    expect(pool.totalCount).to.eql(1)
+    expect(pool.idleCount).to.eql(0)
+
+    // try to return it to the pool - this removes it because its broken
+    client.release()
+    expect(pool.totalCount).to.eql(0)
+    expect(pool.idleCount).to.eql(0)
+
+    // make sure pool still works
+    const { rows } = await pool.query('SELECT NOW()')
+    expect(rows).to.have.length(1)
+    await pool.end()
+  })
+
+  it('removes a client which is ending', async () => {
+    // make a pool w/ only 1 client
+    const pool = new Pool({ max: 1 })
+    expect(pool.totalCount).to.eql(0)
+    const client = await pool.connect()
+    expect(pool.totalCount).to.eql(1)
+    expect(pool.idleCount).to.eql(0)
+    // end the client gracefully (but you shouldn't do this with pooled clients)
+    client.end()
+
+    // try to return it to the pool
+    client.release()
+    expect(pool.totalCount).to.eql(0)
+    expect(pool.idleCount).to.eql(0)
+
+    // make sure pool still works
+    const { rows } = await pool.query('SELECT NOW()')
+    expect(rows).to.have.length(1)
+    await pool.end()
+  })
+})