Retrying transactions with backoff

[thebrianchen]

I was trying to come up with ways to skip the timeouts in integration tests, and though I'm not a fan overwriting the AsyncQueue instead of initializing it, I couldn't find another way, since we instantiate Firestore through the public API in tests (link). The downside to doing this that the readonly property of _queue in the internal Firestore has to be removed, but there aren't many places where _queue is accessed or set.

Another option to consider is mocking out setTimeout within the scope of the runTransaction call, but that seems more hacky.

[mikelehen]

Yup. skipDelaysForTimerId() LGTM!

[thebrianchen]

@mikelehen I've updated the web version to also use the stubbed exponential backoff in tests to match the Android version.

[mikelehen]

Per my Android feedback, I'd prefer go back to the skipDelaysForTimerId() approach. I'm also in favor of trying to bring JS more closely inline with Android (and probably iOS). But we can defer that discussion until the iOS port is further along.

[mikelehen]

Random note relating to unifying the platforms and whether to change web to run the transaction logic on the AsyncQueue or not: Right now, transactions will likely continue to retry even after you call shutdown() on the client. If we hooked the transaction code into the AsyncQueue, that would automatically be fixed.

[mikelehen]

Nice! Glad you got this working. The AsyncQueue stuff is tricky, but I think it's good to match the other platforms. I have a few nits and an optional cleanup. But in general this is looking good.

[mikelehen]

Ship it!