Skip to content

Commit 2b38f47

Browse files
authored
🐛 Unhandled rejection when submitting ops during hard rollback. (#692)
* 🐛 Unhandled rejection when submitting ops during hard rollback. After doing the steps: 1. Create a doc with rich text type (or any other non irreversible type) 2. Make op submission fail 3. Now in the hard rollback we do `this._setType(null);` 4. If there is any op comming before the hard rollback `fetch` is finished, we get the error ``` Cannot submit op. Document has not been created. ``` as in the `_submit` we do: ```typescript if (!this.type) { var err = new ShareDBError( ERROR_CODE.ERR_DOC_DOES_NOT_EXIST, 'Cannot submit op. Document has not been created. ' + this.collection + '.' + this.id ); if (callback) return callback(err); return this.emit('error', err); } ``` We definitely do not handle this case properly. Possible solutions: 1. Just throw error whenever that happens, which is easy to implement and it is not really breaking. User would be then able to react on the error or just ignore it. 2. Store copy of cannonical snapshot in the doc itself, so that we do not have to do fetch for hard rollback. More difficult to implement and has a side effect of storing the doc twice in the memory. * Update to call _isInHArdRollback at the top of fetch
1 parent 4d95a43 commit 2b38f47

File tree

3 files changed

+37
-4
lines changed

3 files changed

+37
-4
lines changed

lib/client/doc.js

+17-4
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ function Doc(connection, collection, id) {
7171
this.pendingFetch = [];
7272
this.pendingSubscribe = [];
7373

74+
this._isInHardRollback = false;
75+
7476
// Whether we think we are subscribed on the server. Synchronously set to
7577
// false on calls to unsubscribe and disconnect. Should never be true when
7678
// this.wantSubscribe is false
@@ -746,10 +748,18 @@ Doc.prototype._submit = function(op, source, callback) {
746748
// The op contains either op, create, delete, or none of the above (a no-op).
747749
if ('op' in op) {
748750
if (!this.type) {
749-
var err = new ShareDBError(
750-
ERROR_CODE.ERR_DOC_DOES_NOT_EXIST,
751-
'Cannot submit op. Document has not been created. ' + this.collection + '.' + this.id
752-
);
751+
if (this._isInHardRollback) {
752+
var err = new ShareDBError(
753+
ERROR_CODE.ERR_DOC_IN_HARD_ROLLBACK,
754+
'Cannot submit op. Document is performing hard rollback. ' + this.collection + '.' + this.id
755+
);
756+
} else {
757+
var err = new ShareDBError(
758+
ERROR_CODE.ERR_DOC_DOES_NOT_EXIST,
759+
'Cannot submit op. Document has not been created. ' + this.collection + '.' + this.id
760+
);
761+
}
762+
753763
if (callback) return callback(err);
754764
return this.emit('error', err);
755765
}
@@ -1028,6 +1038,7 @@ Doc.prototype._rollback = function(err) {
10281038
};
10291039

10301040
Doc.prototype._hardRollback = function(err) {
1041+
this._isInHardRollback = true;
10311042
// Store pending ops so that we can notify their callbacks of the error.
10321043
// We combine the inflight op and the pending ops, because it's possible
10331044
// to hit a condition where we have no inflight op, but we do have pending
@@ -1045,6 +1056,8 @@ Doc.prototype._hardRollback = function(err) {
10451056
// Fetch the latest version from the server to get us back into a working state
10461057
var doc = this;
10471058
this._fetch({force: true}, function(fetchError) {
1059+
doc._isInHardRollback = false;
1060+
10481061
// We want to check that no errors are swallowed, so we check that:
10491062
// - there are callbacks to call, and
10501063
// - that every single pending op called a callback

lib/error.js

+1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ ShareDBError.CODES = {
3030
ERR_DOC_DOES_NOT_EXIST: 'ERR_DOC_DOES_NOT_EXIST',
3131
ERR_DOC_TYPE_NOT_RECOGNIZED: 'ERR_DOC_TYPE_NOT_RECOGNIZED',
3232
ERR_DOC_WAS_DELETED: 'ERR_DOC_WAS_DELETED',
33+
ERR_DOC_IN_HARD_ROLLBACK: 'ERR_DOC_IN_HARD_ROLLBACK',
3334
ERR_INFLIGHT_OP_MISSING: 'ERR_INFLIGHT_OP_MISSING',
3435
ERR_INGESTED_SNAPSHOT_HAS_NO_VERSION: 'ERR_INGESTED_SNAPSHOT_HAS_NO_VERSION',
3536
ERR_MAX_SUBMIT_RETRIES_EXCEEDED: 'ERR_MAX_SUBMIT_RETRIES_EXCEEDED',

test/client/doc.js

+19
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,25 @@ describe('Doc', function() {
636636
}
637637
], done);
638638
});
639+
640+
it('rejects ops with ERR_DOC_IN_HARD_ROLLBACK error when in hard rollback', function(done) {
641+
var backend = this.backend;
642+
var doc = backend.connect().get('dogs', 'snoopy');
643+
644+
async.series([
645+
doc.create.bind(doc, {color: 'red'}),
646+
function(next) {
647+
doc.on('error', function(error) {
648+
expect(error.code).to.be.equal('TEST_ERROR');
649+
});
650+
doc._hardRollback(new ShareDBError('TEST_ERROR'));
651+
doc.submitOp({p: ['color'], oi: 'blue', od: 'red'}, function(error) {
652+
expect(error.code).to.be.equal('ERR_DOC_IN_HARD_ROLLBACK');
653+
next();
654+
});
655+
}
656+
], done);
657+
});
639658
});
640659

641660
describe('errors on ops that could cause prototype corruption', function() {

0 commit comments

Comments
 (0)