diff --git a/lib/client/doc.js b/lib/client/doc.js index 7bb47c33f..ad4b7aefc 100644 --- a/lib/client/doc.js +++ b/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); @@ -977,48 +973,49 @@ Doc.prototype._opAcknowledged = function(message) { this._clearInflightOp(); }; -Doc.prototype._rollback = function(err) { +Doc.prototype._rollback = function(error) { // The server has rejected submission of the current operation. Invert by // just the inflight op if possible. If not possible to invert, cancel all // pending ops and fetch the latest from the server to get us back into a // working state, then call back var op = this.inflightOp; - if ('op' in op && op.type.invert) { - try { - op.op = op.type.invert(op.op); - } catch (error) { - // If the op doesn't support `.invert()`, we just reload the doc - // instead of trying to locally revert it. - return this._hardRollback(err); - } + if (!('op' in op && op.type.invert)) { + return this._hardRollback(error); + } - // Transform the undo operation by any pending ops. - for (var i = 0; i < this.pendingOps.length; i++) { - var transformErr = transformX(this.pendingOps[i], op); - if (transformErr) return this._hardRollback(transformErr); - } + try { + op.op = op.type.invert(op.op); + } catch (invertOpError) { + logger.info('Cannot invert op', op, invertOpError); + // If the op doesn't support `.invert()`, we just reload the doc + // instead of trying to locally revert it. + return this._hardRollback(error); + } - // ... and apply it locally, reverting the changes. - // - // This operation is applied to look like it comes from a remote source. - // I'm still not 100% sure about this functionality, because its really a - // local op. Basically, the problem is that if the client's op is rejected - // by the server, the editor window should update to reflect the undo. - try { - this._otApply(op, false); - } catch (error) { - return this._hardRollback(error); - } + // Transform the undo operation by any pending ops. + for (var i = 0; i < this.pendingOps.length; i++) { + var transformErr = transformX(this.pendingOps[i], op); + if (transformErr) return this._hardRollback(transformErr); + } - this._clearInflightOp(err); - return; + // ... and apply it locally, reverting the changes. + // + // This operation is applied to look like it comes from a remote source. + // I'm still not 100% sure about this functionality, because its really a + // local op. Basically, the problem is that if the client's op is rejected + // by the server, the editor window should update to reflect the undo. + try { + this._otApply(op, false); + } catch (otApplyError) { + logger.info('Cannot apply op in rollback.', op, otApplyError, 'Original error', error); + return this._hardRollback(otApplyError); } - this._hardRollback(err); + this._clearInflightOp(error); }; -Doc.prototype._hardRollback = function(err) { +Doc.prototype._hardRollback = function(error) { // Store pending ops so that we can notify their callbacks of the error. // We combine the inflight op and the pending ops, because it's possible // to hit a condition where we have no inflight op, but we do have pending @@ -1036,17 +1033,23 @@ 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) { + logger.error('Hard rollback doc fetch failed.', fetchError, doc); + doc.emit('error', fetchError); + } + 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, error) && allOpsHadCallbacks; } - if (err && !allOpsHadCallbacks) return doc.emit('error', err); + if (error && !allOpsHadCallbacks) return doc.emit('error', error); }); }; diff --git a/test/client/presence/doc-presence.js b/test/client/presence/doc-presence.js index 1011ac74b..ef1fcef48 100644 --- a/test/client/presence/doc-presence.js +++ b/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); }); diff --git a/test/client/submit.js b/test/client/submit.js index 37716dc41..3f31f8a21 100644 --- a/test/client/submit.js +++ b/test/client/submit.js @@ -919,22 +919,25 @@ module.exports = function() { ], done); }); - it('request.rejectedError() soft rejects a create', function(done) { + it('request.rejectedError() rejects a create', function(done) { this.backend.use('submit', function(request, next) { next(request.rejectedError()); }); var doc = this.backend.connect().get('dogs', 'fido'); doc.create({age: 3}, function(err) { - if (err) return done(err); + if (!err) { + return done('Should emit error'); + } expect(doc.version).equal(0); expect(doc.data).equal(undefined); + expect(err.code).to.be.equal('ERR_OP_SUBMIT_REJECTED'); done(); }); expect(doc.version).equal(null); expect(doc.data).eql({age: 3}); }); - it('request.rejectedError() soft rejects a create without callback', function(done) { + it('request.rejectedError() rejects a create without callback', function(done) { this.backend.use('submit', function(request, next) { next(request.rejectedError()); }); @@ -942,9 +945,8 @@ module.exports = function() { doc.create({age: 3}); expect(doc.version).equal(null); expect(doc.data).eql({age: 3}); - doc.whenNothingPending(function() { - expect(doc.version).equal(0); - expect(doc.data).equal(undefined); + doc.on('error', function(err) { + expect(err.code).to.be.equal('ERR_OP_SUBMIT_REJECTED'); done(); }); }); @@ -1024,7 +1026,7 @@ module.exports = function() { }); }); - it('request.rejectedError() soft rejects an op', function(done) { + it('request.rejectedError() rejects an op', function(done) { this.backend.use('submit', function(request, next) { if ('op' in request.op) return next(request.rejectedError()); next(); @@ -1033,9 +1035,11 @@ module.exports = function() { doc.create({age: 3}, function(err) { if (err) return done(err); doc.submitOp({p: ['age'], na: 1}, function(err) { - if (err) return done(err); - expect(doc.version).equal(1); - expect(doc.data).eql({age: 3}); + if (!err) { + return done('Should throw an error'); + } + + expect(err.code).to.be.equal('ERR_OP_SUBMIT_REJECTED'); done(); }); expect(doc.version).equal(1); @@ -1043,7 +1047,7 @@ module.exports = function() { }); }); - it('request.rejectedError() soft rejects an op without callback', function(done) { + it('request.rejectedError() rejects an op without callback', function(done) { this.backend.use('submit', function(request, next) { if ('op' in request.op) return next(request.rejectedError()); next(); @@ -1051,14 +1055,13 @@ module.exports = function() { var doc = this.backend.connect().get('dogs', 'fido'); doc.create({age: 3}, function(err) { if (err) return done(err); - doc.submitOp({p: ['age'], na: 1}); - expect(doc.version).equal(1); - expect(doc.data).eql({age: 4}); - doc.whenNothingPending(function() { - expect(doc.version).equal(1); - expect(doc.data).eql({age: 3}); + doc.on('error', function(err) { + if (!err) return done('Should throw an error'); + + expect(err.code).to.be.equal('ERR_OP_SUBMIT_REJECTED'); done(); }); + doc.submitOp({p: ['age'], na: 1}); }); });