Merge pull request #4265 from jgonggrijp/wait-error-fix

Trigger error from failing Collection.create with wait:true

backbone.js

@@ -1084,9 +1084,23 @@
       var collection = this;
       var success = options.success;
       options.success = function(m, resp, callbackOpts) {
-        if (wait) collection.add(m, callbackOpts);
+        if (wait) {
+          m.off('error', this._forwardPristineError, this);
+          collection.add(m, callbackOpts);
+        }
         if (success) success.call(callbackOpts.context, m, resp, callbackOpts);
       };
+      // In case of wait:true, our collection is not listening to any
+      // of the model's events yet, so it will not forward the error
+      // event. In this special case, we need to listen for it
+      // separately and handle the event just once.
+      // (The reason we don't need to do this for the sync event is
+      // in the success handler above: we add the model first, which
+      // causes the collection to listen, and then invoke the callback
+      // that triggers the event.)
+      if (wait) {
+        model.once('error', this._forwardPristineError, this);
+      }
       model.save(null, options);
       return model;
     },
@@ -1224,8 +1238,19 @@
         }
       }
       this.trigger.apply(this, arguments);
-    }
+    },
 
+    // Internal callback method used in `create`. It serves as a
+    // stand-in for the `_onModelEvent` method, which is not yet bound
+    // during the `wait` period of the `create` call. We still want to
+    // forward any `'error'` event at the end of the `wait` period,
+    // hence a customized callback.
+    _forwardPristineError: function(model, collection, options) {
+      // Prevent double forward if the model was already in the
+      // collection before the call to `create`.
+      if (this.has(model)) return;
+      this._onModelEvent('error', model, collection, options);
+    }
   });
 
   // Defining an @@iterator method implements JavaScript's Iterable protocol.

test/collection.js

@@ -654,6 +654,47 @@
     assert.equal(collection.length, 1);
   });
 
+  QUnit.test('failing create with wait:true triggers error event (#4262)', function(assert) {
+    assert.expect(2);
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
+    collection.on('error', function() { assert.ok(true); });
+    var model = collection.create({id: '1'}, {wait: true});
+    model.on('error', function() { assert.ok(true); });
+    this.ajaxSettings.error();
+  });
+
+  QUnit.test('successful create with wait:true triggers success event (#4262)', function(assert) {
+    assert.expect(2);
+    var collection = new Backbone.Collection;
+    collection.url = '/test';
+    collection.on('sync', function() { assert.ok(true); });
+    var model = collection.create({id: '1'}, {wait: true});
+    model.on('sync', function() { assert.ok(true); });
+    this.ajaxSettings.success();
+  });
+
+  QUnit.test('failing create pre-existing with wait:true triggers once (#4262)', function(assert) {
+    assert.expect(1);
+    var model = new Backbone.Model;
+    var collection = new Backbone.Collection([model]);
+    collection.url = '/test';
+    collection.on('error', function() { assert.ok(true); });
+    collection.create(model, {wait: true});
+    this.ajaxSettings.error();
+  });
+
+  QUnit.test('successful create pre-existing with wait:true preserves other error bindings (#4262)', function(assert) {
+    assert.expect(1);
+    var model = new Backbone.Model;
+    var collection = new Backbone.Collection([model]);
+    collection.url = '/test';
+    model.on('error', function() { assert.ok(true); });
+    collection.create(model, {wait: true});
+    this.ajaxSettings.success();
+    model.trigger('error');
+  });
+
   QUnit.test('initialize', function(assert) {
     assert.expect(1);
     var Collection = Backbone.Collection.extend({

test/model.js

@@ -712,6 +712,15 @@
     this.ajaxSettings.success();
   });
 
+  QUnit.test('failing save with wait:true triggers error event (#4262)', function(assert) {
+    assert.expect(1);
+    var model = new Backbone.Model;
+    model.urlRoot = '/test';
+    model.on('error', function() { assert.ok(true); });
+    model.save({id: '1'}, {wait: true});
+    this.ajaxSettings.error();
+  });
+
   QUnit.test('fetch', function(assert) {
     assert.expect(2);
     doc.fetch();