vlki / refresh-fetch

Wrapper around fetch capable of graceful authentication token refreshing.
MIT License
84 stars 12 forks source link

Should refresh token doesn't get called #14

Closed oron11 closed 4 years ago

oron11 commented 4 years ago

I used the code like this, with the native fetch instead of fetchJSON: And the should refresh token function doesn't get called with a status of 401.

const fetchWithToken = (url, options = {}) => {
    const accessToken = getAccessToken();

    let optionsWithToken = options;
    if (!util.isEmpty(accessToken)) {
      optionsWithToken = Object.assign(options, {
        headers: getHeadersForServerRequest(accessToken)
      })
    }
    return fetch(url, optionsWithToken);
}
const shouldRefreshToken = (response) => {
    util.log("should refresh token called: ");
    const jsonResponse = JSON.parse(response);

    return ((response.status === 401) && (jsonResponse.message === "Invalid access_token"));
}

const refreshToken = async () => {
   try {
       let body = new FormData();
       body.refreshToken = await getRefreshToken();

    const response = await fetch(SERVER_ADDR + 'auth/refreshToken',
     { 
        method: 'POST',
        body
    });

    if (response.status !== 200) {
        throw new Error("Failed to refresh token");
    }

    const jsonResponse = await response.json();
    await storeTokens(jsonResponse.accessToken, jsonResponse.refreshToken);
   } 
   catch (error) {
    // Clear token and continue with the Promise catch chain
    await clearTokens();
    throw error;
   }
}

const authFetch = configureRefreshFetch({
    fetch: fetchWithToken,
    shouldRefreshToken,
    refreshToken
  })

Looking into the implementation code of the library, it seems that the code is getting called only if the error of the fetch gets to the "catch" statement. Is this valid?

Env: "refresh-fetch": "^0.7.0", "expo": "^38.0.0",

vlki commented 4 years ago

Hey @oron11, oh yeah, that's because the native fetch does not reject the promise when the HTTP response has 4xx code and refresh-fetch expects the rejected promise. I have the rejecting in fetchJSON (see https://github.com/vlki/refresh-fetch/blob/main/src/fetchJSON.js#L25), so adding it similarly to your fetchWithToken should do the trick.

Eg.

const fetchWithToken = (url, options = {}) => {
    const accessToken = getAccessToken();

    let optionsWithToken = options;
    if (!util.isEmpty(accessToken)) {
      optionsWithToken = Object.assign(options, {
        headers: getHeadersForServerRequest(accessToken)
      })
    }
    return fetch(url, optionsWithToken).then(response => response.ok ? response : Promise.reject(response))
}
oron11 commented 4 years ago

@vlki Thanks, it's working now!

yassinebridi commented 3 years ago

@vlki I just tried your solution, but the fetchWithToken function keeps running forever.

yassinebridi commented 3 years ago

I'm using graphql, so this is what i ended up doing:

return fetchJSON(url, optionsWithToken).then(res => res.body.errors ? Promise.reject(res.body.errors[0]) : res)
vlki commented 3 years ago

Hey @yassinebridi , I'm sorry to hear that. If you keep having the problem, please open separate issue and post more code as it is hard to deduct what might be the cause from the little you posted. Thx!

yassinebridi commented 3 years ago

It's working fine with the above snippet. I just had to check for the right object since it is a graphql response.

Thank you for this beautiful package, it is working seamlessly for me now.

vlki commented 3 years ago

Oh, got it, that's good then! And thanks, I'm happy that it is useful.