feat(promises): properly propagate errors

lib/dao.js

@@ -667,11 +667,9 @@ module.exports = (function() {
         identifier = self.__options.hasPrimaryKeys ? self.primaryKeyValues : { id: self.id };
         query = self.QueryInterface.delete(self, self.QueryInterface.QueryGenerator.addSchema(self.Model), identifier, options)
       }
-      return query
+      return query;
     }).then(function (results) {
-      return self.Model.runHooks(self.Model.options.hooks.afterDestroy, self).then(function () {
-        return results;
-      });
+      return self.Model.runHooks(self.Model.options.hooks.afterDestroy, self).return(results);
     });
   }
 

lib/dialects/abstract/query.js

 var Utils              = require('../../utils')
   , CustomEventEmitter = require("../../emitters/custom-event-emitter")
+  , Promise            = require("../../promise")
   , Dot                = require('dottie')
   , _                  = require('lodash')
   , QueryTypes         = require('../../query-types')

lib/dialects/postgres/connector-manager.js

@@ -47,21 +47,15 @@ module.exports = (function() {
     self.pendingQueries++
     self.clientDrained = false
 
-    return new Utils.CustomEventEmitter(function(emitter) {
-      self.connect()
-      .on('error', function(err) {
-        emitter.emit('error', err)
-      })
-      .on('success', function(done) {
+    return self.connect().then(function(done) {
       var query = new Query(self.client, self.sequelize, callee, options || {})
 
-        return query.run(sql)
-          .complete(function(err) {
-            self.endQuery.call(self)
-            done && done(err) })
-          .proxy(emitter)
-      })
-    }).run()
+      // We return the query regardless of error or success in the query
+      return query.run(sql).finally(function () {
+        self.endQuery.call(self);
+        done && done();
+      });
+    });
   }
 
   ConnectorManager.prototype.afterTransactionSetup = function(callback) {
@@ -70,7 +64,7 @@ module.exports = (function() {
 
   ConnectorManager.prototype.connect = function(callback) {
     var self = this
-    var emitter = new (require('events').EventEmitter)()
+    var emitter = new Utils.Promise();
 
     // in case database is slow to connect, prevent orphaning the client
     // TODO: We really need some sort of queue/flush/drain mechanism

lib/dialects/postgres/query.js

@@ -3,6 +3,7 @@ var Utils         = require("../../utils")
   , DataTypes     = require('../../data-types')
   , hstore        = require('./hstore')
   , QueryTypes    = require('../../query-types')
+  , Promise       = require('../../promise')
 
 module.exports = (function() {
   var Query = function(client, sequelize, callee, options) {
@@ -31,6 +32,9 @@ module.exports = (function() {
       this.sequelize.log('Executing (' + this.options.uuid + '): ' + this.sql)
     }
 
+    return new Promise(function (resolve, reject) {
+      var promise = this;
+
       query.on('row', function(row) {
         rows.push(row)
       })
@@ -38,30 +42,20 @@ module.exports = (function() {
       query.on('error', function(err) {
         receivedError = true
         err.sql = sql
-      self.emit('sql', sql, self.options.uuid)
-      self.emit('error', err, self.callee)
+        promise.emit('sql', sql, self.options.uuid)
+        reject(err);
       })
 
       query.on('end', function(result) {
-      self.emit('sql', self.sql, self.options.uuid)
-
         if (receivedError) {
-        return
+          return;
         }
 
-      onSuccess.call(self, rows, sql, result)
+        promise.emit('sql', self.sql, self.options.uuid)
+        resolve([rows, sql, result]);
       })
-
-    return this
-  }
-
-  Query.prototype.getInsertIdField = function() {
-    return 'id'
-  }
-
-  var onSuccess = function(rows, sql, result) {
+    }).spread(function (rows, sql, result) {
       var results          = rows
-      , self             = this
         , isTableNameQuery = (sql.indexOf('SELECT table_name FROM information_schema.tables') === 0)
         , isRelNameQuery   = (sql.indexOf('SELECT relname FROM pg_class WHERE oid IN') === 0)
 
@@ -76,12 +70,12 @@ module.exports = (function() {
         } else {
           results = rows.map(function(row) { return Utils._.values(row) })
         }
-      return this.emit('success', results)
+        return results
       }
 
-    if (this.send('isSelectQuery')) {
-      if (this.sql.toLowerCase().indexOf('select c.column_name') === 0) {
-        var result = {}
+      if (self.send('isSelectQuery')) {
+        if (self.sql.toLowerCase().indexOf('select c.column_name') === 0) {
+          result = {}
 
           rows.forEach(function(_result) {
             result[_result.Field] = {
@@ -111,12 +105,12 @@ module.exports = (function() {
             }
           })
 
-        this.emit('success', result)
+          return result
         } else {
           // Postgres will treat tables as case-insensitive, so fix the case
           // of the returned values to match attributes
-        if (this.options.raw === false && this.sequelize.options.quoteIdentifiers === false) {
-          var attrsMap = Utils._.reduce(this.callee.attributes, function(m, v, k) { m[k.toLowerCase()] = k; return m}, {})
+          if (self.options.raw === false && self.sequelize.options.quoteIdentifiers === false) {
+            var attrsMap = Utils._.reduce(self.callee.attributes, function(m, v, k) { m[k.toLowerCase()] = k; return m}, {})
             rows.forEach(function(row) {
               Utils._.keys(row).forEach(function(key) {
                 var targetAttr = attrsMap[key]
@@ -130,7 +124,7 @@ module.exports = (function() {
 
           // Parse hstore fields if the model has any hstore fields.
           // This cannot be done in the 'pg' lib because hstore is a UDT.
-        if (!!this.callee && !!this.callee._hasHstoreAttributes) {
+          if (!!self.callee && !!self.callee._hasHstoreAttributes) {
             rows.forEach(function(row) {
               Utils._.keys(row).forEach(function(key) {
                 if (self.callee._isHstoreAttribute(key)) {
@@ -140,29 +134,36 @@ module.exports = (function() {
             })
           }
 
-        this.emit('success', this.send('handleSelectQuery', rows))
+          return  self.send('handleSelectQuery', rows)
         }
-    } else if (this.send('isShowOrDescribeQuery')) {
-      this.emit('success', results)
-    } else if ([QueryTypes.BULKUPDATE, QueryTypes.BULKDELETE].indexOf(this.options.type) !== -1) {
-      this.emit('success', result.rowCount)
-    } else if (this.send('isInsertQuery') || this.send('isUpdateQuery')) {
-      if (this.callee !== null) { // may happen for bulk inserts or bulk updates
+      } else if (self.send('isShowOrDescribeQuery')) {
+        return results
+      } else if ([QueryTypes.BULKUPDATE, QueryTypes.BULKDELETE].indexOf(self.options.type) !== -1) {
+        return result.rowCount
+      } else if (self.send('isInsertQuery') || self.send('isUpdateQuery')) {
+        if (self.callee !== null) { // may happen for bulk inserts or bulk updates
           for (var key in rows[0]) {
             if (rows[0].hasOwnProperty(key)) {
               var record = rows[0][key]
-            if (!!this.callee.Model && !!this.callee.Model.rawAttributes && !!this.callee.Model.rawAttributes[key] && !!this.callee.Model.rawAttributes[key].type && this.callee.Model.rawAttributes[key].type.toString() === DataTypes.HSTORE.toString()) {
+              if (!!self.callee.Model && !!self.callee.Model.rawAttributes && !!self.callee.Model.rawAttributes[key] && !!self.callee.Model.rawAttributes[key].type && self.callee.Model.rawAttributes[key].type.toString() === DataTypes.HSTORE.toString()) {
                 record = hstore.parse(record)
               }
-            this.callee.dataValues[key] = record
+              self.callee.dataValues[key] = record
             }
           }
         }
 
-      this.emit('success', this.callee)
+        return self.callee
       } else {
-      this.emit('success', results)
+        return results
+      }
+    })
+
+    return this
   }
+
+  Query.prototype.getInsertIdField = function() {
+    return 'id'
   }
 
   return Query

lib/emitters/custom-event-emitter.js

 var util           = require("util")
   , EventEmitter   = require("events").EventEmitter
-  , Promise        = require("bluebird")
+  , Promise        = require("../promise")
   , proxyEventKeys = ['success', 'error', 'sql']
   , Utils          = require('../utils')
 

lib/promise.js

@@ -20,7 +20,7 @@ var SequelizePromise = function(resolver) {
   this._boundTo = void 0;
 
   // Intercept the resolver so we can resolve with emit's
-  this._resolveFromResolver(function (resolve, reject) {
+  this._resolveFromResolver(function resolverIntercept(resolve, reject) {
     self.seqResolve = resolve;
     self.seqReject = reject;
 
@@ -55,7 +55,7 @@ SequelizePromise.prototype._then = function (
    * Start of sequelize specific 
    * Needed to transfer sql events accross .then() calls
    */
-  if (this.proxySql && ret.emit) {
+  if (this.proxySql && ret && ret.emit) {
     this.proxySql(ret);
   }
   /*
@@ -81,9 +81,7 @@ SequelizePromise.prototype._settlePromiseAt = function (index) {
 
   if (this.$sql && receiver && receiver.emit) {
     this.$sql.forEach(function (sql) {
-      if (receiver && receiver.emit) {
       receiver.emit('sql', sql);
-      }
     });
   }
 
@@ -109,7 +107,7 @@ SequelizePromise.prototype.emit = function(evt) {
 
   if (evt === 'success') {
     this.seqResolve.apply(this, args);
-  } else if (evt === 'error') {
+  } else if (evt === 'error') {;
     this.seqReject.apply(this, args);
   } else {
     // Needed to transfer sql across .then() calls

lib/query-interface.js

@@ -473,10 +473,12 @@ module.exports = (function() {
   }
 
   QueryInterface.prototype.insert = function(dao, tableName, values, options) {
-    var sql = this.QueryGenerator.insertQuery(tableName, values, dao.Model.rawAttributes)
-    return queryAndEmit.call(this, [sql, dao, options], 'insert', {
-      success: function(obj) { obj.isNewRecord = false }
-    })
+    var sql = this.QueryGenerator.insertQuery(tableName, values, dao.Model.rawAttributes);
+
+    return queryAndEmit.call(this, [sql, dao, options], 'insert').then(function (result) {
+      result.isNewRecord = false;
+      return result;
+    });
   }
 
   QueryInterface.prototype.bulkInsert = function(tableName, records, options, Model) {
@@ -533,7 +535,7 @@ module.exports = (function() {
       }
     }
 
-    var emitter = new Promise();
+    var emitter;
     var tick = 0
     var iterate = function(err, i) {
       if (!!err || i >= cascades.length) {
@@ -574,33 +576,22 @@ module.exports = (function() {
 
     var run = function(err) {
       if (!!err) {
-        return emitter.emit('error', err)
+        return emitter.reject(err);
       }
 
-      var chainer = new Utils.QueryChainer()
-
-      chainer.add(self, 'queryAndEmit', [[sql, dao, options], 'delete'])
-
-      chainer.runSerially()
-      .success(function(results){
-        emitter.query = { sql: sql }
-        emitter.emit('sql', sql)
-        emitter.emit('success', results[1])
-      })
-      .error(function(err) {
-        emitter.query = { sql: sql }
-        emitter.emit('sql', sql)
-        emitter.emit('error', err)
-      })
+      if (emitter) {
+        self.queryAndEmit([sql, dao, options], 'delete').proxy(emitter);
+      } else {
+        return self.queryAndEmit([sql, dao, options], 'delete');
+      }
     }
 
     if (cascades.length > 0) {
+      emitter = new Promise();
       iterate(null, tick)
     } else {
-      run()
+      return run()
     }
-    
-    return emitter;
   }
 
   QueryInterface.prototype.bulkDelete = function(tableName, identifier, options) {
@@ -867,6 +858,7 @@ module.exports = (function() {
     }, options || {})
 
     var execQuery = function(emitter) {
+      var query;
       if (Array.isArray(sqlOrQueryParams)) {
         if (sqlOrQueryParams.length === 1) {
           sqlOrQueryParams.push(null)
@@ -876,11 +868,22 @@ module.exports = (function() {
           sqlOrQueryParams.push(typeof options === "object" ? options : {})
         }
 
-        emitter.query = this.sequelize.query.apply(this.sequelize, sqlOrQueryParams)
+        query = this.sequelize.query.apply(this.sequelize, sqlOrQueryParams)
       } else {
-        emitter.query = this.sequelize.query(sqlOrQueryParams, null, options)
+        query = this.sequelize.query(sqlOrQueryParams, null, options)
+      }
+
+      if (!emitter) {
+        return query.then(function (result) {
+          if (options.success) options.success(result);
+          return result;
+        }/*, function (err) {
+          if (options.error) options.error(err);
+        }*/);
       }
 
+      emitter.query = query;
+
       emitter
         .query
         .success(function(obj) {
@@ -905,7 +908,7 @@ module.exports = (function() {
     if (!!emitter) {
       execQuery(emitter)
     } else {
-      return execQuery(new Promise());
+      return execQuery();
     }
   }
 

lib/sequelize.js

@@ -509,7 +509,7 @@ module.exports = (function() {
       type: (sql.toLowerCase().indexOf('select') === 0) ? QueryTypes.SELECT : false
     })
 
-    return this.transactionManager.query(sql, callee, options)
+    return this.transactionManager.query(sql, callee, options);
   }
 
   /**

lib/utils.js

@@ -606,5 +606,6 @@ Utils.col.prototype.toString = function (queryGenerator, parentModel) {
 }
 
 Utils.CustomEventEmitter = require(__dirname + "/emitters/custom-event-emitter")
+Utils.Promise = require(__dirname + "/promise")
 Utils.QueryChainer = require(__dirname + "/query-chainer")
 Utils.Lingo = require("lingo")

test/associations/belongs-to.test.js

@@ -4,6 +4,7 @@ var chai      = require('chai')
   , Support   = require(__dirname + '/../support')
   , DataTypes = require(__dirname + "/../../lib/data-types")
   , Sequelize = require('../../index')
+  , assert    = require('assert')
 
 chai.config.includeStack = true
 
@@ -359,14 +360,17 @@ describe(Support.getTestDialectTeaser("BelongsTo"), function() {
         User.create({ username: 'foo' }).success(function(user) {
           Task.create({ title: 'task' }).success(function(task) {
             task.setUser(user).success(function() {
-              user.destroy().error(function() {
               // Should fail due to FK restriction
+              user.destroy().then(function () {
+                assert(false);
+              }, function(err) {
+                expect(err).to.be.ok;
                 Task.findAll().success(function(tasks) {
                   expect(tasks).to.have.length(1)
                   done()
                 })
               })
-            })
+            });
           })
         })
       })

test/support.js

@@ -6,9 +6,10 @@ var fs        = require('fs')
   , Config    = require(__dirname + "/config/config")
 
 // Make sure errors get thrown when testing
-Sequelize.Promise.onPossiblyUnhandledRejection(function(e, promise){
+Sequelize.Promise.onPossiblyUnhandledRejection(function(e, promise) {
   throw e;
 });
+Sequelize.Promise.longStackTraces();
 
 var Support = {
   Sequelize: Sequelize,