Skip to content

Commit

Permalink
CP
Browse files Browse the repository at this point in the history
  • Loading branch information
@Dawidpol
Dawidpol committed Oct 3, 2023
1 parent 62e4ec5 commit 920b2c0
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 26 deletions.
27 changes: 21 additions & 6 deletions lib/client/doc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -1009,15 +1005,24 @@ Doc.prototype._rollback = function(err) {
// 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 {
console.log('_otApply');
this._otApply(op, false);
} catch (error) {
console.log('_otApply error', error);
return this._hardRollback(error);
}

this._clearInflightOp(err);
// 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) {
console.log(2);
this._clearInflightOp(err);
}
};

Doc.prototype._hardRollback = function(err) {
console.log('Hard rollcback');
// 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
Expand All @@ -1035,14 +1040,24 @@ 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 doc and throw doc error.
logger.error('Hard rollback doc fetch failed.', fetchError, doc);
doc.emit('error', fetchError);
}

var allOpsHadCallbacks = !!pendingOps.length;
console.log('allOpsHadCallbacks', allOpsHadCallbacks);
for (var i = 0; i < pendingOps.length; i++) {
console.log('i', i, pendingOps[i].callbacks, err);
allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks;
}
if (err && !allOpsHadCallbacks) return doc.emit('error', err);
Expand Down
33 changes: 33 additions & 0 deletions test/client/doc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions test/client/presence/doc-presence.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down Expand Up @@ -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);
});
Expand Down
40 changes: 22 additions & 18 deletions test/client/submit.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ types.register(deserializedType.type2);
types.register(numberType.type);

module.exports = function() {
describe('client submit', function() {
describe.only('client submit', function() {
it('can fetch an uncreated doc', function(done) {
var doc = this.backend.connect().get('dogs', 'fido');
expect(doc.data).equal(undefined);
Expand Down Expand Up @@ -919,32 +919,34 @@ 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());
});
var doc = this.backend.connect().get('dogs', 'fido');
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();
});
});
Expand Down Expand Up @@ -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();
Expand All @@ -1033,32 +1035,34 @@ 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});
console.log(1);
if (!err) {
return done('Should throw an error');
}

expect(err.code).to.be.equal('ERR_OP_SUBMIT_REJECTED');
done();
});
expect(doc.version).equal(1);
expect(doc.data).eql({age: 4});
});
});

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();
});
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});
});
});

Expand Down

0 comments on commit 920b2c0

Please sign in to comment.