added handling of connection errors

updated README

README.md

@@ -1,61 +1,85 @@
 # tedious-connection-pool
-A simple connection pool for [tedious](http://github.com/pekim/tedious).
+A connection pool for [tedious](http://github.com/pekim/tedious).
+
+## Installation
+
+    npm install tedious-connection-pool
 
 ## Example
 The only difference from the regular tedious API is how the connection is obtained and released. Once a Connection object has been acquired, the tedious API can be used with the connection as normal.
 
 ```javascript
+
 var ConnectionPool = require('tedious-connection-pool');
+var Request = require('tedious').Request;
+var assert = require('assert');
+
+var poolConfig = {
+    min: 5,
+    max: 10
+};
+
+var connectionConfig = {
+    userName: 'login',
+    password: 'password',
+    server: 'localhost'
+};
 
 var pool = new ConnectionPool(poolConfig, connectionConfig);
 
-pool.acquire(function (err, connection) {
-  if(!err) {
+pool.acquire(function (connection) {
     var request = new Request('select 42', function(err, rowCount) {
-      assert.strictEqual(rowCount, 1);
-
-      // Release the connection back to the pool.
-      connection.release();
+        assert.strictEqual(rowCount, 1);
+
+        connection.release(); // Release the connection back to the pool.
     });
 
     request.on('row', function(columns) {
-      assert.strictEqual(columns[0].value, 42);
+        assert.strictEqual(columns[0].value, 42);
     });
 
     connection.execSql(request);
-  }
+});
+
+pool.on('error', function(err) {
+    assert(!!err);
 });
 ```
 
-When the connection is released it is returned to the pool.
-It is then available to be reused.
+When the connection is released it is returned to the pool and is available to be reused.
 
 ##Class: ConnectionPool
 
 ### new ConnectionPool(poolConfig, connectionConfig)
 
-* `poolConfig` {Object} the configuration for [generic-pool](https://github.com/coopernurse/node-pool) (see link for full list of arguments)
-  * `max` {Number} The maximum number of connections there can be in the pool. Default = `10`
-  * `min` {Number} The minimun of connections there can be in the pool. Default = `0`
-  * `idleTimeoutMillis` {Number} The Number of milliseconds before closing an unused connection. Default = `30000`
+* `poolConfig` {Object} the pool configuration object
+  * `min` {Number} The minimun of connections there can be in the pool. Default = `10`
+  * `max` {Number} The maximum number of connections there can be in the pool. Default = `50`
+  * `idleTimeout` {Number} The number of milliseconds before closing an unused connection. Default = `30000`
+  * `retryDelay` {Number} The number of milliseconds to wait after a connection fails, before trying again. Default = `5000`
 
 * `connectionConfig` {Object} The same configuration that would be used to [create a
   tedious Connection](http://pekim.github.com/tedious/api-connection.html#function_newConnection).
 
 ### connectionPool.acquire(callback)
-
+Acquire a Tedious Connection object from the pool.
 * `callback` {Function} Callback function
-  * `error` {Error Object}
   * `connection` {Object} A [Connection](http://pekim.github.com/tedious/api-connection.html)
 
-### connectionPool.drain(callback)
+### connectionPool.drain()
+Close all pooled connections and stop making new ones. The pool should be discarded after it has been drained.
 
-* `callback` {Function} Callback function
+### connectionPool.error {event}
+The 'error' event is emitted when a connection fails to connect to the SQL Server.
+
+##Class: Connection
+The following method is added to the Tedious [Connection](http://pekim.github.com/tedious/api-connection.html) object.
 
-##Class: PooledConnection
-* An extension of the tedious [Connection](http://pekim.github.com/tedious/api-connection.html) object.
+### Connection.release()
+Release the connect back to the pool to be used again
 
-### pooledConnection.release()
+## Version 0.3.x Breaking Changes
+* The err parameter has been removed from the callback passed to acquire(). Connection errors can happen at many at times other than during acquire(). Subscribe to the 'error' event to be notified of connection errors.
 
 ## Version 0.2.x Breaking Changes
 * To acquire a connection, call on acquire() on a ConnectionPool rather than requestConnection().

lib/connection-pool.js

@@ -42,22 +42,25 @@ function createConnection(pooled) {
         };
     this.connections.push(pooled);
 
+    var handleError = function(err) {
+        self.emit('error', err);
+
+        pooled.status = RETRY;
+        pooled.con = undefined;
+        connection.removeAllListeners('end');
+        connection.close();
+
+        setTimeout(createConnection.bind(self, pooled), self.retryDelay);
+    };
+
     connection.on('connect', function (err) {
         if (self.connections === undefined) { //pool has been drained
             connection.close();
             return;
         }
 
         if (err) {
-            if (EventEmitter.listenerCount(self, 'error'))
-                self.emit('error', err);
-
-            pooled.status = RETRY;
-            pooled.con = undefined;
-            connection.removeAllListeners('end');
-            connection.close();
-
-            setTimeout(createConnection.bind(self, pooled), this.retryDelay);
+            handleError(err);
             return;
         }
 
@@ -68,6 +71,8 @@ function createConnection(pooled) {
             setFree.call(this, pooled);
     });
 
+    connection.on('error', handleError);
+
     connection.on('end', function () {
         if (self.connections === undefined) //pool has been drained
             return;

package.json

@@ -1,6 +1,6 @@
 {
   "name": "tedious-connection-pool",
-  "version": "0.2.4",
+  "version": "0.3.0",
   "description": "Connection Pool for tedious.",
   "main": "lib/connection-pool.js",
   "scripts": {

test/connection-pool.test.js

@@ -7,9 +7,29 @@ var connectionConfig = {
     password: 'test',
     server: 'dev1',
     options: {
-        appName: 'pool-test'
+        appName: 'pool-test',
+        database: 'test'
     }
 };
+/* create a db user with the correct permissions:
+CREATE DATABASE test
+CREATE LOGIN test WITH PASSWORD=N'test', DEFAULT_DATABASE=test, CHECK_POLICY=OFF
+GRANT ALTER ANY CONNECTION TO test
+
+USE test
+CREATE USER test FOR LOGIN test WITH DEFAULT_SCHEMA=dbo
+ALTER ROLE db_owner ADD MEMBER test
+
+USE msdb
+CREATE USER test FOR LOGIN test WITH DEFAULT_SCHEMA=dbo
+ALTER ROLE SQLAgentOperatorRole ADD MEMBER test
+ALTER ROLE SQLAgentReaderRole ADD MEMBER test
+ALTER ROLE SQLAgentUserRole ADD MEMBER test
+*/
+
+/* disable the user when not testing:
+ALTER LOGIN test DISABLE
+*/
 
 describe('ConnectionPool', function () {
     it('min', function (done) {
@@ -34,15 +54,32 @@ describe('ConnectionPool', function () {
         var count = 20;
         var run = 0;
 
-        //run more queries than pooled connections
-        runQueries(pool, count, 200, function() {
-            run++;
-            assert(pool.connections.length <= poolConfig.max);
-            if (run === count) {
-                done();
-                pool.drain();
-            }
-        });
+        var createRequest = function (connection) {
+            var request = new Request('select 42', function (err, rowCount) {
+                assert.strictEqual(rowCount, 1);
+                setTimeout(function() {
+                    run++;
+                    assert(pool.connections.length <= poolConfig.max);
+                    if (run === count) {
+                        done();
+                        pool.drain();
+                    }
+                    connection.release();
+                }, 200);
+            });
+
+            request.on('row', function (columns) {
+                assert.strictEqual(columns[0].value, 42);
+            });
+
+            connection.execSql(request);
+        };
+
+        for (var i = 0; i < count; i++) {
+            setTimeout(function() {
+                pool.acquire(createRequest);
+            })
+        }
     });
 
     it('connection error event', function (done) {
@@ -60,6 +97,7 @@ describe('ConnectionPool', function () {
         this.timeout(10000);
         var poolConfig = {min: 1, max: 5, retryDelay: 5};
         var pool = new ConnectionPool(poolConfig, {});
+
         pool.on('error', function(err) {
             assert(!!err);
             pool.connectionConfig = connectionConfig;
@@ -87,34 +125,62 @@ describe('ConnectionPool', function () {
         var pool = new ConnectionPool(poolConfig, connectionConfig);
 
         setTimeout(function() {
-            runQueries(pool, 1, 0, function() {
-                done();
-                pool.drain();
+            pool.acquire(function (connection) {
+                var request = new Request('select 42', function (err, rowCount) {
+                    assert.strictEqual(rowCount, 1);
+                    done();
+                    pool.drain();
+                });
+
+                request.on('row', function (columns) {
+                    assert.strictEqual(columns[0].value, 42);
+                });
+
+                connection.execSql(request);
             });
+
         }, 300);
     });
-});
 
-function runQueries(pool, count, keepOpen, complete) {
-    var createRequest = function (connection) {
-        var request = new Request('select 42', function (err, rowCount) {
-            assert.strictEqual(rowCount, 1);
-            setTimeout(function() {
-                complete();
-                connection.release();
-            }, keepOpen);
-        });
+    it('lost connection error', function (done) {
+        this.timeout(10000);
+        var poolConfig = {min: 1, max: 5};
+        var pool = new ConnectionPool(poolConfig, connectionConfig);
 
-        request.on('row', function (columns) {
-            assert.strictEqual(columns[0].value, 42);
+        pool.on('error', function(err) {
+            assert(err && err.name === 'ConnectionError');
+            done();
+            pool.drain();
         });
 
-        connection.execSql(request);
-    };
+        //This simulates a lost connections by creating a job that kills the current session and then deleting the job.
+        //The user must have the SQLAgentOperatorRole permission on the msdb database and ALTER ANY CONNECTION on master
+        pool.acquire(function (connection) {
+            var command = 'DECLARE @jobName VARCHAR(68) = \'pool\' + CONVERT(VARCHAR(64),NEWID()), @jobId UNIQUEIDENTIFIER;' +
+            'EXECUTE msdb..sp_add_job @jobName, @owner_login_name=\'' + connectionConfig.userName + '\', @job_id=@jobId OUTPUT;' +
+            'EXECUTE msdb..sp_add_jobserver @job_id=@jobId;' +
 
-    for (var i = 0; i < count; i++) {
-        setTimeout(function() {
-            pool.acquire(createRequest);
-        })
-    }
-}
+            'DECLARE @cmd VARCHAR(50);' +
+            'SELECT @cmd = \'kill \' + CONVERT(VARCHAR(10), @@SPID);' +
+            'EXECUTE msdb..sp_add_jobstep @job_id=@jobId, @step_name=\'Step1\', @command = @cmd, @database_name = \'' + connectionConfig.options.database + '\', @on_success_action = 3;' +
+
+            'DECLARE @deleteCommand varchar(200);' +
+            'SET @deleteCommand = \'execute msdb..sp_delete_job @job_name=\'\'\'+@jobName+\'\'\'\';' +
+            'EXECUTE msdb..sp_add_jobstep @job_id=@jobId, @step_name=\'Step2\', @command = @deletecommand;' +
+
+            'EXECUTE msdb..sp_start_job @job_id=@jobId;' +
+            'WAITFOR DELAY \'00:00:10\';' +
+            'SELECT 42';
+
+            var request = new Request(command, function (err, rowCount) {
+                assert(false);
+            });
+
+            request.on('row', function (columns) {
+                assert(false);
+            });
+
+            connection.execSql(request);
+        });
+    });
+});