do not block execution when async functions are called multiple times

[vonovak]

Summary

firstly, let me say I'm open to discussing this. There may be some issues I didn't notice.

Motivation: currently, when people call some of the functions exposed by the library (eg. signInSilently) multiple times, the first call will "get through" to the native sign in SDKs and start desired operation. The next calls, however, will get blocked while the first call in not done. The error message given in that case is:

"Cannot set promise. You've called \"" + requestedOperation + "\" while \"" + getNameOfCallInProgress() + "\" is already in progress and has not completed yet. " +
                "Make sure you're not repeatedly calling signInSilently, signIn or getTokens from your JS code while the previous call has not completed yet."

From looking at various reported issues, this sometimes seems to be problematic because I got an impression that it sometimes happens that people call some async JS function we export, let's call it f() (f can be eg. signInSilently) which calls some other function F() in the native module. F() requires some network IO and since network can be unreliable, people can try to call the same function f() again when they think it has been taking too long. This time, because F() didn't finish (maybe there is a network problem), the native module will decide not to call F() again and rejects with the error mentioned above.

This PR changes the behavior: again consider user calls f(). The native module calls F(), and when it's taking too long, user again calls f(). What happens in this PR is that the promise we're storing in the native module for the result of the first F() will get rejected and we'll store the promise from the second call. F() is then repeated without waiting for the first F() to finish.

what can happen here is

• only first F() finishes with a delay: we'll use the promise we stored for the second call of F() and resolve/reject with result of the first F()
• first F() does not finish but second F() does: we'll use the promise we stored for the second call of F() and resolve/reject with result of the second F()
• both first and second F() finish: we'll use the promise we stored for the second call of F() and resolve/reject with result of the first F(). When second F() finishes, the promise is already resolved, there is nothing to be done.
• neither finishes, or they finish unsuccessfully: in this case user can try again

It's not perfect but I hope it'll improve things, and we'll still be avoiding possible memory leaks. There certainly are alternatives, such as storing not just 1 promise in the native module, but storing all promises, or using events. Overall I think this is a a good tradeoff.

Test Plan

tested exported functions manually

What's required for testing (prerequisites)?

What are the steps to reproduce (after prerequisites)?

Compatibility

OS	Implemented
iOS	✅❌
Android	✅❌

Checklist

• I have tested this on a device and a simulator
• I added the documentation in README.md
• I mentioned this change in CHANGELOG.md
• I updated the typed files (TS and Flow)
• I added a sample use of the API in the example project (example/App.js)

[jozan]

I’ll take a look at this today.

…

On Sat 7. Mar 2020 at 18.08, Vojtech Novak ***@***.***> wrote: @vonovak <https://github.com/vonovak> requested your review on: #830 <#830> do not block execution when async functions are called multiple times. — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#830>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6ZKIYUPNISRZCDNEF2T3RGJWRNANCNFSM4LDQVIBQ> .

[jozan]

This is a good change. Makes this whole flow more readable. 👏

[jozan]

Approved the changes! I like this approach more than the alternatives provided.

What happens for the first Javascript promise when user presses sign in button again and another promise get’s called?

[vonovak]

@jozan the first promise will be rejected, see https://github.com/react-native-community/google-signin/blob/9649cd5305c1f9e2e11ce588c82ea5b47c8254be/android/src/main/java/co/apptailor/googlesignin/PromiseWrapper.java#L65

What happens in this PR is that the promise we're storing in the native module for the result of the first F() will get rejected and we'll store the promise from the second call.