3

I am using interceptor that refreshes the access token upon a status code of 401 but my catch has stopped working upon any other error codes such as 400

request.js

import axios from 'axios'

const axiosApiInstance = axios.create()

axiosApiInstance.interceptors.response.use(
    (response) => {
      return response
    },
    (error) => {
      return new Promise((resolve) => {
        const originalRequest = error.config
        const refreshToken = localStorage.getItem("refresh")
        if (error.response && error.response.status === 401 && error.config && !error.config.__isRetryRequest && refreshToken) {
          originalRequest._retry = true
  
          const response = fetch('http://127.0.0.1:8000/api/token/refresh/', {
            method: 'POST',
            headers: {
              'Content-Type': 'application/json',
            },
            body: JSON.stringify({
              refresh: refreshToken,
            }),
          })
            .then((res) => res.json())
            .then((res) => {
              localStorage.setItem("access", res.access) 
              originalRequest.headers['Authorization'] = 'Bearer ' + res.access
              return axios(originalRequest)
            })
          resolve(response)
        }
        return Promise.reject(error)
      })
    },
  )

  export default axiosApiInstance
Usama Nadeem
  • 75
  • 3
  • 10
  • 1
    Your code has the [Explicit Promise construction antipattern](https://stackoverflow.com/questions/23803743/what-is-the-explicit-promise-construction-antipattern-and-how-do-i-avoid-it)! – FZs Nov 27 '20 at 15:13
  • 1
    Are you saying that a 400 status does not result in the catch being called? Or is it because your condition contains `error.response.status === 401` that the bulk of it isn't called? – Bravo Nov 27 '20 at 15:13
  • I am using this axios instance where I need to call my API and whenever a 400 response is sent by server, the catch does not get called over there. I used default axios call and that did go inside catch upon 400. I am using this custom instance to refresh my access token whenever I get 401 response. – Usama Nadeem Nov 27 '20 at 15:43
  • the answer below explains the issue ... `return Promise.reject(error)` in the Promise executor does NOT cause Promise to reject, so, in the case of any other state (error 400, for example) your code is handling the error and returning a resolved promise - also, you don't actually need to construct a new promise, since the axios interceptors are just part of a promise chain ... and the functions you provide are used as the `onResolved, onRejected` callback pair in a `.then` – Bravo Nov 27 '20 at 15:56

1 Answers1

4

You need to pass a second argument to your callback to new Promise called reject and then reject the promise using that function instead of Promise.reject.

I can't verify without testing, and that's a bit difficult to set up right now, but it looks very much like you're not properly rejecting the promise that you're returning from the error callback. If you can confirm that the promise is never resolved or rejected in the case of a 400, that would help confirm my suspicion. Try making this change, though:

The arguments to your new Promise callback:

      return new Promise((resolve) => {

becomes

      return new Promise((resolve, reject) => {

and later to reject, instead of

        return Promise.reject(error)

just do this

        reject(error) // return does nothing here, can be omitted

Edit:

Although, a very good point by someone that this is a promise anti-pattern. You're creating a new Promise for no reason.

import axios from 'axios'

const axiosApiInstance = axios.create()

axiosApiInstance.interceptors.response.use(
    (response) => {
      return response
    },
    (error) => {
        const originalRequest = error.config
        const refreshToken = localStorage.getItem("refresh")
        if (error.response && error.response.status === 401 && error.config && !error.config.__isRetryRequest && refreshToken) {
            originalRequest._retry = true

            return fetch('http://127.0.0.1:8000/api/token/refresh/', {
            method: 'POST',
            headers: {
                'Content-Type': 'application/json',
            },
            body: JSON.stringify({
                refresh: refreshToken,
            }),
            })
            .then((res) => res.json())
            .then((res) => {
                localStorage.setItem("access", res.access) 
                originalRequest.headers['Authorization'] = 'Bearer ' + res.access
                return axios(originalRequest)
            })
        }
        return Promise.reject(error)
    },
  )

  export default axiosApiInstance

Should be something like this.

Matthew Brooks
  • 521
  • 2
  • 5
  • the interceptors in axios are part of a promise chain already , so you do not need to create a new promise at all - that is an anti-pattern - besides that you are on the money – Bravo Nov 27 '20 at 15:32
  • 1
    Yup, just edited to include that. Was trying to provide a minimal solution for fixing it, but it's important to clear up the anti-pattern in here. It's pretty late and I missed that it was unnecessary. – Matthew Brooks Nov 27 '20 at 15:36
  • Note: you can pass in `undefined` (or `null` or anything that isn't a function actually) instead of the "no-op" `(response) => { return response}` since interceptors are just a chain of `.then(onResolved, onRejected)` – Bravo Nov 27 '20 at 16:07