Calling oAuth2Fetch.fetch() immediately after invoking constructor causes race condition with async IIFE invoking getStoredToken

[debater-coder]

This following code fails because getStoredToken is always invoked as an async function, meaning that fetchWrapper.fetch() will be called before getStoredToken() has even been called. See this async IIFE. By the time the fetchWrapper.fetch() is called, the private property fetchWrapper.token is still null, so getToken() attempts to refresh it, which fails, invoking refreshToken() which attempts to refresh the non-existent token, which fails invoking getNewToken() which also fails, finally invoking onError, which prompts the user to log in again. When this is finished, control returns to the async IIFE, which finally sets the token to the correct value, and the next time fetchWrapper.fetch() is invoked, it behaves correctly. I have verified this using the debugger in Chrome DevTools. It is possible to create an async constructor by returning an IIAFE from the constructor. However, this could be tricky without introducing a breaking change. Alternatively, the IIAFE could be stored in a private property which could be awaited inside of the fetchWrapper.fetch() function, ensuring that getStoredToken() has been invoked. Until a fix is released, I have a temporary workaround, but it is quite hacky, so I would appreciate a stable fix.

Problematic Code

const fetchWrapper = new OAuth2Fetch({
    client,
    getNewToken: () => {
      return null; // Fail this step, we don't want to log out until the user does so explicitly
    },
    storeToken,
    getStoredToken,
    onError: (error) => {
      // Prompt the user to log in again
    },
  });

const res = await fetchWrapper.fetch(...)

Temporary workaround

let ready = false;

const initialiseFetchWrapper = async () => {
  const fetchWrapper = new OAuth2Fetch({
    client,

    getNewToken: () => {
      return null; // Fail this step, we don't want to log out until the user does so explicitly
    },

    storeToken,

    getStoredToken,

    onError: (error) => {
      if (ready) {/* Prompt user to log in */}
    },
  });
  try {
    await fetchWrapper.getToken();
  } catch {
    // Ignore we expect this to error the first time
  }
  ready = true;
  return fetchWrapper;
};

let _fetchWrapper: OAuth2Fetch | null = null;

// We defer initialisation until the first call to fetchAuthenticated to ensure that initialisation has been completed
const getFetchWrapper = async () => {
  if (!_fetchWrapper) {
    _fetchWrapper = await initialiseFetchWrapper();
  }

  return _fetchWrapper;
};

[debater-coder]

Hi, it's been a little while. Are there any plans to remedy this or am I doing this wrong. @evert

[evert]

Hi! This ticket makes perfect sense.

I think the ideal way to solve this might be to store the promise that's returned from getNewToken on 'this' until it's resolved and await it in fetch().

I'll try to get to this in the next few days

[debater-coder]

Thank you!

[brianjenkins94]

I'm not sure this is fixed. I have to do this to get it to behave correctly:

await fetchWrapper.activeGetStoredToken
await fetchWrapper.getToken()

Calling just fetchWrapper.getToken() on its own appears to always get a new token.

[brianjenkins94]

Here's my workaround:

export const fetchWrapper = new OAuth2Fetch({
	"client": oauth2Client,
	"getNewToken": async function() {
		// WORKAROUND: https://github.com/badgateway/oauth2-client/issues/110
		if (fetchWrapper.activeGetStoredToken !== null) {
			await fetchWrapper.activeGetStoredToken;

			if (existsSync("token.json")) {
				return JSON.parse(await fs.readFile("token.json", { "encoding": "utf8" }));
			}
		}