fix compatibility issues with generic-pool v3

- connection validators should always resolve with a boolean
- rely on generic pool acquire timeouts to avoid unhandled rejections
- simplify promise usage, avoiding Promise constructor anti-pattern
- enable testOnBorrow in docker tests to evict broken connections
- enable docker tests on Darwin
- enable docker tests for mysql2 dialect

src/client.js

@@ -17,6 +17,7 @@ import ColumnBuilder from './schema/columnbuilder';
 import ColumnCompiler from './schema/columncompiler';
 
 import * as genericPool from 'generic-pool';
+import * as genericPoolErrors from 'generic-pool/lib/errors'
 import inherits from 'inherits';
 import { EventEmitter } from 'events';
 
@@ -169,7 +170,14 @@ assign(Client.prototype, {
   },
 
   getPoolSettings(poolConfig) {
-    poolConfig = defaults(poolConfig, {min: 2, max: 10});
+    poolConfig = defaults(poolConfig, {min: 2, max: 10, Promise});
+    const timeouts = [
+      this.config.acquireConnectionTimeout,
+      poolConfig.acquireTimeoutMillis
+    ].map(parseInt).filter(x => x > 0);
+    if (timeouts.length > 0) {
+      poolConfig.acquireTimeoutMillis = Math.min(...timeouts);
+    }
 
     return {
       config: poolConfig,
@@ -200,7 +208,7 @@ assign(Client.prototype, {
         validate: (connection) => {
           if (connection.__knex__disposed) {
             helpers.warn(`Connection Error: ${connection.__knex__disposed}`)
-            return Promise.reject();
+            return Promise.resolve(false);
           }
           return this.validateConnection(connection)
         }
@@ -220,39 +228,24 @@ assign(Client.prototype, {
   },
 
   validateConnection(connection) {
-    return Promise.resolve();
+    return Promise.resolve(true);
   },
 
   // Acquire a connection from the pool.
   acquireConnection() {
-    return new Promise((resolver, rejecter) => {
-      if (!this.pool) {
-        return rejecter(new Error('Unable to acquire a connection'))
-      }
-      let wasRejected = false
-      const t = setTimeout(() => {
-        wasRejected = true
-        rejecter(new Promise.TimeoutError(
+    if (!this.pool) {
+      return Promise.reject(new Error('Unable to acquire a connection'))
+    }
+    return this.pool.acquire()
+      .tap(connection => {
+        debug('acquired connection from pool: %s', connection.__knexUid)
+      })
+      .catch(genericPoolErrors.TimeoutError, () => {
+        throw new Promise.TimeoutError(
           'Knex: Timeout acquiring a connection. The pool is probably full. ' +
           'Are you missing a .transacting(trx) call?'
-        ))
-      }, this.config.acquireConnectionTimeout || 60000)
-      this.pool.acquire()
-      .then((connection) => {
-        clearTimeout(t)
-        if(wasRejected) {
-          this.pool.release(connection);
-        } else {
-          debug('acquired connection from pool: %s', connection.__knexUid)
-          resolver(connection);
-        }
-      })
-      .catch((error) => {
-        clearTimeout(t);
-
-        throw error;
+        )
       });
-    })
   },
 
   // Releases a connection back to the connection pool,

src/dialects/maria/index.js

@@ -45,9 +45,9 @@ assign(Client_MariaSQL.prototype, {
 
   validateConnection(connection) {
     if(connection.connected === true) {
-      return Promise.resolve();
+      return Promise.resolve(true);
     }
-    return Promise.reject();
+    return Promise.resolve(false);
   },
 
   // Used to explicitly close a connection, called internally by the pool

src/dialects/mssql/index.js

@@ -89,9 +89,9 @@ assign(Client_MSSQL.prototype, {
 
   validateConnection(connection) {
     if(connection.connected === true) {
-      return Promise.resolve();
+      return Promise.resolve(true);
     }
-    return Promise.reject();
+    return Promise.resolve(false);
   },
 
   // Used to explicitly close a connection, called internally by the pool

src/dialects/mysql/index.js

@@ -86,9 +86,9 @@ assign(Client_MySQL.prototype, {
 
   validateConnection(connection) {
     if(connection.state === 'connected' || connection.state === 'authenticated') {
-      return Promise.resolve();
+      return Promise.resolve(true);
     }
-    return Promise.reject();
+    return Promise.resolve(false);
   },
 
   // Grab a connection, run the query via the MySQL streaming interface,

src/dialects/mysql2/index.js

@@ -29,8 +29,11 @@ assign(Client_MySQL2.prototype, {
     return require('mysql2')
   },
 
-  validateConnection() {
-    return Promise.resolve();
+  validateConnection(connection) {
+    if(connection._fatalError) {
+      return Promise.resolve(false);
+    }
+    return Promise.resolve(true);
   },
 
   // Get a raw connection, called by the `pool` whenever a new

src/runner.js

@@ -185,24 +185,21 @@ assign(Runner.prototype, {
 
   // Check whether there's a transaction flag, and that it has a connection.
   ensureConnection() {
-    return Promise.try(() => {
-      return this.connection || new Promise((resolver, rejecter) => {
-        // need to return promise or null from handler to prevent warning from bluebird
-        return this.client.acquireConnection()
-          .then(resolver)
-          .catch(Promise.TimeoutError, (error) => {
-            if (this.builder) {
-              error.sql = this.builder.sql;
-              error.bindings = this.builder.bindings;
-            }
-            throw error
-          })
-          .catch(rejecter)
+    if(this.connection) {
+      return Promise.resolve(this.connection)
+    }
+    return this.client.acquireConnection()
+      .catch(Promise.TimeoutError, error => {
+        if (this.builder) {
+          error.sql = this.builder.sql;
+          error.bindings = this.builder.bindings;
+        }
+        throw error;
       })
-    }).disposer(() => {
-      // need to return promise or null from handler to prevent warning from bluebird
-      return this.client.releaseConnection(this.connection)
-    })
+      .disposer(() => {
+        // need to return promise or null from handler to prevent warning from bluebird
+        return this.client.releaseConnection(this.connection)
+      });
   }
 
 })

test/docker/index.js

@@ -1,27 +1,29 @@
+/*global describe*/
+
 'use strict';
 
-var os      = require('os');
-var proc    = require('child_process')
-var config  = require('../knexfile');
-var knex    = require('../../knex');
+const os      = require('os');
+const proc    = require('child_process')
+const config  = require('../knexfile');
+const knex    = require('../../knex');
 
 if (canRunDockerTests()) {
-  var dialectName;
-  for (dialectName in config) {
+  for (const dialectName in config) {
     if (config[dialectName].docker) {
-      require('./reconnect')(config[dialectName], knex);
+      describe(`${dialectName} dialect`, function() {
+        require('./reconnect')(config[dialectName], knex);
+      })
     }
   }
 }
 
 function canRunDockerTests() {
-  var isLinux   = os.platform() === 'linux';
-
+  const isLinux   = os.platform() === 'linux';
+  const isDarwin  = os.platform() === 'darwin'
   // dont even try on windows / osx for now
-  var hasDocker = false;
-  if (isLinux) {
+  let hasDocker = false;
+  if (isLinux || isDarwin) {
     hasDocker = proc.execSync('docker -v 1>/dev/null 2>&1 ; echo $?').toString('utf-8') === '0\n';
   }
-
-  return isLinux && hasDocker;
+  return hasDocker;
 }

test/docker/reconnect.js

@@ -119,7 +119,8 @@ module.exports = function(config, knex) {
         max:                       7,
         idleTimeoutMillis:         IDLE_TIMEOUT_MILLIS,
         acquireTimeoutMillis:      ACQUIRE_TIMEOUT_MILLIS,
-        evictionRunIntervalMillis: EVICTION_RUN_INTERVAL_MILLIS
+        evictionRunIntervalMillis: EVICTION_RUN_INTERVAL_MILLIS,
+        testOnBorrow: true
       },
       connection:               {
         database: dockerConf.database,

test/knexfile.js

@@ -88,7 +88,17 @@ var testConfigs = {
     },
     pool: mysqlPool,
     migrations: migrations,
-    seeds: seeds
+    seeds: seeds,
+    docker: {
+      factory:   './mysql/index.js',
+      container: 'knex-test-mysql2',
+      image:     'mysql:5.7',
+      database:  'mysql',
+      username:  'root',
+      password:  'root',
+      hostPort:  '49153',
+      client:    'mysql'
+    }
   },
 
   oracle: {
@@ -133,7 +143,7 @@ var testConfigs = {
       username:  'postgres',
       password:  '',
       hostPort:  '49152',
-      client:    'pg',
+      client:    'pg'
     }
   },
 
@@ -142,7 +152,7 @@ var testConfigs = {
     connection: testConfig.sqlite3 || {
       filename: __dirname + '/test.sqlite3'
     },
-    pool: pool,
+    pool: Object.assign({max: 1}, pool),
     migrations: migrations,
     seeds: seeds
   },