Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fall back to doing a _hardRollback when op.type.invert fails #520

Merged
merged 1 commit into from
Oct 5, 2021

Conversation

hsource
Copy link
Contributor

@hsource hsource commented Oct 4, 2021

Motivation

When we had rollbacks due to conflicts on our server, the rollback would fail when the rejected ops were rich-text ops, which aren't invertible due to the error in #442.

This fix is a less breaking change than what's proposed in #442, #443, and #451, and can potentially close those issues. Unlike those changes, this change:

  1. Doesn't require any changes in libraries to support it
  2. Only changes the behaviour in cases that are totally broken today

Closes #442.

Fix

When the ops fail to invert, just fall back to a hard rollback (refetching the data from the server)

Testing

Added automated test for this case that failed before, but passes now.

@alecgibson
Copy link
Collaborator

alecgibson commented Oct 5, 2021

I think wrapping this in a try/catch is sensible, but I also think this is a bug in json0? I'm right in thinking this doesn't happen if you use a "pure" (non-sub-typed) rich-text document, right? Which is because json0 doesn't check if its subtypes support invert

Edit: on reflection, I'm not sure json0 can do anything particularly useful in this case other than throw, so we should definitely still catch this error, and I'll try to make the json0 error more descriptive.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 97.375% when pulling 20dbec2 on hsource:fix-invert into 46c4aba on share:master.

Copy link
Collaborator

@alecgibson alecgibson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one! Thanks for this.

@alecgibson alecgibson merged commit dd0b7fd into share:master Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not invert automatically in Doc.prototype._rollback
3 participants