diff --git a/src/client.js b/src/client.js index 92c261be54..378b709ed1 100644 --- a/src/client.js +++ b/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, diff --git a/src/dialects/maria/index.js b/src/dialects/maria/index.js index d4f7c1fc4d..4d9d77fa7a 100644 --- a/src/dialects/maria/index.js +++ b/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 diff --git a/src/dialects/mssql/index.js b/src/dialects/mssql/index.js index cd17e8b9bc..27985cf09f 100644 --- a/src/dialects/mssql/index.js +++ b/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 diff --git a/src/dialects/mysql/index.js b/src/dialects/mysql/index.js index 0654fc5e4e..be6d267495 100644 --- a/src/dialects/mysql/index.js +++ b/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, diff --git a/src/dialects/mysql2/index.js b/src/dialects/mysql2/index.js index 29621e9322..7b027c62cb 100644 --- a/src/dialects/mysql2/index.js +++ b/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 diff --git a/src/runner.js b/src/runner.js index d372ff3642..f1aaabdef4 100644 --- a/src/runner.js +++ b/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) + }); } }) diff --git a/test/docker/index.js b/test/docker/index.js index 58b201a14d..0db3a2e2d8 100644 --- a/test/docker/index.js +++ b/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; } diff --git a/test/docker/reconnect.js b/test/docker/reconnect.js index 78d8ade413..164497381e 100644 --- a/test/docker/reconnect.js +++ b/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, diff --git a/test/knexfile.js b/test/knexfile.js index 41ecb4aea2..855aa25c0f 100644 --- a/test/knexfile.js +++ b/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 },