Use async/await to allow for async storage

[splurgebudget]

Changed the entire call tree to allow for async storage.

Also: Redid the way requests queue in the interceptor to remove a race condition.

If two interceptors were run back-to-back, it was possible for this to happen:

1. Interceptor1 starts and checks isRefreshing - which is false so it skips the queue
2. Interceptor1 then yields before getting to the isRefreshing = true in refreshToken
3. Interceptor2 starts and checks isRefreshing - which is still false! So it skips the queue
4. Interceptor1 runs and refreshes the token
5. Interceptor2 runs and refreshes the token

Why didn't the (well designed!) test catch this before? Because there happened to not be any await calls in-between starting the interceptor and setting isRefreshing = true. However, this was a race condition waiting to happen (which it did - as soon as I added an await!)

My fix removes the queue and utilizes the native "queue" created by multiple waiters waiting on the same Promise. This is more robust due to the lack of distance between waiting and setting the Promise - and is quite a bit simpler.

[valentin-debris]

This PR arrives at the right time !

I'm starting a new react-native app that will use SecureStore from Expo, which use async calls, so for now, the custom Storage I made can't works due to the lack of supporting async/await.

Hope this will be releases soon :)

[splurgebudget]

Yep - I made this for Expo SecureStore myself… it works great! You can always directly NPM install from my repo/branch if you want to. That’s how I’m working while I’m waiting on this to be merged.

[revmischa]

Thanks for this work! This will need to be a new major version right, since it will break any existing users?

[revmischa]

Maybe we can return early here for clarity

[splurgebudget]

Can't return here - needs to fall through to the code below to set the headers and and the config.

[revmischa]

Can put this in a finally block, it will always get called

[splurgebudget]

I don't understand? No need for finally... we're using await.

[splurgebudget]

Wait - I see what you are saying now. I thought you were referencing waiting on the Promise - but you were actually talking about setting it to undefined. Sure - that would be fine.

I don't actually know the semantics though - does a finally get run even if we rethrow like we are on 171? At any rate - I would say to hold off on that change until we get this in. What's here now matches up with the original code.

[revmischa]

It does get called even if rethrowing, I believe the doc I linked is saying that
Not a big deal

[splurgebudget]

Thanks for this work! This will need to be a new major version right, since it will break any existing users?

No problem! Glad to contribute!

A new major version might be a good idea. The upgrade will be pretty easy for most users... they really just need to add an await in front of setAuthTokens.

[revmischa]

Can you bump the major version and we can merge it?

[splurgebudget]

@revmischa - done!

[revmischa]

Thanks for your work!

[revmischa]

4.0.0 published