Merge pull request #1177 from wubzz/bugfix/knex_hangs_when_pool_is_fi…

…lled_with_trs

Bugfix/knex hangs when pool is filled with trs

index.html

@@ -60,6 +60,7 @@
       <li>&nbsp;&nbsp;– <a href="#Installation-client">client</a></li>
       <li>&nbsp;&nbsp;– <a href="#Installation-debug">debug</a></li>
       <li>&nbsp;&nbsp;– <a href="#Installation-pooling">pooling</a></li>
+      <li>&nbsp;&nbsp;– <a href="#Installation-acquireConnectionTimeout">acquireConnectionTimeout</a></li>
       <li>&nbsp;&nbsp;– <a href="#Installation-migrations">migrations</a></li>
     </ul>
 
@@ -504,6 +505,26 @@ <h3 id="Installation-pooling">Pooling</h3>
       You may use <tt>knex.destroy</tt> by passing a callback, or by chaining as a promise, just not both.
     </p>
 
+
+    <h3 id="Installation-acquireConnectionTimeout">acquireConnectionTimeout</h3>
+
+    <p>
+      <tt>acquireConnectionTimeout</tt> defaults to 60000ms and is used to determine how long knex should wait before throwing a timeout error when acquiring a connection is not possible.
+      The most common cause for this is using up all the pool for transaction connections and then attempting to run queries outside of transactions while the pool is still full.
+      The error thrown will provide information on the query the connection was for to simplify the job of locating the culprit.
+    </p>
+
+<pre>
+<code class="js">var knex = require('knex')({
+  client: 'pg',
+  connection: {...},
+  pool: {...},
+  acquireConnectionTimeout: 10000
+  });
+</code>
+</pre>
+
+
     <h3 id="Installation-migrations">Migrations</h3>
 
     <p>

src/runner.js

@@ -135,8 +135,29 @@ assign(Runner.prototype, {
   // Check whether there's a transaction flag, and that it has a connection.
   ensureConnection: function() {
     var runner = this
+    var acquireConnectionTimeout = runner.client.config.acquireConnectionTimeout || 60000
     return Promise.try(function() {
-      return runner.connection || runner.client.acquireConnection()
+      return runner.connection || new Promise((resolver, rejecter) => {
+            runner.client.acquireConnection()
+            .timeout(acquireConnectionTimeout)
+            .then(resolver)
+            .catch(Promise.TimeoutError, (error) => {
+                  var timeoutError = new Error('Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?')
+                  var additionalErrorInformation = {
+                    timeoutStack: error.stack
+                  }
+
+                  if(runner.builder) {
+                    additionalErrorInformation.sql = runner.builder.sql
+                    additionalErrorInformation.bindings = runner.builder.bindings
+                  }
+
+                  assign(timeoutError, additionalErrorInformation)
+
+                  rejecter(timeoutError)
+            })
+            .catch(rejecter)
+          })
     }).disposer(function() {
       if (runner.connection.__knex__disposed) return
       runner.client.releaseConnection(runner.connection)

test/integration/builder/transaction.js

@@ -3,6 +3,8 @@
 'use strict';
 
 var Promise = testPromise;
+var Knex   = require('../../../knex');
+var _ = require('lodash');
 
 module.exports = function(knex) {
 
@@ -277,6 +279,35 @@ module.exports = function(knex) {
 
     });
 
+    it('#1040, #1171 - When pool is filled with transaction connections, Non-transaction queries should not hang the application, but instead throw a timeout error', function() {
+      //To make this test easier, I'm changing the pool settings to max 1.
+      var knexConfig = _.clone(knex.client.config);
+      knexConfig.pool.min = 0;
+      knexConfig.pool.max = 1;
+      knexConfig.acquireConnectionTimeout = 1000;
+
+      var knexDb = new Knex(knexConfig);
+
+      //Create a transaction that will occupy the only available connection, and avoid trx.commit.
+     return knexDb.transaction(function(trx) {
+       trx.raw('SELECT 1 = 1').then(function () {
+         //No connection is available, so try issuing a query without transaction.
+         //Since there is no available connection, it should throw a timeout error based on `aquireConnectionTimeout` from the knex config.
+         return knexDb.raw('select * FROM accounts WHERE username = ?', ['Test'])
+       })
+       .then(function () {
+         //Should never reach this point
+         expect(false).to.be.ok();
+       }).catch(function (error) {
+         expect(error.bindings).to.be.an('array');
+         expect(error.bindings[0]).to.equal('Test');
+         expect(error.sql).to.equal('select * FROM accounts WHERE username = ?');
+         expect(error.message).to.equal('Knex: Timeout acquiring a connection. The pool is probably full. Are you missing a .transacting(trx) call?');
+         trx.commit();//Test done
+       });
+      });
+    });
+
   });
 
 };