fix(mssql): reject if end was called before connect (#9037)

lib/dialects/mssql/connection-manager.js

@@ -40,42 +40,45 @@ class ConnectionManager extends AbstractConnectionManager {
   }
 
   connect(config) {
-    return new Promise((resolve, reject) => {
-      const connectionConfig = {
-        userName: config.username,
-        password: config.password,
-        server: config.host,
-        options: {
-          port: config.port,
-          database: config.database
-        }
-      };
+    const connectionConfig = {
+      userName: config.username,
+      password: config.password,
+      server: config.host,
+      options: {
+        port: config.port,
+        database: config.database
+      }
+    };
 
-      if (config.dialectOptions) {
-        // only set port if no instance name was provided
-        if (config.dialectOptions.instanceName) {
-          delete connectionConfig.options.port;
-        }
+    if (config.dialectOptions) {
+      // only set port if no instance name was provided
+      if (config.dialectOptions.instanceName) {
+        delete connectionConfig.options.port;
+      }
 
-        // The 'tedious' driver needs domain property to be in the main Connection config object
-        if (config.dialectOptions.domain) {
-          connectionConfig.domain = config.dialectOptions.domain;
-        }
+      // The 'tedious' driver needs domain property to be in the main Connection config object
+      if (config.dialectOptions.domain) {
+        connectionConfig.domain = config.dialectOptions.domain;
+      }
 
-        for (const key of Object.keys(config.dialectOptions)) {
-          connectionConfig.options[key] = config.dialectOptions[key];
-        }
+      for (const key of Object.keys(config.dialectOptions)) {
+        connectionConfig.options[key] = config.dialectOptions[key];
       }
+    }
 
+    return new Promise((resolve, reject) => {
       const connection = new this.lib.Connection(connectionConfig);
-      const connectionLock = new ResourceLock(connection);
       connection.lib = this.lib;
+      const resourceLock = new ResourceLock(connection);
+
+      connection.on('end', () => {
+        reject(new sequelizeErrors.ConnectionError('Connection was closed by remote server'));
+      });
 
       connection.on('connect', err => {
         if (!err) {
           debug('connection acquired');
-          resolve(connectionLock);
-          return;
+          return resolve(resourceLock);
         }
 
         if (!err.code) {
@@ -121,17 +124,21 @@ class ConnectionManager extends AbstractConnectionManager {
           switch (err.code) {
             case 'ESOCKET':
             case 'ECONNRESET':
-              this.pool.destroy(connectionLock)
+              this.pool.destroy(resourceLock)
                 .catch(/Resource not currently part of this pool/, () => {});
           }
         });
       }
-
     });
   }
 
   disconnect(connectionLock) {
-    const connection = connectionLock.unwrap();
+    /**
+     * Abstract connection may try to disconnect raw connection used for fetching version
+     */
+    const connection = connectionLock.unwrap
+      ? connectionLock.unwrap()
+      : connectionLock;
 
     // Dont disconnect a connection that is already disconnected
     if (connection.closed) {
@@ -146,7 +153,13 @@ class ConnectionManager extends AbstractConnectionManager {
   }
 
   validate(connectionLock) {
-    const connection = connectionLock.unwrap();
+    /**
+     * Abstract connection may try to validate raw connection used for fetching version
+     */
+    const connection = connectionLock.unwrap
+      ? connectionLock.unwrap()
+      : connectionLock;
+
     return connection && connection.loggedIn;
   }
 }

test/unit/dialects/mssql/connection-manager.test.js

@@ -6,17 +6,12 @@ const chai = require('chai'),
   Support = require(__dirname + '/../../support'),
   dialect = Support.getTestDialect(),
   tedious = require('tedious'),
-  sinon = require('sinon'),
-  connectionStub = sinon.stub(tedious, 'Connection');
-
-connectionStub.returns({on() {}});
+  sinon = require('sinon');
 
 if (dialect === 'mssql') {
   describe('[MSSQL Specific] Connection Manager', () => {
-    let instance,
-      config;
-    beforeEach(() => {
-      config = {
+    beforeEach(function() {
+      this.config = {
         dialect: 'mssql',
         database: 'none',
         username: 'none',
@@ -28,17 +23,49 @@ if (dialect === 'mssql') {
           domain: 'TEST.COM'
         }
       };
-      instance = new Sequelize(config.database
-        , config.username
-        , config.password
-        , config);
+      this.instance = new Sequelize(
+        this.config.database,
+        this.config.username,
+        this.config.password,
+        this.config
+      );
+
+      this.connectionStub = sinon.stub(tedious, 'Connection');
+    });
+
+    afterEach(function() {
+      this.connectionStub.restore();
     });
 
-    it('connectionManager._connect() Does not delete `domain` from config.dialectOptions',
-      () => {
-        expect(config.dialectOptions.domain).to.equal('TEST.COM');
-        instance.dialect.connectionManager._connect(config);
-        expect(config.dialectOptions.domain).to.equal('TEST.COM');
+    it('connectionManager._connect() does not delete `domain` from config.dialectOptions', function() {
+      this.connectionStub.returns({on(event, cb) {
+        if (event === 'connect') {
+          setTimeout(() => {
+            cb();
+          }, 500);
+        }
+      }});
+
+      expect(this.config.dialectOptions.domain).to.equal('TEST.COM');
+      return this.instance.dialect.connectionManager._connect(this.config).then(() => {
+        expect(this.config.dialectOptions.domain).to.equal('TEST.COM');
       });
+    });
+
+    it('connectionManager._connect() should reject if end was called and connect was not', function() {
+      this.connectionStub.returns({ on(event, cb) {
+        if (event === 'end') {
+          setTimeout(() => {
+            cb();
+          }, 500);
+        }
+      } });
+
+      return this.instance.dialect.connectionManager._connect(this.config)
+        .catch(err => {
+          expect(err.name).to.equal('SequelizeConnectionError');
+          expect(err.parent).to.equal('Connection was closed by remote server');
+        });
+    });
   });
 }