From 84cec97c7ca1ef5ea20d9230feaf1b495675a5ee Mon Sep 17 00:00:00 2001 From: Adam Date: Mon, 26 Oct 2015 12:11:59 -0700 Subject: [PATCH 1/2] When a connection error occurs don't call connection.close() as the driver always closes the connection itself when it transitions to STATE.FINAL. Fix tests to handle this change and also be able read connectionConfig from a file similar to tedious module tests. Also add test commands to package.json so it's easier to discover how to run unit tests. --- lib/connection-pool.js | 3 - package.json | 2 +- test/connection-pool.test.js | 115 ++++++++++++++++++++++++++++------- 3 files changed, 95 insertions(+), 25 deletions(-) mode change 100644 => 100755 lib/connection-pool.js mode change 100644 => 100755 package.json mode change 100644 => 100755 test/connection-pool.test.js diff --git a/lib/connection-pool.js b/lib/connection-pool.js old mode 100644 new mode 100755 index 0d553c3..13dfaf9 --- a/lib/connection-pool.js +++ b/lib/connection-pool.js @@ -82,15 +82,12 @@ function createConnection(pooled) { self.log('connection closing because of error'); connection.removeListener('end', endHandler); - connection.close(); - pooled.status = RETRY; pooled.con = undefined; if (pooled.timeout) clearTimeout(pooled.timeout); pooled.timeout = setTimeout(createConnection.bind(self, pooled), self.retryDelay); - self.emit('error', err); }; diff --git a/package.json b/package.json old mode 100644 new mode 100755 index 6429eeb..7556cbf --- a/package.json +++ b/package.json @@ -4,7 +4,7 @@ "description": "Connection Pool for tedious.", "main": "lib/connection-pool.js", "scripts": { - "test": "test" + "test": "mocha test/naming.test.js && mocha test/connection-pool.test.js" }, "repository": { "type": "git", diff --git a/test/connection-pool.test.js b/test/connection-pool.test.js old mode 100644 new mode 100755 index f1f2a8d..2478f2e --- a/test/connection-pool.test.js +++ b/test/connection-pool.test.js @@ -1,16 +1,13 @@ var assert = require('assert'); var Request = require('tedious').Request; +var fs = require('fs'); var ConnectionPool = require('../lib/connection-pool'); -var connectionConfig = { - userName: 'test', - password: 'test', - server: 'dev1', - options: { - appName: 'pool-test', - database: 'test' - } -}; +function getConfig() { + var config = JSON.parse(fs.readFileSync(process.env.HOME + '/.tedious/test-connection-pool.json', 'utf8')).config; + return config; +} + /* create a db user with the correct permissions: CREATE DATABASE test CREATE LOGIN test WITH PASSWORD=N'test', DEFAULT_DATABASE=test, CHECK_POLICY=OFF @@ -36,7 +33,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 2}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); setTimeout(function() { assert.equal(pool.connections.length, poolConfig.min); @@ -48,7 +45,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 0, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); setTimeout(function() { assert.equal(pool.connections.length, 0); @@ -80,7 +77,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 2, max: 5}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); var count = 20; var run = 0; @@ -119,7 +116,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = { min: 5, max: 1, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); setTimeout(function() { assert.equal(pool.connections.length, 1); @@ -151,7 +148,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {max: 1, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); setTimeout(function() { assert.equal(pool.connections.length, 1); @@ -199,7 +196,7 @@ describe('ConnectionPool', function () { pool.on('error', function(err) { assert(!!err); - pool.connectionConfig = connectionConfig; + pool.connectionConfig = getConfig(); }); function testConnected() { @@ -221,7 +218,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 1, max: 1, acquireTimeout: 2000}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); pool.acquire(function(err, connection) { assert(!err); @@ -238,7 +235,7 @@ describe('ConnectionPool', function () { it('idle timeout', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5, idleTimeout: 100}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); setTimeout(function() { pool.acquire(function (err, connection) { @@ -262,11 +259,15 @@ describe('ConnectionPool', function () { it('lost connection handling', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5}; + var connectionConfig = getConfig(); + + connectionConfig.options = connectionConfig.options || {}; + connectionConfig.options.requestTimeout = 5000; + var pool = new ConnectionPool(poolConfig, connectionConfig); pool.on('error', function(err) { assert(err && err.name === 'ConnectionError'); - pool.drain(done); }); //This simulates a lost connections by creating a job that kills the current session and then deletesthe job. @@ -290,7 +291,9 @@ describe('ConnectionPool', function () { 'SELECT 42'; var request = new Request(command, function (err, rowCount) { - assert(false); + assert(err); + assert(err.code,'ETIMEOUT'); + pool.drain(done); }); request.on('row', function (columns) { @@ -305,7 +308,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {max: 1}; - var pool = new ConnectionPool(poolConfig, connectionConfig); + var pool = new ConnectionPool(poolConfig, getConfig()); var createRequest = function(query, value, callback) { var request = new Request(query, function (err, rowCount) { @@ -342,7 +345,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 3}; - var pool = new ConnectionPool(poolConfig, {}); + var pool = new ConnectionPool(poolConfig, getConfig()); pool.acquire(function() { }); @@ -351,4 +354,74 @@ describe('ConnectionPool', function () { pool.drain(done); }, 4); }); + + it('request callback should be called on socket error', function (done) { + this.timeout(10000); + + var poolConfig = {min: 1}; + + var connectionConfig = getConfig(); + connectionConfig.options = connectionConfig.options || {}; + connectionConfig.options.requestTimeout = 60000; + + var pool = new ConnectionPool(poolConfig, connectionConfig); + + pool.acquire(function(err, conn) { + var request = new Request("WAITFOR 00:00:30", function(err) { + assert(err); + assert.equal(err.message, 'socket error'); + pool.drain(done); + }); + conn.execSql(request); + conn.socket.emit('error', new Error('socket error')); + }); + pool.on('error', function(err) { + assert.equal(err.code, 'ESOCKET'); + }); + }); + + it('connection should still work after socket error', function (done) { + this.timeout(10000); + + var poolConfig = {min: 1, max: 1}; + + var connectionConfig = getConfig(); + connectionConfig.options = connectionConfig.options || {}; + connectionConfig.options.requestTimeout = 60000; + + var pool = new ConnectionPool(poolConfig, connectionConfig); + + var createRequest = function(query, value, callback) { + var request = new Request(query, function (err, rowCount) { + assert.strictEqual(rowCount, 1); + callback(); + }); + + request.on('row', function (columns) { + assert.strictEqual(columns[0].value, value); + }); + + return request; + }; + + pool.acquire(function(err, conn) { + var request = new Request("WAITFOR 00:00:30", function(err) { + assert(err); + assert.equal(err.message, 'socket error'); + conn.release(); + pool.acquire(function(err, conn) { + assert(!err); + conn.execSql(createRequest('SELECT 42', 42, function () { + pool.drain(done); + })); + }); + }); + conn.execSql(request); + conn.socket.emit('error', new Error('socket error')); + }); + pool.on('error', function(err) { + assert.equal(err.code, 'ESOCKET'); + }); + }); + }); From 5d0ad400b304b259d63b378c077105ac1fc13f2c Mon Sep 17 00:00:00 2001 From: Ben Page Date: Tue, 27 Oct 2015 14:40:03 -0500 Subject: [PATCH 2/2] fixed up tests --- package.json | 2 +- test/connection-pool.test.js | 111 +++++++---------------------------- 2 files changed, 21 insertions(+), 92 deletions(-) diff --git a/package.json b/package.json index d8826ef..b5adeef 100755 --- a/package.json +++ b/package.json @@ -37,7 +37,7 @@ ], "licenses": "MIT", "dependencies": { - "tedious": "^1.8.0" + "tedious": "^1.13.0" }, "devDependencies": { "mocha": ">=1.6.0" diff --git a/test/connection-pool.test.js b/test/connection-pool.test.js index 2478f2e..a133340 100755 --- a/test/connection-pool.test.js +++ b/test/connection-pool.test.js @@ -1,12 +1,16 @@ var assert = require('assert'); var Request = require('tedious').Request; -var fs = require('fs'); var ConnectionPool = require('../lib/connection-pool'); -function getConfig() { - var config = JSON.parse(fs.readFileSync(process.env.HOME + '/.tedious/test-connection-pool.json', 'utf8')).config; - return config; -} +var connectionConfig = { + userName: 'test', + password: 'test', + server: 'dev1', + options: { + appName: 'pool-test', + database: 'test' + } +}; /* create a db user with the correct permissions: CREATE DATABASE test @@ -33,7 +37,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 2}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { assert.equal(pool.connections.length, poolConfig.min); @@ -45,7 +49,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 0, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { assert.equal(pool.connections.length, 0); @@ -77,7 +81,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 2, max: 5}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); var count = 20; var run = 0; @@ -116,7 +120,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = { min: 5, max: 1, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { assert.equal(pool.connections.length, 1); @@ -148,7 +152,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {max: 1, idleTimeout: 10}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { assert.equal(pool.connections.length, 1); @@ -196,7 +200,7 @@ describe('ConnectionPool', function () { pool.on('error', function(err) { assert(!!err); - pool.connectionConfig = getConfig(); + pool.connectionConfig = connectionConfig; }); function testConnected() { @@ -218,7 +222,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 1, max: 1, acquireTimeout: 2000}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); pool.acquire(function(err, connection) { assert(!err); @@ -235,7 +239,7 @@ describe('ConnectionPool', function () { it('idle timeout', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5, idleTimeout: 100}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); setTimeout(function() { pool.acquire(function (err, connection) { @@ -256,13 +260,9 @@ describe('ConnectionPool', function () { }, 300); }); - it('lost connection handling', function (done) { + it('connection error handling', function (done) { this.timeout(10000); var poolConfig = {min: 1, max: 5}; - var connectionConfig = getConfig(); - - connectionConfig.options = connectionConfig.options || {}; - connectionConfig.options.requestTimeout = 5000; var pool = new ConnectionPool(poolConfig, connectionConfig); @@ -292,7 +292,6 @@ describe('ConnectionPool', function () { var request = new Request(command, function (err, rowCount) { assert(err); - assert(err.code,'ETIMEOUT'); pool.drain(done); }); @@ -308,7 +307,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {max: 1}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); var createRequest = function(query, value, callback) { var request = new Request(query, function (err, rowCount) { @@ -345,7 +344,7 @@ describe('ConnectionPool', function () { this.timeout(10000); var poolConfig = {min: 3}; - var pool = new ConnectionPool(poolConfig, getConfig()); + var pool = new ConnectionPool(poolConfig, connectionConfig); pool.acquire(function() { }); @@ -354,74 +353,4 @@ describe('ConnectionPool', function () { pool.drain(done); }, 4); }); - - it('request callback should be called on socket error', function (done) { - this.timeout(10000); - - var poolConfig = {min: 1}; - - var connectionConfig = getConfig(); - connectionConfig.options = connectionConfig.options || {}; - connectionConfig.options.requestTimeout = 60000; - - var pool = new ConnectionPool(poolConfig, connectionConfig); - - pool.acquire(function(err, conn) { - var request = new Request("WAITFOR 00:00:30", function(err) { - assert(err); - assert.equal(err.message, 'socket error'); - pool.drain(done); - }); - conn.execSql(request); - conn.socket.emit('error', new Error('socket error')); - }); - pool.on('error', function(err) { - assert.equal(err.code, 'ESOCKET'); - }); - }); - - it('connection should still work after socket error', function (done) { - this.timeout(10000); - - var poolConfig = {min: 1, max: 1}; - - var connectionConfig = getConfig(); - connectionConfig.options = connectionConfig.options || {}; - connectionConfig.options.requestTimeout = 60000; - - var pool = new ConnectionPool(poolConfig, connectionConfig); - - var createRequest = function(query, value, callback) { - var request = new Request(query, function (err, rowCount) { - assert.strictEqual(rowCount, 1); - callback(); - }); - - request.on('row', function (columns) { - assert.strictEqual(columns[0].value, value); - }); - - return request; - }; - - pool.acquire(function(err, conn) { - var request = new Request("WAITFOR 00:00:30", function(err) { - assert(err); - assert.equal(err.message, 'socket error'); - conn.release(); - pool.acquire(function(err, conn) { - assert(!err); - conn.execSql(createRequest('SELECT 42', 42, function () { - pool.drain(done); - })); - }); - }); - conn.execSql(request); - conn.socket.emit('error', new Error('socket error')); - }); - pool.on('error', function(err) { - assert.equal(err.code, 'ESOCKET'); - }); - }); - });