Improve the "Pause the instance while "refresh logic" is running" section of the docs

[paveldz]

Link to section: https://github.com/Flyrell/axios-auth-refresh#pause-the-instance-while-refresh-logic-is-running

The description of the pauseInstanceWhileRefreshing parameter is not really clear. At first glance, it seems that this parameter is needed to indicate whether requests should be queued if refreshing is already in progress, but this is not the case.
We had to investigate the source code to understand whether we need to use this parameter or not. It would be great to add some use cases with description when and why it might be needed.

[ten-kis]

Agree, I'd like to get at least one real example why this flag may be needed

[Flyrell]

It's so hard to explain that I might just delete it in the future versions.

It's basically only needed when you cannot add skipAuthRefresh flag to the requests in your refreshing logic and/or there's no call at all (very rare use case, I guess). It's there mainly because of historical reasons, as axios was having troubles with custom object properties in v0.19.0, so this was the only way to not get into the infinite loop when your refreshing logic calls also failed.

To be honest, whenever I find a minute to come here and try to maintain the package, this is basically causing 90% of all issues. Not to mention I need to go through the code every time to check what this flag is actually doing 😄

I think the best move would probably be removing it and keep things simple. If someone has an actual use case for it, they can always alter their refreshing logic.

[guilliamxavier]

Isn't it also needed (or maybe just useful?) when using a separate axios instance for the refresh call? e.g. (simplified):

const auth = axios.create({ baseURL: AUTH_URL });
const initToken = (credentials) => auth.post('/token', credentials).then(response => {
    localStorage.setItem('token', response.data.token);
    localStorage.setItem('refresh_token', response.data.refresh_token);
});
const refreshToken = () => auth.post('/refresh', { refresh_token: localStorage.getItem('refresh_token') }).then(response => {
    localStorage.setItem('token', response.data.token);
});

const api = axios.create({ baseURL: API_URL });
api.interceptors.request.use(config => {
    config.headers['Authorization'] = `Bearer ${localStorage.getItem('token')}`;
    return config;
});
createAuthRefreshInterceptor(api, failedRequest => refreshToken().then(() => {
    failedRequest.response.config.headers['Authorization'] = `Bearer ${localStorage.getItem('token')}`;
    return Promise.resolve();
}), {
    pauseInstanceWhileRefreshing: true // ?
});

// ---

if (!localStorage.getItem('token')) {
    initToken(CREDENTIALS);
}

[
    '/foo',
    '/bar',
    '/qux',
].forEach(path => {
    api.get(path).then(response => {
        console.log(path, response);
    });
});

[Flyrell]

@guilliamxavier Not really, as the interceptor will be bound only to the instance you provided as the first parameter. That means even when your auth logic fails, it will not get intercepted (so there's no risk of getting into the infinite loop).

[guilliamxavier]

I know there's no risk of infinite loop (and I don't need to add skipAuthRefresh in the refresh call) when using 2 isolated instances. But you said that pauseInstanceWhileRefreshing is needed when the refreshing logic makes "no call at all", which I understand as "no call with the bound instance", which is the case in my example (the interceptor is bound to api but the refresh call is made with auth, so there's actually no call from the interceptor POV). Now you are saying that actually it isn't needed? and is it at least useful (e.g. to avoid extra refresh calls)?

[guilliamxavier]

After updating to v3.2.0 and testing a bit, I come to the conclusion that I must not use pauseInstanceWhileRefreshing: even without it I don't see extra refresh calls (so it doesn't seem useful), and with it the /bar and /qux failed requests are not retried after /foo (so it seems harmful)...

[Flyrell]

@guilliamxavier Sure, I see the point you're making. What I mean by "no call" is to literally have no HTTP call inside it (I guess there was not a single use case for that 😅), but now when I think about it, it's not useful anymore for whatever reason. The "no call" I mentioned does not make sense at all, as if there's "no call", the refreshing logic would never trigger an interceptor again - so, my bad.

TL;DR;

There is a couple of ways you can prevent the infinite loop right now:

1. use skipAuthRefresh in your refreshing call's configuration

createAuthRefreshInterceptor(axiosInstance, failedRequest => {
    return axiosInstance.post('/', {}, { skipAuthRefresh: true }).then(/* ... */);
});

2. use different instance for your refreshing call

createAuthRefreshInterceptor(instance1, failedRequest => {
    return instance2.post('/', {}).then(/* ... */);
});

There's not a single use case I can think of, that would need the pauseInstanceWhileRefreshing flag anymore.

---

The pauseInstanceWhileRefreshing flag was introduced in one of the first versions of the library (when there was no skipAuthRefresh flag) to prevent the instance from getting into the infinite loop (back then it was called skipWhileRefresh). It was ensuring that when the refreshingLogic is resolving, the library (specifically, response interceptor) would ignore any error originating from the instance provided in createAuthRefreshInterceptor (so that this logic is not called again as that error could have been the result of refreshing logic itself which would create the infinite loop). This might sound weird, but it was useful for a while, when the axios v0.19.0 was published and they've accidentally removed the possibility to pass the custom property into your request config. Fortunately, axios team did fix it in next versions, and I added the skipAuthRefresh flag to the library, which made pauseInstanceWhileRefreshing useless. After all, this is not useful anymore whatsoever, if you choose to implement the refreshing logic in one of the ways I mentioned above.

Back then I haven't really think about using the second instance for refreshing as you suggested, and to be honest, it would definitely make more sense. I guess in the future versions I might even change 'the API', so that the library works more developer-friendly, as we can probably do a lot by working with multiple instances. We'll see. However, thank you for taking your time and contributing.