issue#2609 | Sasha | predictable axios requests (#2702)

* issue#2609 | Sasha | predictable axios requests

- axios requests are not delayed by pre-emptive promise creation by default
- add options to interceptors api ("synchronous" and "runWhen")
- add documentation and unit tests

* issue#2609 | Sasha | pull request feedback changes

* issue#2609 | Sasha | additional feedback changes

* issue#2609 | Sasha | put back try/catch

* issue#2609 | Sasha | add 2 adapter unit tests

- remove check for requestCancelled

Co-authored-by: ak71845 <alexandre.korotkov@kroger.com>
Co-authored-by: Xianming Zhong <chinesedfan@qq.com>
Co-authored-by: Jay <jasonsaayman@gmail.com>

README.md

@@ -590,6 +590,34 @@ const instance = axios.create();
 instance.interceptors.request.use(function () {/*...*/});
 ```
 
+When you add request interceptors, they are presumed to be asynchronous by default. This can cause a delay
+in the execution of your axios request when the main thread is blocked (a promise is created under the hood for 
+the interceptor and your request gets put on the bottom of the call stack). If your request interceptors are synchronous you can add a flag
+to the options object that will tell axios to run the code synchronously and avoid any delays in request execution.
+
+```js
+axios.interceptors.request.use(function (config) {
+  config.headers.test = 'I am only a header!';
+  return config;
+}, null, { synchronous: true });
+```
+
+If you want to execute a particular interceptor based on a runtime check, 
+you can add a `runWhen` function to the options object. The interceptor will not be executed **if and only if** the return
+of `runWhen` is `false`. The function will be called with the config
+object (don't forget that you can bind your own arguments to it as well.) This can be handy when you have an
+asynchronous request interceptor that only needs to run at certain times.
+
+```js
+function onGetCall(config) {
+  return config.method === 'get';
+}
+axios.interceptors.request.use(function (config) {
+  config.headers.test = 'special get headers';
+  return config;
+}, null, { runWhen: onGetCall });
+```
+
 ## Handling Errors
 
 ```js

lib/core/Axios.js

@@ -45,20 +45,61 @@ Axios.prototype.request = function request(config) {
     config.method = 'get';
   }
 
-  // Hook up interceptors middleware
+  // filter out skipped interceptors
-  var chain = [dispatchRequest, undefined];
+  var requestInterceptorChain = [];
-  var promise = Promise.resolve(config);
+  var synchronousRequestInterceptors = true;
-
   this.interceptors.request.forEach(function unshiftRequestInterceptors(interceptor) {
-    chain.unshift(interceptor.fulfilled, interceptor.rejected);
+    if (typeof interceptor.runWhen === 'function' && interceptor.runWhen(config) === false) {
+      return;
+    }
+
+    synchronousRequestInterceptors = synchronousRequestInterceptors && interceptor.synchronous;
+
+    requestInterceptorChain.unshift(interceptor.fulfilled, interceptor.rejected);
   });
 
+  var responseInterceptorChain = [];
   this.interceptors.response.forEach(function pushResponseInterceptors(interceptor) {
-    chain.push(interceptor.fulfilled, interceptor.rejected);
+    responseInterceptorChain.push(interceptor.fulfilled, interceptor.rejected);
   });
 
-  while (chain.length) {
+  var promise;
-    promise = promise.then(chain.shift(), chain.shift());
+
+  if (!synchronousRequestInterceptors) {
+    var chain = [dispatchRequest, undefined];
+
+    Array.prototype.unshift.apply(chain, requestInterceptorChain);
+    chain.concat(responseInterceptorChain);
+
+    promise = Promise.resolve(config);
+    while (chain.length) {
+      promise = promise.then(chain.shift(), chain.shift());
+    }
+
+    return promise;
+  }
+
+
+  var newConfig = config;
+  while (requestInterceptorChain.length) {
+    var onFulfilled = requestInterceptorChain.shift();
+    var onRejected = requestInterceptorChain.shift();
+    try {
+      newConfig = onFulfilled(newConfig);
+    } catch (error) {
+      onRejected(error);
+      break;
+    }
+  }
+
+  try {
+    promise = dispatchRequest(newConfig);
+  } catch (error) {
+    return Promise.reject(error);
+  }
+
+  while (responseInterceptorChain.length) {
+    promise = promise.then(responseInterceptorChain.shift(), responseInterceptorChain.shift());
   }
 
   return promise;

lib/core/InterceptorManager.js

@@ -14,10 +14,12 @@ function InterceptorManager() {
  *
  * @return {Number} An ID used to remove interceptor later
  */
-InterceptorManager.prototype.use = function use(fulfilled, rejected) {
+InterceptorManager.prototype.use = function use(fulfilled, rejected, options) {
   this.handlers.push({
     fulfilled: fulfilled,
-    rejected: rejected
+    rejected: rejected,
+    synchronous: options ? options.synchronous : false,
+    runWhen: options ? options.runWhen : null
   });
   return this.handlers.length - 1;
 };

test/specs/adapter.spec.js

@@ -1,19 +1,98 @@
 var axios = require('../../index');
 
 describe('adapter', function () {
-  it('should support custom adapter', function (done) {
+  beforeEach(function () {
-    var called = false;
+    jasmine.Ajax.install();
+  });
 
+  afterEach(function () {
+    jasmine.Ajax.uninstall();
+  });
+
+  it('should support custom adapter', function (done) {
     axios('/foo', {
-      adapter: function (config) {
+      adapter: function barAdapter(config) {
-        called = true;
+        return new Promise(function dispatchXhrRequest(resolve) {
+          var request = new XMLHttpRequest();
+          request.open('GET', '/bar');
+
+          request.onreadystatechange = function () {
+            resolve({
+              config: config,
+              request: request
+            });
+          };
+
+          request.send(null);
+        });
       }
+    }).catch(console.log);
+
+    getAjaxRequest().then(function(request) {
+      expect(request.url).toBe('/bar');
+      done();
+    });
+  });
+
+  it('should execute adapter code synchronously', function (done) {
+    var asyncFlag = false;
+    axios('/foo', {
+      adapter: function barAdapter(config) {
+        return new Promise(function dispatchXhrRequest(resolve) {
+          var request = new XMLHttpRequest();
+          request.open('GET', '/bar');
+
+          request.onreadystatechange = function () {
+            resolve({
+              config: config,
+              request: request
+            });
+          };
+
+          expect(asyncFlag).toBe(false);
+          request.send(null);
+        });
+      }
+    }).catch(console.log);
+
+    asyncFlag = true;
+
+    getAjaxRequest().then(function() {
+      done();
+    });
+  });
+
+  it('should execute adapter code asynchronously when interceptor is present', function (done) {
+    var asyncFlag = false;
+
+    axios.interceptors.request.use(function (config) {
+      config.headers.async = 'async it!';
+      return config;
     });
 
-    setTimeout(function () {
+    axios('/foo', {
-      expect(called).toBe(true);
+      adapter: function barAdapter(config) {
+        return new Promise(function dispatchXhrRequest(resolve) {
+          var request = new XMLHttpRequest();
+          request.open('GET', '/bar');
+
+          request.onreadystatechange = function () {
+            resolve({
+              config: config,
+              request: request
+            });
+          };
+
+          expect(asyncFlag).toBe(true);
+          request.send(null);
+        });
+      }
+    }).catch(console.log);
+
+    asyncFlag = true;
+
+    getAjaxRequest().then(function() {
       done();
-    }, 100);
+    });
   });
 });
-

test/specs/interceptors.spec.js

@@ -9,25 +9,164 @@ describe('interceptors', function () {
     axios.interceptors.response.handlers = [];
   });
 
-  it('should add a request interceptor', function (done) {
+  it('should add a request interceptor (asynchronous by default)', function (done) {
+    var asyncFlag = false;
     axios.interceptors.request.use(function (config) {
       config.headers.test = 'added by interceptor';
+      expect(asyncFlag).toBe(true);
       return config;
     });
 
     axios('/foo');
+    asyncFlag = true;
 
     getAjaxRequest().then(function (request) {
-      request.respondWith({
-        status: 200,
-        responseText: 'OK'
-      });
-
       expect(request.requestHeaders.test).toBe('added by interceptor');
       done();
     });
   });
 
+  it('should add a request interceptor (explicitly flagged as asynchronous)', function (done) {
+    var asyncFlag = false;
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'added by interceptor';
+      expect(asyncFlag).toBe(true);
+      return config;
+    }, null, { synchronous: false });
+
+    axios('/foo');
+    asyncFlag = true;
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.test).toBe('added by interceptor');
+      done();
+    });
+  });
+
+  it('should add a request interceptor that is executed synchronously when flag is provided', function (done) {
+    var asyncFlag = false;
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'added by synchronous interceptor';
+      expect(asyncFlag).toBe(false);
+      return config;
+    }, null, { synchronous: true });
+
+    axios('/foo');
+    asyncFlag = true;
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.test).toBe('added by synchronous interceptor');
+      done();
+    });
+  });
+
+  it('should execute asynchronously when not all interceptors are explicitly flagged as synchronous', function (done) {
+    var asyncFlag = false;
+    axios.interceptors.request.use(function (config) {
+      config.headers.foo = 'uh oh, async';
+      expect(asyncFlag).toBe(true);
+      return config;
+    });
+
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'added by synchronous interceptor';
+      expect(asyncFlag).toBe(true);
+      return config;
+    }, null, { synchronous: true });
+
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'added by the async interceptor';
+      expect(asyncFlag).toBe(true);
+      return config;
+    });
+
+    axios('/foo');
+    asyncFlag = true;
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.foo).toBe('uh oh, async');
+      /* request interceptors have a reversed execution order */
+      expect(request.requestHeaders.test).toBe('added by synchronous interceptor');
+      done();
+    });
+  });
+
+  it('runs the interceptor if runWhen function is provided and resolves to true', function (done) {
+    function onGetCall(config) {
+      return config.method === 'get';
+    }
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'special get headers';
+      return config;
+    }, null, { runWhen: onGetCall });
+
+    axios('/foo');
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.test).toBe('special get headers');
+      done();
+    });
+  });
+
+  it('does not run the interceptor if runWhen function is provided and resolves to false', function (done) {
+    function onPostCall(config) {
+      return config.method === 'post';
+    }
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'special get headers';
+      return config;
+    }, null, { runWhen: onPostCall });
+
+    axios('/foo');
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.test).toBeUndefined()
+      done();
+    });
+  });
+
+  it('does not run async interceptor if runWhen function is provided and resolves to false (and run synchronously)', function (done) {
+    var asyncFlag = false;
+
+    function onPostCall(config) {
+      return config.method === 'post';
+    }
+    axios.interceptors.request.use(function (config) {
+      config.headers.test = 'special get headers';
+      return config;
+    }, null, { synchronous: false, runWhen: onPostCall });
+
+    axios.interceptors.request.use(function (config) {
+      config.headers.sync = 'hello world';
+      expect(asyncFlag).toBe(false);
+      return config;
+    }, null, { synchronous: true });
+
+    axios('/foo');
+    asyncFlag = true
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.test).toBeUndefined()
+      expect(request.requestHeaders.sync).toBe('hello world')
+      done();
+    });
+  });
+
+  it('should add a request interceptor with an onRejected block that is called if interceptor code fails', function (done) {
+    var rejectedSpy = jasmine.createSpy('rejectedSpy');
+    var error = new Error('deadly error');
+    axios.interceptors.request.use(function () {
+      throw error;
+    }, rejectedSpy, { synchronous: true });
+
+    axios('/foo');
+
+    getAjaxRequest().then(function () {
+      expect(rejectedSpy).toHaveBeenCalledWith(error);
+      done();
+    });
+  });
+
   it('should add a request interceptor that returns a new config object', function (done) {
     axios.interceptors.request.use(function () {
       return {
@@ -237,6 +376,32 @@ describe('interceptors', function () {
     });
   });
 
+  it('should remove async interceptor before making request and execute synchronously', function (done) {
+    var asyncFlag = false;
+    var asyncIntercept = axios.interceptors.request.use(function (config) {
+      config.headers.async = 'async it!';
+      return config;
+    }, null, { synchronous: false });
+
+    var syncIntercept = axios.interceptors.request.use(function (config) {
+      config.headers.sync = 'hello world';
+      expect(asyncFlag).toBe(false);
+      return config;
+    }, null, { synchronous: true });
+
+
+    axios.interceptors.request.eject(asyncIntercept);
+
+    axios('/foo')
+    asyncFlag = true
+
+    getAjaxRequest().then(function (request) {
+      expect(request.requestHeaders.async).toBeUndefined();
+      expect(request.requestHeaders.sync).toBe('hello world');
+      done()
+    });
+  });
+
   it('should execute interceptors before transformers', function (done) {
     axios.interceptors.request.use(function (config) {
       config.data.baz = 'qux';