From 62d625603916115691bcea2842c5d6e331279b99 Mon Sep 17 00:00:00 2001 From: Sasha Korotkov Date: Mon, 1 Mar 2021 04:11:35 -0500 Subject: [PATCH] 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 Co-authored-by: Xianming Zhong Co-authored-by: Jay --- README.md | 28 +++++ lib/core/Axios.js | 57 +++++++++-- lib/core/InterceptorManager.js | 6 +- test/specs/adapter.spec.js | 93 +++++++++++++++-- test/specs/interceptors.spec.js | 175 +++++++++++++++++++++++++++++++- 5 files changed, 337 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 06c5b32520..302bcdcce2 100755 --- a/README.md +++ b/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 diff --git a/lib/core/Axios.js b/lib/core/Axios.js index c28c413ad3..8408e1687c 100644 --- a/lib/core/Axios.js +++ b/lib/core/Axios.js @@ -45,20 +45,61 @@ Axios.prototype.request = function request(config) { config.method = 'get'; } - // Hook up interceptors middleware - var chain = [dispatchRequest, undefined]; - var promise = Promise.resolve(config); - + // filter out skipped interceptors + var requestInterceptorChain = []; + 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) { - promise = promise.then(chain.shift(), chain.shift()); + var promise; + + 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; diff --git a/lib/core/InterceptorManager.js b/lib/core/InterceptorManager.js index 50d667bb44..900f44880d 100644 --- a/lib/core/InterceptorManager.js +++ b/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; }; diff --git a/test/specs/adapter.spec.js b/test/specs/adapter.spec.js index 6a00bc9df2..d4428a080f 100644 --- a/test/specs/adapter.spec.js +++ b/test/specs/adapter.spec.js @@ -1,19 +1,98 @@ var axios = require('../../index'); describe('adapter', function () { + beforeEach(function () { + jasmine.Ajax.install(); + }); + + afterEach(function () { + jasmine.Ajax.uninstall(); + }); + it('should support custom adapter', function (done) { - var called = 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 + }); + }; + + 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 (config) { - called = 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(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; + }); + + 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(true); + request.send(null); + }); + } + }).catch(console.log); - setTimeout(function () { - expect(called).toBe(true); + asyncFlag = true; + + getAjaxRequest().then(function() { done(); - }, 100); + }); }); }); - diff --git a/test/specs/interceptors.spec.js b/test/specs/interceptors.spec.js index effbcde258..61b4180840 100644 --- a/test/specs/interceptors.spec.js +++ b/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';