share / sharedb / 925

96%

master: 97%

push

travis-ci

Update hard rollback callback logic

We previously followed the following logic for callbacks in a hard
rollback:

  - check there's an inflight op, and that it has a callback
  - if this isn't the case, then emit the error

This logic doesn't work very well in the situation where we don't have
an inflight op. This can happen in the following case:

  - I submit an invalid op
  - the op is added to the pending ops queue
  - we attempt to apply the op to the local document before flushing
  - `type.apply` throws, so the op is never flushed from pending to
    inflight
  - we perform a hard rollback
  - we now find we have no inflight op, but we do have a pending op

In this case, since we have no inflight op, we'd emit the error, but
also call the pending op callback, causing both a callback call _and_
an error emission, which is a bit surprising.

This change updates our logic, so that:

  - we check that there are some ops to callback
  - and that all of those ops have callbacks

If there are no callbacks, or if any of the ops don't handle the error,
then we emit, so that we can be sure of never swallowing an error. We
also avoid treating the inflight op any differently to pending ops, so
that we avoid behaviour that's dependent on where in its lifecycle an op
throws.

However, if the client handles all of their op submission errors, then
we never emit.

1093 of 1311 branches covered (83.37%)

7 of 7 new or added lines in 1 file covered. (100.0%)

2304 of 2410 relevant lines covered (95.6%)

407.65 hits per line