CP

lib/client/doc.js

@@ -329,10 +329,6 @@ Doc.prototype._handleSubscribe = function(error, snapshot) {
 Doc.prototype._handleOp = function(err, message) {
   if (err) {
     if (this.inflightOp) {
-      // The server has rejected submission of the current operation. If we get
-      // an "Op submit rejected" error, this was done intentionally
-      // and we should roll back but not return an error to the user.
-      if (err.code === ERROR_CODE.ERR_OP_SUBMIT_REJECTED) err = null;
       return this._rollback(err);
     }
     return this.emit('error', err);
@@ -1014,6 +1010,13 @@ Doc.prototype._rollback = function(err) {
     return this._hardRollback(error);
   }
 
+  // The server has rejected submission of the current operation. If we get
+  // an "Op submit rejected" error, this was done intentionally
+  // and we should roll back but not return an error to the user.
+  if (err.code === ERROR_CODE.ERR_OP_SUBMIT_REJECTED) {
+    return this._clearInflightOp(null);
+  }
+
   this._clearInflightOp(err);
 };
 
@@ -1023,9 +1026,8 @@ Doc.prototype._hardRollback = function(err) {
   // to hit a condition where we have no inflight op, but we do have pending
   // ops. This can happen when an invalid op is submitted, which causes us
   // to hard rollback before the pending op was flushed.
-  var pendingOps = [];
-  if (this.inflightOp) pendingOps.push(this.inflightOp);
-  pendingOps = pendingOps.concat(this.pendingOps);
+  var pendingOps = this.pendingOps.slice();
+  var inflightOp = this.inflightOp;
 
   // Cancel all pending ops and reset if we can't invert
   this._setType(null);
@@ -1035,17 +1037,53 @@ Doc.prototype._hardRollback = function(err) {
 
   // Fetch the latest version from the server to get us back into a working state
   var doc = this;
-  this._fetch({force: true}, function() {
+  this._fetch({force: true}, function(fetchError) {
     // We want to check that no errors are swallowed, so we check that:
     // - there are callbacks to call, and
     // - that every single pending op called a callback
     // If there are no ops queued, or one of them didn't handle the error,
     // then we emit the error.
+
+    if (fetchError) {
+      // This is critical error as it means that our doc is not in usable state
+      // anymore, we should throw doc error.
+      logger.error('Hard rollback doc fetch failed.', fetchError, doc);
+      doc.emit('error', fetchError);
+    }
+
+    if (err.code !== ERROR_CODE.ERR_OP_SUBMIT_REJECTED) {
+      if (inflightOp) pendingOps.push(inflightOp);
+      var allOpsHadCallbacks = !!pendingOps.length;
+      for (var i = 0; i < pendingOps.length; i++) {
+        allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks;
+      }
+      if (err && !allOpsHadCallbacks) doc.emit('error', err);
+      return;
+    }
+
+    /**
+     * Handle special case of ERR_OP_SUBMIT_REJECTED
+     * This ensures that we resolve the main op callback and reject
+     * all the pending ops. This is hard rollback so all the pending ops will be 
+     * discarded. This will ensure that the user is at least informed about it.
+     * more info: https://github.com/share/sharedb/pull/626
+     */
+    if (inflightOp) {
+      util.callEach(inflightOp.callbacks);
+    }
+
+    if (!pendingOps.length) return;
+
+    var hardRollbackError = new ShareDBError(
+      ERROR_CODE.ERR_OP_PENDING_OP_SUBMIT_REJECTED,
+      'Pending op are present when doing rollback of invertible operation'
+    );
+
     var allOpsHadCallbacks = !!pendingOps.length;
     for (var i = 0; i < pendingOps.length; i++) {
-      allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks;
+      allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, hardRollbackError) && allOpsHadCallbacks;
     }
-    if (err && !allOpsHadCallbacks) return doc.emit('error', err);
+    if (!allOpsHadCallbacks) return doc.emit('error', hardRollbackError);
   });
 };
 
@@ -1061,3 +1099,5 @@ Doc.prototype._clearInflightOp = function(err) {
 
   if (err && !called) return this.emit('error', err);
 };
+
+

lib/error.js

@@ -38,6 +38,11 @@ ShareDBError.CODES = {
   ERR_OP_ALREADY_SUBMITTED: 'ERR_OP_ALREADY_SUBMITTED',
   ERR_OP_NOT_ALLOWED_IN_PROJECTION: 'ERR_OP_NOT_ALLOWED_IN_PROJECTION',
   ERR_OP_SUBMIT_REJECTED: 'ERR_OP_SUBMIT_REJECTED',
+  /**
+   * This may happen if server rejected op with ERR_OP_SUBMIT_REJECTED and the type is
+   * not invertible or there are some pending ops after the create op was rejected with ERR_OP_SUBMIT_REJECTED
+   */
+  ERR_OP_PENDING_OP_SUBMIT_REJECTED: 'ERR_OP_PENDING_OP_SUBMIT_REJECTED',
   ERR_OP_VERSION_MISMATCH_AFTER_TRANSFORM: 'ERR_OP_VERSION_MISMATCH_AFTER_TRANSFORM',
   ERR_OP_VERSION_MISMATCH_DURING_TRANSFORM: 'ERR_OP_VERSION_MISMATCH_DURING_TRANSFORM',
   ERR_OP_VERSION_NEWER_THAN_CURRENT_SNAPSHOT: 'ERR_OP_VERSION_NEWER_THAN_CURRENT_SNAPSHOT',

test/client/doc.js

@@ -6,6 +6,7 @@ var richText = require('rich-text').type;
 var ShareDBError = require('../../lib/error');
 var errorHandler = require('../util').errorHandler;
 var sinon = require('sinon');
+var types = require('../../lib/types');
 
 describe('Doc', function() {
   beforeEach(function() {
@@ -514,6 +515,38 @@ describe('Doc', function() {
       ], done);
     });
 
+    it('throws an error when hard rollback fetch failed', function(done) {
+      var backend = this.backend;
+      doc = this.connection.get('dogs', 'scrappy');
+      types.register(richText);
+
+      async.series([
+        doc.create.bind(doc, {ops: [{insert: 'Scrappy'}]}, 'rich-text'),
+        function(next) {
+          backend.use('reply', function(replyContext, cb) {
+            if (replyContext.request.a !== 'f') return cb();
+            cb({code: 'FETCH_ERROR'});
+          });
+          backend.use('submit', function(_context, cb) {
+            cb(new ShareDBError('SUBMIT_ERROR'));
+          });
+          var nonInvertibleOp = [{insert: 'e'}];
+
+          var count = 0;
+          function expectError(code) {
+            count++;
+            return function(error) {
+              expect(error.code).to.equal(code);
+              count--;
+              if (!count) next();
+            };
+          }
+
+          doc.on('error', expectError('FETCH_ERROR'));
+          doc.submitOp(nonInvertibleOp, expectError('SUBMIT_ERROR'));
+        }
+      ], done);
+    });
 
     it('rescues an irreversible op collision', function(done) {
       // This test case attempts to reconstruct the following corner case, with

test/client/presence/doc-presence.js

@@ -705,7 +705,13 @@ describe('DocPresence', function() {
           });
         });
 
-        doc1.submitOp({index: 5, value: 'ern'}, errorHandler(done));
+        doc1.submitOp({index: 5, value: 'ern'}, errorHandler(function(error) {
+          if (!error) {
+            return done('should throw an error');
+          }
+
+          expect(error.code).to.be.equal('ERR_OP_SUBMIT_REJECTED');
+        }));
       }
     ], done);
   });
@@ -741,7 +747,13 @@ describe('DocPresence', function() {
           });
         });
 
-        doc1.submitOp({index: 5, value: 'ern'}, errorHandler(done));
+        doc1.submitOp({index: 5, value: 'ern'}, errorHandler(function(error) {
+          if (!error) {
+            return done('should throw an error');
+          }
+
+          expect(error.code).to.be.equal('ERR_OP_SUBMIT_REJECTED');
+        }));
       }
     ], done);
   });

test/client/submit.js

@@ -934,7 +934,7 @@ module.exports = function() {
       expect(doc.data).eql({age: 3});
     });
 
-    it('request.rejectedError() soft rejects a create without callback', function(done) {
+    it.only('request.rejectedError() soft rejects a create without callback', function(done) {
       this.backend.use('submit', function(request, next) {
         next(request.rejectedError());
       });