From 6f3869c660246b8ae8ba09b390875d5285a9c60f Mon Sep 17 00:00:00 2001 From: Dawid Kisielwski <91346346+dawidreedsy@users.noreply.github.com> Date: Mon, 3 Mar 2025 16:43:42 +0100 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=90=9B=20Unhandled=20rejection=20when?= =?UTF-8?q?=20submitting=20ops=20during=20hard=20rollback.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- lib/client/doc.js | 25 ++++++++++++++++++++----- lib/error.js | 1 + test/client/doc.js | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index acb457374..7a1e7d0b9 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -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 @@ -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); } @@ -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 @@ -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' @@ -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; }); }; diff --git a/lib/error.js b/lib/error.js index cde95bdbe..85750104d 100644 --- a/lib/error.js +++ b/lib/error.js @@ -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', diff --git a/test/client/doc.js b/test/client/doc.js index 4156ab4a1..3caa5ddfd 100644 --- a/test/client/doc.js +++ b/test/client/doc.js @@ -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) { + doc.on('error', function(error) { + expect(error.code).to.be.equal('TEST_ERROR'); + }); + 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'); + next(); + }); + } + ], done); + }); }); describe('errors on ops that could cause prototype corruption', function() { From fd67b16660ea74376c9f85576a17b6dbe9d2bd43 Mon Sep 17 00:00:00 2001 From: Dawid Kisielwski <91346346+dawidreedsy@users.noreply.github.com> Date: Tue, 4 Mar 2025 18:25:44 +0100 Subject: [PATCH 2/2] Update to call _isInHArdRollback at the top of fetch --- lib/client/doc.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/client/doc.js b/lib/client/doc.js index 7a1e7d0b9..a7ad4e708 100644 --- a/lib/client/doc.js +++ b/lib/client/doc.js @@ -1058,6 +1058,8 @@ 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(fetchError) { + doc._isInHardRollback = false; + // 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 @@ -1088,10 +1090,7 @@ Doc.prototype._hardRollback = function(err) { inflightOp = null; } - if (!pendingOps.length) { - doc._isInHardRollback = false; - return; - } + if (!pendingOps.length) 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' @@ -1104,7 +1103,6 @@ Doc.prototype._hardRollback = function(err) { allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks; } if (err && !allOpsHadCallbacks) doc.emit('error', err); - doc._isInHardRollback = false; }); };