🐛 Wrong error when submitting ops during hard rollback.

[dawidreedsy]

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:

    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.

[coveralls]

coverage: 97.491% (+0.004%) from 97.487%
when pulling fd67b16 on bug-fix/broken-hard-rollback
into ca50594 on master.

[alecgibson]

Test, please!

[alecgibson]

You could just move this after the var err = new ShareDBError(ERR_DOC_DOES_NOT_EXIST) to override this "default" error and then avoid having to duplicate the emit() machinery

[dawidreedsy]

Yes i know want to prepare the test today, just need to fiddle a bit to make it work. Just wanted to push it first, so the @ericyhwang get a chance to look into it before the meeting :)

[dawidreedsy]

To be honest i think there might be more to this than just my fix as before fetch we do

  this._setType(null);
  this.version = null;
  this.inflightOp = null;
  this.pendingOps = [];

and after fetch is finished

    if (inflightOp) pendingOps.unshift(inflightOp);
    var allOpsHadCallbacks = !!pendingOps.length;
    for (var i = 0; i < pendingOps.length; i++) {
      allOpsHadCallbacks = util.callEach(pendingOps[i].callbacks, err) && allOpsHadCallbacks;
    }
    if (err && !allOpsHadCallbacks) doc.emit('error', err);
    doc._isInHardRollback = false;
```,

I think it was menat to reject all ops that were added after fetch started. I am not sure just throwing the above error wouldn't break anything. Even though all tests are passing.

[ericyhwang]

If we want to avoid potential future issues with forgetting to reset _isInHardRollback on early returns, we could reset to false at the top of this _fetch callback