Retry IndexedDB Failures

[nVitius]

The Firestore SDK recently received an update (firebase/firebase-js-sdk#2938) that allows it to recover from certain IndexedDB failures. Previously, this would cause the SDK to crash and would require a page refresh.

The current behavior of the syncStack in easy-firestore is to catch errors from batch.commit. I think it would be a good idea to implement some retry logic (with back-off?) for the IndexedDB errors, as these are potentially sane commits that errored out due to external factors.

I wanted to open up an issue and discuss the potential approaches. I am able to work on this if nobody more familiar with the codebase is able to.

[mesqueeb]

@nVitius I'm very interested in this. Also I'm heavily working on the complete rewrite for v2.0, done with the core lib, and now just started on the Firebase logic. So I want to integrate this in 2.0 as well.

To get things started I think this will be required first:

• A snippet of code, to show as per example which Firebase SDK function can fail where (in relation to this)
• Clarification on how such an error could be captured with Firebase SDK and what error format it uses

Based on this it'll be easier to determine how/which part of the codebase to implement this in.

If you could help out with the above code snippets, that would be great!

_{--
Vuex Easy Firestore was made with ♥ by Luca Ban.
If you use this library in your projects, you can support the maintenance of this library by a small contribution via Github 💜.
You can also reach out on twitter if you want a one-on-one coding review/lesson. 🦜}

[nVitius]

The function that catches and throws the IDB error is SyncEngine.write. This is called by the internal Firestore function FirestoreClient.write.

This function in turn is used to write changes from the public APIs for DocumentReference.set, DocumentReference.update, DocumentReference.Delete, and WriteBatch.commit.

It doesn't have a unique error code, but it looks like all the other places that throw the same error code (unavailable) won't affect these functions. It should be safe to catch and retry in this fashion:

batch.commit()
  .then(() => { ... })
  .catch(err => {
    if (err instanceof firestore.FirestoreError && err.code === 'unavailable')
      // handle retry
  }

P.D.: I really appreciate how actively you support this project. I just saw your post about the v2; it's very exciting. If by any chance you happen to need any help with work on the new version, let me know. I would love to collaborate on it. Even just to talk about the experience I've had using easy-firestore over the past year, if you think that would be valuable.

[mesqueeb]

@nVitius thanks for this.

Based on your guidance I'll be able to implement this right away into v2.0.

By the way i'm almost ready for beta launch for v2.0, at which point I'll definitely reach out!

Can I request your help for v1.0?

How I imagine this would be, at this line:

vuex-easy-firestore/src/module/actions.ts

Line 235 in 63e2aa6

state._sync.patching = 'error'

you check for if (err instanceof firestore.FirestoreError && err.code === 'unavailable'), and based on that you retry once to dispatch batchSync again.

Make sure only to retry once and if it succeeds the second time, resolve.
If it doesn't work a second time, reject the original promise returned by the batchSync action.

You can fork, npm i, make changes and check if all tests pass or not with npm run build. If you have just the 3 failing server related tests it should be OK.

[nVitius]

Yeah, I can work on that. Will probably be ready for review in the next couple of days.

[mesqueeb]

@nVitius Cool, thank you so much!