Skip to content

Commit

Permalink
🐛 Unhandled rejection when submitting ops during hard rollback.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dawidreedsy committed Mar 4, 2025
1 parent ca50594 commit 814e814
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 5 deletions.
25 changes: 20 additions & 5 deletions lib/client/doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ function Doc(connection, collection, id) {
this.pendingFetch = [];
this.pendingSubscribe = [];

this._isInHardRollback = false;

// Whether we think we are subscribed on the server. Synchronously set to
// false on calls to unsubscribe and disconnect. Should never be true when
// this.wantSubscribe is false
Expand Down Expand Up @@ -748,10 +750,18 @@ Doc.prototype._submit = function(op, source, callback) {
// The op contains either op, create, delete, or none of the above (a no-op).
if ('op' in op) {
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 (this._isInHardRollback) {
var err = new ShareDBError(
ERROR_CODE.ERR_DOC_IN_HARD_ROLLBACK,
'Cannot submit op. Document is performing hard rollback. ' + this.collection + '.' + this.id
);
} else {
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);
}
Expand Down Expand Up @@ -1030,6 +1040,7 @@ Doc.prototype._rollback = function(err) {
};

Doc.prototype._hardRollback = function(err) {
this._isInHardRollback = true;
// 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 Down Expand Up @@ -1077,7 +1088,10 @@ Doc.prototype._hardRollback = function(err) {
inflightOp = null;
}

if (!pendingOps.length) return;
if (!pendingOps.length) {
doc._isInHardRollback = false;
return;
}
err = new ShareDBError(
ERROR_CODE.ERR_PENDING_OP_REMOVED_BY_OP_SUBMIT_REJECTED,
'Discarding pending op because of hard rollback during ERR_OP_SUBMIT_REJECTED'
Expand All @@ -1090,6 +1104,7 @@ Doc.prototype._hardRollback = function(err) {
allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks;
}
if (err && !allOpsHadCallbacks) doc.emit('error', err);
doc._isInHardRollback = false;
});
};

Expand Down
1 change: 1 addition & 0 deletions lib/error.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ ShareDBError.CODES = {
ERR_DOC_DOES_NOT_EXIST: 'ERR_DOC_DOES_NOT_EXIST',
ERR_DOC_TYPE_NOT_RECOGNIZED: 'ERR_DOC_TYPE_NOT_RECOGNIZED',
ERR_DOC_WAS_DELETED: 'ERR_DOC_WAS_DELETED',
ERR_DOC_IN_HARD_ROLLBACK: 'ERR_DOC_IN_HARD_ROLLBACK',
ERR_INFLIGHT_OP_MISSING: 'ERR_INFLIGHT_OP_MISSING',
ERR_INGESTED_SNAPSHOT_HAS_NO_VERSION: 'ERR_INGESTED_SNAPSHOT_HAS_NO_VERSION',
ERR_MAX_SUBMIT_RETRIES_EXCEEDED: 'ERR_MAX_SUBMIT_RETRIES_EXCEEDED',
Expand Down
19 changes: 19 additions & 0 deletions test/client/doc.js
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,25 @@ describe('Doc', function() {
}
], done);
});

it('rejects ops with ERR_DOC_IN_HARD_ROLLBACK error when in hard rollback', function(done) {
var backend = this.backend;
var doc = backend.connect().get('dogs', 'snoopy');

async.series([
doc.create.bind(doc, {color: 'red'}),
function (next) {

Check failure on line 646 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Unexpected space before function parentheses

Check failure on line 646 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Unexpected space before function parentheses

Check failure on line 646 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Unexpected space before function parentheses
doc.on('error', function (error) {

Check failure on line 647 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Unexpected space before function parentheses

Check failure on line 647 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Unexpected space before function parentheses

Check failure on line 647 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Unexpected space before function parentheses
expect(error.code).to.be.equal('TEST_ERROR')

Check failure on line 648 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 648 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 648 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
})

Check failure on line 649 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 649 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 649 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
doc._hardRollback(new ShareDBError('TEST_ERROR'));
doc.submitOp({p: ['color'], oi: 'blue', od: 'red'}, function(error) {
expect(error.code).to.be.equal('ERR_DOC_IN_HARD_ROLLBACK')

Check failure on line 652 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 652 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 652 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
next()

Check failure on line 653 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 653 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 653 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
})

Check failure on line 654 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 654 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 654 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
},

Check failure on line 655 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Unexpected trailing comma

Check failure on line 655 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Unexpected trailing comma

Check failure on line 655 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Unexpected trailing comma
], done)

Check failure on line 656 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 18

Missing semicolon

Check failure on line 656 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 20

Missing semicolon

Check failure on line 656 in test/client/doc.js

View workflow job for this annotation

GitHub Actions / Node 22

Missing semicolon
});
});

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

0 comments on commit 814e814

Please sign in to comment.