fix(transaction): fixed unhandled rejection when connection acquire timeout (#9879)

lib/dialects/abstract/connection-manager.js

@@ -4,6 +4,7 @@ const Pooling = require('generic-pool');
 const _ = require('lodash');
 const _ = require('lodash');
 const semver = require('semver');
 const semver = require('semver');
 const Promise = require('../../promise');
 const Promise = require('../../promise');
+const errors = require('../../errors');
 const logger = require('../../utils/logger');
 const logger = require('../../utils/logger');
 const debug = logger.getLogger().debugContext('pool');
 const debug = logger.getLogger().debugContext('pool');
 
 
@@ -151,11 +152,9 @@ class ConnectionManager {
       acquire: (priority, queryType, useMaster) => {
       acquire: (priority, queryType, useMaster) => {
         useMaster = _.isUndefined(useMaster) ? false : useMaster;
         useMaster = _.isUndefined(useMaster) ? false : useMaster;
         if (queryType === 'SELECT' && !useMaster) {
         if (queryType === 'SELECT' && !useMaster) {
-          return this.pool.read.acquire(priority)
-            .then(mayBeConnection => this._determineConnection(mayBeConnection));
+          return this.pool.read.acquire(priority);
         } else {
         } else {
-          return this.pool.write.acquire(priority)
-            .then(mayBeConnection => this._determineConnection(mayBeConnection));
+          return this.pool.write.acquire(priority);
         }
         }
       },
       },
       destroy: mayBeConnection => {
       destroy: mayBeConnection => {
@@ -273,6 +272,7 @@ class ConnectionManager {
     return promise.then(() => {
     return promise.then(() => {
       return this.pool.acquire(options.priority, options.type, options.useMaster)
       return this.pool.acquire(options.priority, options.type, options.useMaster)
         .then(mayBeConnection => this._determineConnection(mayBeConnection))
         .then(mayBeConnection => this._determineConnection(mayBeConnection))
+        .catch({name: 'TimeoutError'}, err => { throw new errors.ConnectionAcquireTimeoutError(err); })
         .tap(() => { debug('connection acquired'); });
         .tap(() => { debug('connection acquired'); });
     });
     });
   }
   }

lib/errors/index.js

@@ -509,6 +509,20 @@ class ConnectionTimedOutError extends ConnectionError {
 exports.ConnectionTimedOutError = ConnectionTimedOutError;
 exports.ConnectionTimedOutError = ConnectionTimedOutError;
 
 
 /**
 /**
+ * Thrown when connection is not acquired due to timeout
+ *
+ * @extends ConnectionError
+ */
+class ConnectionAcquireTimeoutError extends ConnectionError {
+  constructor(parent) {
+    super(parent);
+    this.name = 'SequelizeConnectionAcquireTimeoutError';
+    Error.captureStackTrace(this, this.constructor);
+  }
+}
+exports.ConnectionAcquireTimeoutError = ConnectionAcquireTimeoutError;
+
+/**
  * Thrown when a some problem occurred with Instance methods (see message for details)
  * Thrown when a some problem occurred with Instance methods (see message for details)
  *
  *
  * @extends BaseError
  * @extends BaseError

lib/transaction.js

@@ -91,6 +91,10 @@ class Transaction {
       return Promise.reject(new Error('Transaction cannot be rolled back because it has been finished with state: ' + this.finished));
       return Promise.reject(new Error('Transaction cannot be rolled back because it has been finished with state: ' + this.finished));
     }
     }
 
 
+    if (!this.connection) {
+      return Promise.reject(new Error('Transaction cannot be rolled back because it never started'));
+    }
+
     this._clearCls();
     this._clearCls();
 
 
     return this
     return this

test/integration/pool.test.js

@@ -18,7 +18,7 @@ describe(Support.getTestDialectTeaser('Pooling'), function() {
     this.sinon.restore();
     this.sinon.restore();
   });
   });
 
 
-  it('should reject when unable to acquire connection in given time', () => {
+  it('should reject with ConnectionAcquireTimeoutError when unable to acquire connection in given time', () => {
     this.testInstance = new Sequelize('localhost', 'ffd', 'dfdf', {
     this.testInstance = new Sequelize('localhost', 'ffd', 'dfdf', {
       dialect,
       dialect,
       databaseVersion: '1.2.3',
       databaseVersion: '1.2.3',
@@ -31,7 +31,7 @@ describe(Support.getTestDialectTeaser('Pooling'), function() {
       .returns(new Sequelize.Promise(() => {}));
       .returns(new Sequelize.Promise(() => {}));
 
 
     return expect(this.testInstance.authenticate())
     return expect(this.testInstance.authenticate())
-      .to.eventually.be.rejectedWith('ResourceRequest timed out');
+      .to.eventually.be.rejectedWith(Sequelize.ConnectionAcquireTimeoutError);
   });
   });
 
 
   it('should not result in unhandled promise rejection when unable to acquire connection', () => {
   it('should not result in unhandled promise rejection when unable to acquire connection', () => {
@@ -47,8 +47,8 @@ describe(Support.getTestDialectTeaser('Pooling'), function() {
     this.sinon.stub(this.testInstance.connectionManager, '_connect')
     this.sinon.stub(this.testInstance.connectionManager, '_connect')
       .returns(new Sequelize.Promise(() => {}));
       .returns(new Sequelize.Promise(() => {}));
 
 
-    return expect(this.testInstance.transaction()
-      .then(() => this.testInstance.transaction()))
-      .to.eventually.be.rejectedWith('ResourceRequest timed out');
+    return expect(this.testInstance.transaction(() => {
+      return this.testInstance.transaction(() => {});
+    })).to.eventually.be.rejectedWith(Sequelize.ConnectionAcquireTimeoutError);
   });
   });
 });
 });

test/integration/transaction.test.js

@@ -187,6 +187,16 @@ if (current.dialect.supports.transactions) {
       ).to.eventually.be.rejected;
       ).to.eventually.be.rejected;
     });
     });
 
 
+    it('should not rollback if connection was not acquired', function() {
+      this.sinon.stub(this.sequelize.connectionManager, '_connect')
+        .returns(new Support.Sequelize.Promise(() => {}));
+
+      const transaction = new Transaction(this.sequelize);
+
+      return expect(transaction.rollback())
+        .to.eventually.be.rejectedWith('Transaction cannot be rolled back because it never started');
+    });
+
     it('does not allow queries immediatly after rollback call', function() {
     it('does not allow queries immediatly after rollback call', function() {
       const self = this;
       const self = this;
       return expect(
       return expect(