Merge pull request #1221 from sequelize/hotfix/transaction-promise

Fix/Tests for #1218

lib/sequelize.js

@@ -418,12 +418,19 @@ module.exports = (function() {
   Sequelize.prototype.transaction = function(_options, _callback) {
   Sequelize.prototype.transaction = function(_options, _callback) {
     var options     = (typeof _options === 'function') ? {} : _options
     var options     = (typeof _options === 'function') ? {} : _options
       , callback    = (typeof _options === 'function') ? _options : _callback
       , callback    = (typeof _options === 'function') ? _options : _callback
+      , wantsError  = (callback.length === 2)
       , transaction = new Transaction(this, options)
       , transaction = new Transaction(this, options)
       , self        = this
       , self        = this
 
 
     Utils.tick(function() {
     Utils.tick(function() {
+      if (wantsError) {
+        transaction.error(function(err) {
+          callback(err, transaction)
+        })
+      }
+
       transaction.prepareEnvironment(function() {
       transaction.prepareEnvironment(function() {
-        callback(transaction)
+        wantsError ? callback(null, transaction) : callback(transaction)
       })
       })
     })
     })
 
 

lib/transaction.js

@@ -55,7 +55,7 @@ Transaction.prototype.begin = function(callback) {
     .getQueryInterface()
     .getQueryInterface()
     .startTransaction(this, {})
     .startTransaction(this, {})
     .success(callback)
     .success(callback)
-
+    .error(onError.bind(this))
 }
 }
 
 
 Transaction.prototype.setAutocommit = function(callback) {
 Transaction.prototype.setAutocommit = function(callback) {
@@ -64,6 +64,7 @@ Transaction.prototype.setAutocommit = function(callback) {
     .getQueryInterface()
     .getQueryInterface()
     .setAutocommit(this, this.options.autocommit)
     .setAutocommit(this, this.options.autocommit)
     .success(callback)
     .success(callback)
+    .error(onError.bind(this))
 }
 }
 
 
 Transaction.prototype.setIsolationLevel = function(callback) {
 Transaction.prototype.setIsolationLevel = function(callback) {
@@ -72,5 +73,11 @@ Transaction.prototype.setIsolationLevel = function(callback) {
     .getQueryInterface()
     .getQueryInterface()
     .setIsolationLevel(this, this.options.isolationLevel)
     .setIsolationLevel(this, this.options.isolationLevel)
     .success(callback)
     .success(callback)
-    .error(function(err) { console.log(err) })
+    .error(onError.bind(this))
+}
+
+// private
+
+var onError = function(err) {
+  this.emit('error', err)
 }
 }

test/sequelize.transaction.test.js

+var chai        = require('chai')
+  , expect      = chai.expect
+  , Support     = require(__dirname + '/support')
+  , Transaction = require(__dirname + '/../lib/transaction')
+
+describe(Support.getTestDialectTeaser("Sequelize#transaction"), function () {
+  describe('success', function() {
+    it("gets triggered once a transaction has been successfully committed", function(done) {
+      this
+        .sequelize
+        .transaction(function(t) { t.commit() })
+        .success(function() { done() })
+    })
+
+    it("gets triggered once a transaction has been successfully rollbacked", function(done) {
+      this
+        .sequelize
+        .transaction(function(t) { t.rollback() })
+        .success(function() { done() })
+    })
+  })
+
+  describe('error', function() {
+    if (Support.getTestDialect() === 'sqlite') {
+      // not sure if we can test this in sqlite ...
+      // how could we enforce an authentication error in sqlite?
+    } else {
+      it("gets triggered once an error occurs", function(done) {
+        var sequelize = Support.createSequelizeInstance({ dialect: Support.getTestDialect() })
+
+        // lets overwrite the host to get an error
+        sequelize.config.username = 'foobarbaz'
+
+        sequelize
+          .transaction(function() {})
+          .error(function(err) {
+            expect(err).to.not.be.undefined
+            done()
+          })
+      })
+    }
+  })
+
+  describe('callback', function() {
+    it("receives the transaction if only one argument is passed", function(done) {
+      this.sequelize.transaction(function(t) {
+        expect(t).to.be.instanceOf(Transaction)
+        t.commit()
+      }).done(done)
+    })
+
+    it("receives an error and the transaction if two arguments are passed", function(done) {
+      this.sequelize.transaction(function(err, t) {
+        expect(err).to.not.be.instanceOf(Transaction)
+        expect(t).to.be.instanceOf(Transaction)
+        t.commit()
+      }).done(done)
+    })
+  })
+})

test/transaction.test.js

@@ -16,14 +16,26 @@ describe(Support.getTestDialectTeaser("Transaction"), function () {
     })
     })
   })
   })
 
 
+  describe('success', function() {
+    it("is a success method available", function() {
+      expect(Transaction).to.respondTo("success")
+    })
+  })
+
+  describe('error', function() {
+    it("is an error method available", function() {
+      expect(Transaction).to.respondTo("error")
+    })
+  })
+
   describe('commit', function() {
   describe('commit', function() {
-    it('is a commit message available', function() {
+    it('is a commit method available', function() {
       expect(Transaction).to.respondTo('commit')
       expect(Transaction).to.respondTo('commit')
     })
     })
   })
   })
 
 
   describe('rollback', function() {
   describe('rollback', function() {
-    it('is a rollback message available', function() {
+    it('is a rollback method available', function() {
       expect(Transaction).to.respondTo('rollback')
       expect(Transaction).to.respondTo('rollback')
     })
     })
   })
   })