Remove timed-out checkouts from queue correctly (brianc#86)

* Add failing test for correct removal from checkout queue on timeout

* Remove timed-out checkouts from queue correctly

Fixes brianc#85.

Author: charmander

index.js

@@ -3,6 +3,14 @@ const EventEmitter = require('events').EventEmitter
 
 const NOOP = function () { }
 
+const remove = (list, value) => {
+  const i = list.indexOf(value)
+
+  if (i !== -1) {
+    list.splice(i, 1)
+  }
+}
+
 const removeWhere = (list, predicate) => {
   const i = list.findIndex(predicate)
 
@@ -157,18 +165,20 @@ class Pool extends EventEmitter {
         return result
       }
 
+      const queueCallback = (err, res, done) => {
+        clearTimeout(tid)
+        response.callback(err, res, done)
+      }
+
       // set connection timeout on checking out an existing client
       const tid = setTimeout(() => {
         // remove the callback from pending waiters because
         // we're going to call it with a timeout error
-        this._pendingQueue = this._pendingQueue.filter(cb => cb === response.callback)
+        remove(this._pendingQueue, queueCallback)
         response.callback(new Error('timeout exceeded when trying to connect'))
       }, this.options.connectionTimeoutMillis)
 
-      this._pendingQueue.push(function (err, res, done) {
-        clearTimeout(tid)
-        response.callback(err, res, done)
-      })
+      this._pendingQueue.push(queueCallback)
       return result
     }
 

test/connection-timeout.js

@@ -73,6 +73,28 @@ describe('connection timeout', () => {
     })
   })
 
+  it('should not break further pending checkouts on a timeout', (done) => {
+    const pool = new Pool({ connectionTimeoutMillis: 200, max: 1 })
+    pool.connect((err, client, releaseOuter) => {
+      expect(err).to.be(undefined)
+
+      pool.connect((err, client) => {
+        expect(err).to.be.an(Error)
+        expect(client).to.be(undefined)
+        releaseOuter()
+      })
+
+      setTimeout(() => {
+        pool.connect((err, client, releaseInner) => {
+          expect(err).to.be(undefined)
+          expect(client).to.not.be(undefined)
+          releaseInner()
+          pool.end(done)
+        })
+      }, 100)
+    })
+  })
+
   it('should timeout on query if all clients are busy', (done) => {
     const pool = new Pool({ connectionTimeoutMillis: 100, max: 1 })
     pool.connect((err, client, release) => {