axios icon indicating copy to clipboard operation
axios copied to clipboard

request intercepters order is reversed

Open latel opened this issue 7 years ago • 27 comments

const axios = require('axios');

axios.interceptors.request.use(function(config) {
	console.log(1);
}, undefined);


axios.interceptors.request.use(function(config) {
	console.log(2);
}, undefined);

axios.get('http://www.baidu.com').then(function() {
	console.log('ok');
}).catch(function() {
	console.log('fail');
});

which prints:

node index.js
2
1
fail

latel avatar Jul 09 '18 18:07 latel

Hey there @latel,

An axios contributor/manager can correct me if I'm wrong, but as I understand it (and from the comments in the code), the interceptors are saved in a stack. Therefore, when comes the time for the interceptors to be executed, it starts with the last added interceptor (first in last out ... or ... last in first out (LIFO)).

Here's the part of the code where a stack is mentioned:

// axios.js line 1263
/**
* Add a new interceptor to the stack
*/

Now here's the hookup of interceptors, notice the use of "unshift()"

// lib/core/Axios.js line 43
this.interceptors.request.forEach(function unshiftRequestInterceptors(interceptor) {
  chain.unshift(interceptor.fulfilled, interceptor.rejected);
});

Therefore I believe it's normal for axios to behave the way it behaves.

On a side note, may I know your use case? Why do you need two interceptors back to back like this? If one interceptor must be executed before the other, then I suggest you just build one interceptor that does the whole thing.

Also, you might want to add this inside your interceptors

return config;

This way you'll get

node index.js
2
1
ok

Hope this helps 😃 !

anthonygauthier avatar Jul 17 '18 17:07 anthonygauthier

This actually was asked a year ago, but it got closed and no changes had been made.

https://github.com/axios/axios/issues/841

If this feature is to stay, at least it should be documented.

OpenGG avatar Aug 07 '18 12:08 OpenGG

Thank you for your reply @delirius325,

and as @OpenGG mentioned, I also notice this PR, it is merged but now it's gone again @le0nik.

I think sometimes it's better to have an option to specify the order we really need, whether it's reverse or not. The default execute oder of request intercepters and response intercepters are opposite, which is a confusing thing.

Think about the following situation:

  1. We have a request modules which just wrappers axios for default behaviour management, and expose beforeEach and afterEach hook, and it also defines a request intercepter which just mark every request with a uniq id index.
import axios from 'axios';

const beforeEach = (succFn, errFn) =>
  axios.interceptors.request.use(succFn, errFn);
const afterEach = (succFn, errFn) =>
  axios.interceptors.response.use(succFn, errFn);

// this interceptor add a uniq id index
beforeEach(config => {
  config.id = uniqIndexedId();
  return config;
});

export { beforeEach, afterEach }
  1. Here we have a module which import the above request module, and define interceptors using thebeforeEach hook.
import { beforeEach } from '@/service/request';

beforeEach(config => {
    console.log(config.id); // => UNDEFINED
});

This fails because the later registered interceptor is executed before the interceptor which is registered in request module.

I think we do need the first-in-first-out order because sometimes we have to convergence some logic into a single module, and ensure that these interceptors are executed earlier.

latel avatar Aug 14 '18 06:08 latel

anyone here to see this problem?

latel avatar Sep 02 '18 13:09 latel

This is required, because I am using axios-case-converter and custom case converter for specific fields. Please check this link: https://www.npmjs.com/package/axios-case-converter My backend was built with Rails and it requires _destory to remove a field. But using axios-case-converter, there's no way to make the request with field name _destroy. I've tried various strings like Destroy and _destroy but plugin just parsed everything to destroy. So I need another interceptor to process this edge case, but axios api seems like to run the interceptors randomly. In some cases, the interceptors run in desired order, but some cases, my custom interceptor runs first. I need help on this as soon as possible.

danZheng1993 avatar Dec 24 '18 17:12 danZheng1993

I'd like to support this change too. There's an old PR https://github.com/axios/axios/pull/1041 to solve it.

ppdeassis avatar Jan 17 '19 18:01 ppdeassis

Yes, I've notice this weird behavior too. If this behavior was made in purpose, it should be well document it.

mactavishz avatar Feb 01 '19 09:02 mactavishz

I'd like to support this change too. There's an old PR #1041 to solve it.

this pr seemed to be merged, but for some reason is now gone

latel avatar Feb 04 '19 15:02 latel

feature instead of bug ?

lili21 avatar Mar 18 '19 07:03 lili21

Hi, this problem is relevant. It is not possible to override and line up the order of the handlers. It is important!

KonovalovaKV avatar Apr 05 '19 09:04 KonovalovaKV

Is there any progress? At least there should be a configuration.

zgayjjf avatar Apr 08 '19 09:04 zgayjjf

Hi everyone,

So this has been a long standing "issue" which can easily be worked around by adding the following snippet to your code:

// add this after your interceptor(s') declaration
axios.request.interceptors.handlers.reverse();

However, I did add a reverseInterceptorsOrder method in this PR https://github.com/axios/axios/pull/2085.

anthonygauthier avatar Apr 08 '19 14:04 anthonygauthier

Any news?

KonovalovaKV avatar May 07 '19 04:05 KonovalovaKV

@KonovalovaKV

PR is still ready to be merged. As mentioned in my previous comment, you can DIY with the following code snippet:

axios.request.interceptors.handlers.reverse();

Hope this helps!

anthonygauthier avatar May 07 '19 15:05 anthonygauthier

It should be, isn't?

axios.interceptors.request.handlers.reverse();

7rulnik avatar Jun 21 '19 16:06 7rulnik

@delirius325 Would you or someone else mind opening another PR to emphasize it in the document? The current behavior is not straight-forward, but it has been already there several years. And we don't want to introduce extra interface or option.

chinesedfan avatar Feb 16 '20 05:02 chinesedfan

@chinesedfan not sure I understand what you mean by "emphasize it in the document"? What needs to be emphasized exactly? The method I had created? Or the current behaviour?

anthonygauthier avatar Feb 17 '20 13:02 anthonygauthier

The current behavior is not straight-forward, but it has been already there server years. And we don't want to introduce extra interface or option.

The current behavior. Thanks.

chinesedfan avatar Feb 17 '20 14:02 chinesedfan

I have no idea why is this issue getting ignored. Please, either document this unnatural reverse execution order or please merge this PR https://github.com/axios/axios/pull/1041 previously created.

hxiongg avatar Apr 09 '20 06:04 hxiongg

Is this even considered a bug? What is the position of the team regarding this behaviour? I don't think this was communicated clearly.

If this is considered a bug I suggest to fix it with the provided PR. If this is considered working as intended I suggest closing this issue with a note.

b-strauss avatar May 07 '20 12:05 b-strauss

Adding my support for this as either a fix or as an option. And yes, it should be documented as well, since this seems very unnatural to me and it's not how the response interceptors work. My use case is similar to the others mentioned, in that I'm creating plugin wrappers that utilize interceptors to add modular functionality. It makes sense to me to have the interceptors run in the order that they are attached.

Also, I've seen no replies here that give any reason for why this behavior exists in the first place. It does seem explicitly written this way, so it'd be interesting to hear the implementation reasons.

ansonatloop avatar May 22 '20 16:05 ansonatloop

The current behavior is not straight-forward, but it has been already there server years. And we don't want to introduce extra interface or option.

The current behavior. Thanks.

yep clearly not straight-forward ! and if not a bug, an algorithmic issue !

Adding my support for either a fix or an option !

linqFR avatar Nov 19 '20 21:11 linqFR

/* Edited: I had forgotten to pipe the handlers */

Well, I've used a different aproach. First off, I've defined an Object called InterceptionHandler. I also have defined a pipe function to chain the handlers

class InterceptionHandler {
  constructor({request, response}) {
    this.request = request;
    this.response = response;
  }
}

const pipe = (...handlers) => arg => handlers.reduce((output, actual) => actual ? actual(output) : output, arg);

So, the ideia is create multiple handlers, and use them in a single pair of interceptors.

const handler1 = new InterceptionHandler(
  {
    request: (config) => {
      console.log('request 1');
      return config;
    },

    response: (response) => {
      console.log('response 1')
      return response;
    }
  }
);

const handler2 = new InterceptionHandler(
  {
    request: (config) => {
      console.log('request 2');
      return config;
    },

    response: (response) => {
      console.log('response 2')
      return response;
    }
  }
);

After that we can line up the handlers in an appropriate execution order.


axios.interceptors.request.use(
  (config) => pipe(handler1.request, handler2.request)(config),
  (error) => Promise.reject(error)
);

axios.interceptors.response.use(
  (response) => pipe(handler1.response, handler2.response)(response),
  (error) => Promise.reject(error)
);

Which produces the output

request 1
request 2
response 1
response 2

marceloloureiro avatar Dec 10 '20 14:12 marceloloureiro

Can someone maybe propose a PR for this? I think we can fix it but it will be a breaking change.

jasonsaayman avatar Oct 22 '21 11:10 jasonsaayman

may be we can resolve this issue in another way, provide a option to define whether this interceptor should be executed ealier or later, just like enforce in webpack loaders do.

latel avatar Nov 11 '21 10:11 latel

related to the documentation

... then they are executed in the order they were added ...

// Standard File Core System not possible to overwrite
const axios = require('axios');

export default function factory(context) {
        return new axios(context)
}

axios.interceptors.request.use(function(config) {
        config.headers['not-cors-header'] = 'bla';
	console.log(1);
});

axios.interceptors.request.use(function(config) {
	console.log(2);
});


// our custom file later included after core system was loaded

const httpClient = CoreSystem.httpClient; 

axios.interceptors.request.use(function(config) {
        delete config.headers['not-cors-header'];
        config.headers.Authentication = 'Baerer Token';
	console.log(3);
}, undefined);

axios.get('http://www.bla.bla').then(function() {
	console.log('ok');
}).catch(function() {
	console.log('fail');
});

Resulting into the following results:

# current result
3
2
1
ok

# expected result
1
2
3
ok

The header still contains "not-cors-header" as this is the last interceptor used, but the first one to be implemented.

This is the code section where the unshift was implemented. Axios -> Core -> Line 128

const requestInterceptorChain = [];
let synchronousRequestInterceptors = true;
this.interceptors.request.forEach(function unshiftRequestInterceptors(interceptor) {
      if (typeof interceptor.runWhen === 'function' && interceptor.runWhen(config) === false) {
            return;
      }

      synchronousRequestInterceptors = synchronousRequestInterceptors && interceptor.synchronous;

      requestInterceptorChain.unshift(interceptor.fulfilled, interceptor.rejected);
});

Denfie avatar Jul 10 '24 09:07 Denfie

I'm going to offer an unpopular opinion:

The current behavior is better than the proposed change. The ordering of the request interceptors should just be documented better.


Reasoning:

There are some helper libraries built on top of axios, like axios-retry, that will add both a request and a response interceptor for you.

The way the library currently works, I could call several third-party helpers like this, each adding a request and response interceptor, and they will logically form a stack. The last added helper will intercept the request first, and intercept the response last. This is the same way that middleware's work in django on the server side.

It's often more useful if the middlewares form a stack, because you may want middleware that retries for 500's to be outermost, and higher-level middlewares, like retrying for some type of auth error, to be inwards of that, and anything that changes other details of the request to be inwards of that, in order to get the best behavior of the client overall.

If this were changed so that requests are handled in order registered and responses are handled in order registered, it would be harder for people to publish middlewares like axios-retry and for you to just add several of them and get the right behavior. You would have to re-order the interceptors manually, and that would be pretty annoying.

cbeck88 avatar Jul 31 '24 01:07 cbeck88

closing due to being very stale, if this is still relevant please open a new issue

jasonsaayman avatar Sep 26 '24 17:09 jasonsaayman