From c26b1b5e6e751de7f238f12909ad3660404411bf Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Thu, 18 Mar 2021 22:04:50 +0200 Subject: [PATCH 1/7] Draft --- lib/adapters/xhr.js | 12 +++--------- lib/defaults.js | 12 +++++++++--- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/adapters/xhr.js b/lib/adapters/xhr.js index f83f1444bc..16a4b9b794 100644 --- a/lib/adapters/xhr.js +++ b/lib/adapters/xhr.js @@ -14,15 +14,9 @@ module.exports = function xhrAdapter(config) { var requestData = config.data; var requestHeaders = config.headers; - if (utils.isFormData(requestData)) { - delete requestHeaders['Content-Type']; // Let the browser set it - } - - if ( - (utils.isBlob(requestData) || utils.isFile(requestData)) && - requestData.type - ) { - delete requestHeaders['Content-Type']; // Let the browser set it + if (utils.isFormData(requestData) || + (utils.isBlob(requestData) || utils.isFile(requestData)) && requestData.type) { + delete requestHeaders['content-type']; // Let the browser set it } var request = new XMLHttpRequest(); diff --git a/lib/defaults.js b/lib/defaults.js index 2b2a1a7d55..bcc2b352de 100644 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -31,6 +31,9 @@ var defaults = { transformRequest: [function transformRequest(data, headers) { normalizeHeaderName(headers, 'Accept'); normalizeHeaderName(headers, 'Content-Type'); + + var requestContentType = headers['Content-Type']; + if (utils.isFormData(data) || utils.isArrayBuffer(data) || utils.isBuffer(data) || @@ -47,19 +50,22 @@ var defaults = { setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8'); return data.toString(); } - if (utils.isObject(data)) { + if (utils.isObject(data) || requestContentType === 'application/json') { setContentTypeIfUnset(headers, 'application/json;charset=utf-8'); return JSON.stringify(data); } return data; }], - transformResponse: [function transformResponse(data) { + transformResponse: [function transformResponse(data, headers) { + console.log('transformResponse', arguments); /*eslint no-param-reassign:0*/ if (typeof data === 'string') { try { data = JSON.parse(data); - } catch (e) { /* Ignore */ } + } catch (e) { + + } } return data; }], From bc402a76fa8d387e13935bc15d7dcfd16b1ace63 Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Fri, 19 Mar 2021 17:21:26 +0200 Subject: [PATCH 2/7] Added support for primitive types to be converted to JSON if the request Content-Type is 'application/json'; Added throwing SyntaxError if JSON parsing failed and responseType is json; Added transitional option object; Added options validator to assert transitional options; Added transitional option `silentJSONParsing= true` for backward compatibility; Updated README.md; Updated typings; --- README.md | 7 +++ index.d.ts | 5 ++ lib/adapters/xhr.js | 16 ++---- lib/core/Axios.js | 10 ++++ lib/core/dispatchRequest.js | 9 ++-- lib/core/transformData.js | 3 +- lib/defaults.js | 28 +++++++--- lib/helpers/validator.js | 101 +++++++++++++++++++++++++++++++++++ test/specs/transform.spec.js | 54 +++++++++++++++++++ 9 files changed, 210 insertions(+), 23 deletions(-) create mode 100644 lib/helpers/validator.js diff --git a/README.md b/README.md index 302bcdcce2..6e9e22ec6c 100755 --- a/README.md +++ b/README.md @@ -455,6 +455,13 @@ These are the available config options for making requests. Only the `url` is re // - Node only (XHR cannot turn off decompression) decompress: true // default + // transitional options for backward compatibility that may be removed in the newer versions + transitional: { + // silent JSON parsing mode + // `true` - ignore JSON parsing errors and set response.data to null if parsing failed (old behaviour) + // `false` - throw SyntaxError if JSON parsing failed (Note: responseType must be set to 'json') + silentJSONParsing: true // default value for the current Axios version + } } ``` diff --git a/index.d.ts b/index.d.ts index c74e93ca92..4456549931 100644 --- a/index.d.ts +++ b/index.d.ts @@ -41,6 +41,10 @@ export type ResponseType = | 'text' | 'stream' +export interface TransitionalOptions{ + silentJSONParsing: boolean; +} + export interface AxiosRequestConfig { url?: string; method?: Method; @@ -71,6 +75,7 @@ export interface AxiosRequestConfig { proxy?: AxiosProxyConfig | false; cancelToken?: CancelToken; decompress?: boolean; + transitional?: TransitionalOptions } export interface AxiosResponse { diff --git a/lib/adapters/xhr.js b/lib/adapters/xhr.js index 3027752e1a..3e0de1799d 100644 --- a/lib/adapters/xhr.js +++ b/lib/adapters/xhr.js @@ -13,6 +13,7 @@ module.exports = function xhrAdapter(config) { return new Promise(function dispatchXhrRequest(resolve, reject) { var requestData = config.data; var requestHeaders = config.headers; + var responseType = config.responseType; if (utils.isFormData(requestData)) { delete requestHeaders['Content-Type']; // Let the browser set it @@ -49,7 +50,8 @@ module.exports = function xhrAdapter(config) { // Prepare the response var responseHeaders = 'getAllResponseHeaders' in request ? parseHeaders(request.getAllResponseHeaders()) : null; - var responseData = !config.responseType || config.responseType === 'text' ? request.responseText : request.response; + var responseData = !responseType || responseType === 'text' || responseType === 'json' ? + request.responseText : request.response; var response = { data: responseData, status: request.status, @@ -133,16 +135,8 @@ module.exports = function xhrAdapter(config) { } // Add responseType to request if needed - if (config.responseType) { - try { - request.responseType = config.responseType; - } catch (e) { - // Expected DOMException thrown by browsers not compatible XMLHttpRequest Level 2. - // But, this can be suppressed for 'json' type as it can be parsed by default 'transformResponse' function. - if (config.responseType !== 'json') { - throw e; - } - } + if (responseType && responseType !== 'json') { + request.responseType = config.responseType; } // Handle progress if needed diff --git a/lib/core/Axios.js b/lib/core/Axios.js index 8408e1687c..4403dabb0e 100644 --- a/lib/core/Axios.js +++ b/lib/core/Axios.js @@ -5,7 +5,9 @@ var buildURL = require('../helpers/buildURL'); var InterceptorManager = require('./InterceptorManager'); var dispatchRequest = require('./dispatchRequest'); var mergeConfig = require('./mergeConfig'); +var validator = require('../helpers/validator'); +var validators = validator.validators; /** * Create a new instance of Axios * @@ -45,6 +47,14 @@ Axios.prototype.request = function request(config) { config.method = 'get'; } + var transitional = config.transitional; + + if (transitional !== undefined) { + validator.assertOptions(transitional, { + silentJSONParsing: validators.transitional(validators.boolean, '1.0.0') + }); + } + // filter out skipped interceptors var requestInterceptorChain = []; var synchronousRequestInterceptors = true; diff --git a/lib/core/dispatchRequest.js b/lib/core/dispatchRequest.js index c8267adb2a..9ce3b96e6d 100644 --- a/lib/core/dispatchRequest.js +++ b/lib/core/dispatchRequest.js @@ -27,7 +27,8 @@ module.exports = function dispatchRequest(config) { config.headers = config.headers || {}; // Transform request data - config.data = transformData( + config.data = transformData.call( + config, config.data, config.headers, config.transformRequest @@ -53,7 +54,8 @@ module.exports = function dispatchRequest(config) { throwIfCancellationRequested(config); // Transform response data - response.data = transformData( + response.data = transformData.call( + config, response.data, response.headers, config.transformResponse @@ -66,7 +68,8 @@ module.exports = function dispatchRequest(config) { // Transform response data if (reason && reason.response) { - reason.response.data = transformData( + reason.response.data = transformData.call( + config, reason.response.data, reason.response.headers, config.transformResponse diff --git a/lib/core/transformData.js b/lib/core/transformData.js index e0653620e6..3dd7e5af40 100644 --- a/lib/core/transformData.js +++ b/lib/core/transformData.js @@ -11,9 +11,10 @@ var utils = require('./../utils'); * @returns {*} The resulting transformed data */ module.exports = function transformData(data, headers, fns) { + var context = this; /*eslint no-param-reassign:0*/ utils.forEach(fns, function transform(fn) { - data = fn(data, headers); + data = fn.call(context, data, headers); }); return data; diff --git a/lib/defaults.js b/lib/defaults.js index bcc2b352de..6dc0907ede 100644 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -2,6 +2,7 @@ var utils = require('./utils'); var normalizeHeaderName = require('./helpers/normalizeHeaderName'); +var enhanceError = require('./core/enhanceError'); var DEFAULT_CONTENT_TYPE = { 'Content-Type': 'application/x-www-form-urlencoded' @@ -26,14 +27,17 @@ function getDefaultAdapter() { } var defaults = { + + transitional: { + silentJSONParsing: true + }, + adapter: getDefaultAdapter(), transformRequest: [function transformRequest(data, headers) { normalizeHeaderName(headers, 'Accept'); normalizeHeaderName(headers, 'Content-Type'); - var requestContentType = headers['Content-Type']; - if (utils.isFormData(data) || utils.isArrayBuffer(data) || utils.isBuffer(data) || @@ -50,23 +54,31 @@ var defaults = { setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8'); return data.toString(); } - if (utils.isObject(data) || requestContentType === 'application/json') { + if (utils.isObject(data) || (headers && headers['Content-Type'] === 'application/json')) { setContentTypeIfUnset(headers, 'application/json;charset=utf-8'); return JSON.stringify(data); } return data; }], - transformResponse: [function transformResponse(data, headers) { - console.log('transformResponse', arguments); - /*eslint no-param-reassign:0*/ - if (typeof data === 'string') { + transformResponse: [function transformResponse(data) { + var silentJSONParsing = this.transitional && this.transitional.silentJSONParsing; + var strictJSONParsing = !silentJSONParsing && this.responseType === 'json'; + + if (strictJSONParsing || typeof data === 'string') { try { - data = JSON.parse(data); + return JSON.parse(data); } catch (e) { + if (strictJSONParsing) { + if (e.name === 'SyntaxError') { + throw enhanceError(e, this, 'E_JSON_PARSE'); + } + throw e; + } } } + return data; }], diff --git a/lib/helpers/validator.js b/lib/helpers/validator.js new file mode 100644 index 0000000000..b07ed8d62b --- /dev/null +++ b/lib/helpers/validator.js @@ -0,0 +1,101 @@ +'use strict'; + +var pkg = require('./../../package.json'); + +var validators = {}; + +// eslint-disable-next-line func-names +['object', 'boolean', 'number', 'function', 'string', 'symbol'].forEach(function(type, i) { + validators[type] = function validator(thing) { + return typeof thing === type || 'a' + (i < 1 ? 'n ' : ' ') + type; + }; +}); + +var deprecatedWarnings = {}; +var currentVer = pkg.version.split('.'); + +/** + * Compare package versions + * @param {string} version + * @returns {boolean} + */ + +function isOlderVersion(version) { + var destVer = version.split('.'); + for (var i = 0; i < 3; i++) { + if (currentVer[i] < destVer[i]) { + return false; + } + } + return true; +} + +/** + * Transitional option validator + * @param {function|boolean?} validator + * @param {string?} version + * @param {string} message + * @returns {function} + */ +validators.transitional = function transitional(validator, version, message) { + var isDeprecated = version && isOlderVersion(version); + + function formatMessage(opt, desc) { + return '[Axios v' + pkg.version + '] Transitional option \'' + opt + '\'' + desc + (message ? '. ' + message : ''); + } + + // eslint-disable-next-line func-names + return function(value, opt, opts) { + if (validator === false) { + throw new Error(formatMessage(opt, ' has been removed in ' + version)); + } + + if (isDeprecated && !deprecatedWarnings[opt]) { + deprecatedWarnings[opt] = true; + // eslint-disable-next-line no-console + console.warn( + formatMessage( + opt, + ' is has been deprecated since v' + version + ' and will be removed in the near future' + ) + ); + } + + return validator ? validator(value, opt, opts) : true; + }; +}; + +/** + * Assert object's properties type + * @param {object} options + * @param {object} schema + * @param {boolean} allowUnknown + */ + +function assertOptions(options, schema, allowUnknown) { + if (typeof options !== 'object') { + throw new TypeError('options must be an object'); + } + var keys = Object.keys(options); + var i = keys.length; + while (i-- > 0) { + var opt = keys[i]; + var validator = schema[opt]; + if (validator) { + var value = options[opt]; + var result = value === undefined || validator(value, opt, options); + if (result !== true) { + throw new TypeError('option ' + opt + ' must be ' + result); + } + continue; + } + if (allowUnknown !== true) { + throw Error('Unknown option ' + opt); + } + } +} + +module.exports = { + assertOptions: assertOptions, + validators: validators +}; diff --git a/test/specs/transform.spec.js b/test/specs/transform.spec.js index 5199ecbbe3..a166a391ea 100644 --- a/test/specs/transform.spec.js +++ b/test/specs/transform.spec.js @@ -41,6 +41,60 @@ describe('transform', function () { }); }); + it('should throw a SyntaxError if JSON parsing failed and responseType is "json" if silentJSONParsing is false', + function (done) { + var thrown; + + axios({ + url: '/foo', + responseType: 'json', + transitional: {silentJSONParsing: false} + }).then(function () { + done(new Error('should fail')); + }, function (err) { + thrown = err; + }); + + getAjaxRequest().then(function (request) { + request.respondWith({ + status: 200, + responseText: '{foo": "bar"}' // JSON SyntaxError + }); + + setTimeout(function () { + expect(thrown).toBeTruthy(); + expect(thrown.name).toContain('SyntaxError'); + expect(thrown.message).toContain('JSON'); + done(); + }, 100); + }); + } + ); + + it('should send data as JSON if request content-type is application/json', function (done) { + var response; + + axios.post('/foo', 123, {headers: {'Content-Type': 'application/json'}}).then(function (_response) { + response = _response; + }, function (err) { + done(err); + }); + + getAjaxRequest().then(function (request) { + request.respondWith({ + status: 200, + responseText: '' + }); + + setTimeout(function () { + expect(response).toBeTruthy(); + expect(request.requestHeaders['Content-Type']).toBe('application/json'); + expect(JSON.parse(request.params)).toBe(123); + done(); + }, 100); + }); + }); + it('should override default transform', function (done) { var data = { foo: 'bar' From 5112ba3ee1cdd287638f4cc81d7dd0edaf67ca15 Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Fri, 19 Mar 2021 19:17:12 +0200 Subject: [PATCH 3/7] Fixed isOlderVersion helper; Fixed typo; Added validator.spec.js; --- lib/helpers/validator.js | 16 +++++--- test/specs/helpers/validator.spec.js | 58 ++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 6 deletions(-) create mode 100644 test/specs/helpers/validator.spec.js diff --git a/lib/helpers/validator.js b/lib/helpers/validator.js index b07ed8d62b..e591071c18 100644 --- a/lib/helpers/validator.js +++ b/lib/helpers/validator.js @@ -12,22 +12,25 @@ var validators = {}; }); var deprecatedWarnings = {}; -var currentVer = pkg.version.split('.'); +var currentVerArr = pkg.version.split('.'); /** * Compare package versions * @param {string} version + * @param {string?} thanVersion * @returns {boolean} */ - -function isOlderVersion(version) { +function isOlderVersion(version, thanVersion) { + var pkgVersionArr = thanVersion ? thanVersion.split('.') : currentVerArr; var destVer = version.split('.'); for (var i = 0; i < 3; i++) { - if (currentVer[i] < destVer[i]) { + if (pkgVersionArr[i] > destVer[i]) { + return true; + } else if (pkgVersionArr[i] < destVer[i]) { return false; } } - return true; + return false; } /** @@ -56,7 +59,7 @@ validators.transitional = function transitional(validator, version, message) { console.warn( formatMessage( opt, - ' is has been deprecated since v' + version + ' and will be removed in the near future' + ' has been deprecated since v' + version + ' and will be removed in the near future' ) ); } @@ -96,6 +99,7 @@ function assertOptions(options, schema, allowUnknown) { } module.exports = { + isOlderVersion: isOlderVersion, assertOptions: assertOptions, validators: validators }; diff --git a/test/specs/helpers/validator.spec.js b/test/specs/helpers/validator.spec.js new file mode 100644 index 0000000000..9a125d4139 --- /dev/null +++ b/test/specs/helpers/validator.spec.js @@ -0,0 +1,58 @@ +'use strict'; + +var validator = require('../../../lib/helpers/validator'); + +describe('validator::isOlderVersion', function () { + it('should return true if dest version is older than the package version', function () { + expect(validator.isOlderVersion('0.0.1', '1.0.0')).toEqual(true); + expect(validator.isOlderVersion('0.0.1', '0.1.0')).toEqual(true); + expect(validator.isOlderVersion('0.0.1', '0.0.1')).toEqual(false); + + + expect(validator.isOlderVersion('100.0.0', '1.0.0')).toEqual(false); + expect(validator.isOlderVersion('100.0.0', '0.1.0')).toEqual(false); + expect(validator.isOlderVersion('100.0.0', '0.0.1')).toEqual(false); + + expect(validator.isOlderVersion('0.10000.0', '1000.0.1')).toEqual(true); + }); +}); + +describe('validator::assertOptions', function () { + it('should throw only if unknown an option was passed', function () { + expect(function() { + validator.assertOptions({ + x: true + }, { + y: validator.validators.boolean + }); + }).toThrow(new Error('Unknown option x')); + + expect(function() { + validator.assertOptions({ + x: true + }, { + x: validator.validators.boolean, + y: validator.validators.boolean + }); + }).not.toThrow(new Error('Unknown option x')); + }); + + it('should throw TypeError only if option type doesn\'t match', function () { + expect(function() { + validator.assertOptions({ + x: 123 + }, { + x: validator.validators.boolean + }); + }).toThrow(new TypeError('option x must be a boolean')); + + expect(function() { + validator.assertOptions({ + x: true + }, { + x: validator.validators.boolean, + y: validator.validators.boolean + }); + }).not.toThrow(); + }); +}); From aa6360589025e569adbc3ab8df070f62664c64f7 Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Fri, 19 Mar 2021 22:20:57 +0200 Subject: [PATCH 4/7] Added forcedJSONParsing transitional option #2791 --- README.md | 7 +++++-- index.d.ts | 1 + lib/core/Axios.js | 3 ++- lib/defaults.js | 9 ++++++--- test/specs/defaults.spec.js | 2 +- test/specs/transform.spec.js | 30 ++++++++++++++++++++++++++++++ 6 files changed, 45 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 6e9e22ec6c..35c6bedbda 100755 --- a/README.md +++ b/README.md @@ -458,9 +458,12 @@ These are the available config options for making requests. Only the `url` is re // transitional options for backward compatibility that may be removed in the newer versions transitional: { // silent JSON parsing mode - // `true` - ignore JSON parsing errors and set response.data to null if parsing failed (old behaviour) + // `true` - ignore JSON parsing errors and set response.data to null if parsing failed (old behaviour) // `false` - throw SyntaxError if JSON parsing failed (Note: responseType must be set to 'json') - silentJSONParsing: true // default value for the current Axios version + silentJSONParsing: true; // default value for the current Axios version + + // try to parse the response string as JSON even if `resposeType` is not 'json' + forcedJSONParsing: true; } } ``` diff --git a/index.d.ts b/index.d.ts index 4456549931..7582a3e8c2 100644 --- a/index.d.ts +++ b/index.d.ts @@ -43,6 +43,7 @@ export type ResponseType = export interface TransitionalOptions{ silentJSONParsing: boolean; + forcedJSONParsing: boolean; } export interface AxiosRequestConfig { diff --git a/lib/core/Axios.js b/lib/core/Axios.js index 4403dabb0e..5cb91de20b 100644 --- a/lib/core/Axios.js +++ b/lib/core/Axios.js @@ -51,7 +51,8 @@ Axios.prototype.request = function request(config) { if (transitional !== undefined) { validator.assertOptions(transitional, { - silentJSONParsing: validators.transitional(validators.boolean, '1.0.0') + silentJSONParsing: validators.transitional(validators.boolean, '1.0.0'), + forcedJSONParsing: validators.transitional(validators.boolean, '1.0.0') }); } diff --git a/lib/defaults.js b/lib/defaults.js index 6dc0907ede..b36bd90ad6 100644 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -29,7 +29,8 @@ function getDefaultAdapter() { var defaults = { transitional: { - silentJSONParsing: true + silentJSONParsing: true, + forcedJSONParsing: true }, adapter: getDefaultAdapter(), @@ -62,10 +63,12 @@ var defaults = { }], transformResponse: [function transformResponse(data) { - var silentJSONParsing = this.transitional && this.transitional.silentJSONParsing; + var transitional = this.transitional; + var silentJSONParsing = transitional && transitional.silentJSONParsing; + var forcedJSONParsing = transitional && transitional.forcedJSONParsing; var strictJSONParsing = !silentJSONParsing && this.responseType === 'json'; - if (strictJSONParsing || typeof data === 'string') { + if (strictJSONParsing || (forcedJSONParsing && typeof data === 'string')) { try { return JSON.parse(data); } catch (e) { diff --git a/test/specs/defaults.spec.js b/test/specs/defaults.spec.js index c8ee72eab2..d71cd66a4e 100644 --- a/test/specs/defaults.spec.js +++ b/test/specs/defaults.spec.js @@ -25,7 +25,7 @@ describe('defaults', function () { }); it('should transform response json', function () { - var data = defaults.transformResponse[0]('{"foo":"bar"}'); + var data = defaults.transformResponse[0].call(defaults, '{"foo":"bar"}'); expect(typeof data).toEqual('object'); expect(data.foo).toEqual('bar'); diff --git a/test/specs/transform.spec.js b/test/specs/transform.spec.js index a166a391ea..86d5b5344b 100644 --- a/test/specs/transform.spec.js +++ b/test/specs/transform.spec.js @@ -95,6 +95,36 @@ describe('transform', function () { }); }); + it('should not assume JSON if responseType is not `json`', function (done) { + var response; + + axios.get('/foo', { + responseType: 'text', + transitional: { + forcedJSONParsing: false + } + }).then(function (_response) { + response = _response; + }, function (err) { + done(err); + }); + + var rawData = '{"x":1}'; + + getAjaxRequest().then(function (request) { + request.respondWith({ + status: 200, + responseText: rawData + }); + + setTimeout(function () { + expect(response).toBeTruthy(); + expect(response.data).toBe(rawData); + done(); + }, 100); + }); + }); + it('should override default transform', function (done) { var data = { foo: 'bar' From ba6d2bc3017ff47dc090a398bd8fceec03ac4d02 Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Wed, 24 Mar 2021 14:51:33 +0200 Subject: [PATCH 5/7] `transformData` is now called in the default configuration context if the function context is not specified (for tests compatibility); --- lib/core/transformData.js | 3 ++- lib/defaults.js | 3 +-- test/unit/defaults/transformReponse.js | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/core/transformData.js b/lib/core/transformData.js index 3dd7e5af40..c584d12bc2 100644 --- a/lib/core/transformData.js +++ b/lib/core/transformData.js @@ -1,6 +1,7 @@ 'use strict'; var utils = require('./../utils'); +var defaults = require('./../defaults'); /** * Transform the data for a request or a response @@ -11,7 +12,7 @@ var utils = require('./../utils'); * @returns {*} The resulting transformed data */ module.exports = function transformData(data, headers, fns) { - var context = this; + var context = this || defaults; /*eslint no-param-reassign:0*/ utils.forEach(fns, function transform(fn) { data = fn.call(context, data, headers); diff --git a/lib/defaults.js b/lib/defaults.js index 5729ca0bb7..42196f8618 100644 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -68,7 +68,7 @@ var defaults = { var forcedJSONParsing = transitional && transitional.forcedJSONParsing; var strictJSONParsing = !silentJSONParsing && this.responseType === 'json'; - if (strictJSONParsing || (forcedJSONParsing && utils.isString(result) && result.length)) { + if (strictJSONParsing || (forcedJSONParsing && utils.isString(data) && data.length)) { try { return JSON.parse(data); } catch (e) { @@ -76,7 +76,6 @@ var defaults = { if (e.name === 'SyntaxError') { throw enhanceError(e, this, 'E_JSON_PARSE'); } - throw e; } } diff --git a/test/unit/defaults/transformReponse.js b/test/unit/defaults/transformReponse.js index 162b98d717..450069e993 100644 --- a/test/unit/defaults/transformReponse.js +++ b/test/unit/defaults/transformReponse.js @@ -27,4 +27,4 @@ describe('transformResponse', function () { assert.strictEqual(result, data); }); }); -}); \ No newline at end of file +}); From e1340e0ca0672d16e7079d4c944748477551d48a Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Mon, 5 Apr 2021 19:38:13 +0300 Subject: [PATCH 6/7] Added `transitional.clarifyTimeoutError` to throw ETIMEDOUT error instead of generic ECONNABORTED on request timeouts; Added support of onloadend handler if available instead of onreadystatechange; Added xhr timeout test; Fixed potential bug of xhr adapter with proper handling timeouts&errors (FakeXMLHTTPRequest failed to handle timeouts); --- README.md | 3 +++ index.d.ts | 1 + lib/adapters/http.js | 7 +++++- lib/adapters/xhr.js | 44 ++++++++++++++++++++++++------------ lib/core/Axios.js | 5 +++-- lib/defaults.js | 3 ++- lib/helpers/validator.js | 2 +- test/specs/requests.spec.js | 45 +++++++++++++++++++++++++++++++++++++ 8 files changed, 91 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 35c6bedbda..18d2f3014c 100755 --- a/README.md +++ b/README.md @@ -464,6 +464,9 @@ These are the available config options for making requests. Only the `url` is re // try to parse the response string as JSON even if `resposeType` is not 'json' forcedJSONParsing: true; + + // throw ETIMEDOUT error instead of generic ECONNABORTED on request timeouts + clarifyTimeoutError: false; } } ``` diff --git a/index.d.ts b/index.d.ts index 7582a3e8c2..df8c808844 100644 --- a/index.d.ts +++ b/index.d.ts @@ -44,6 +44,7 @@ export type ResponseType = export interface TransitionalOptions{ silentJSONParsing: boolean; forcedJSONParsing: boolean; + clarifyTimeoutError: boolean; } export interface AxiosRequestConfig { diff --git a/lib/adapters/http.js b/lib/adapters/http.js index f32241f77e..4fd6c1724f 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -277,7 +277,12 @@ module.exports = function httpAdapter(config) { // ClientRequest.setTimeout will be fired on the specify milliseconds, and can make sure that abort() will be fired after connect. req.setTimeout(config.timeout, function handleRequestTimeout() { req.abort(); - reject(createError('timeout of ' + config.timeout + 'ms exceeded', config, 'ECONNABORTED', req)); + reject(createError( + 'timeout of ' + config.timeout + 'ms exceeded', + config, + config.transitional && config.transitional.clarifyTimeoutError ? 'ETIMEDOUT' : 'ECONNABORTED', + req + )); }); } diff --git a/lib/adapters/xhr.js b/lib/adapters/xhr.js index 3e0de1799d..a386dd24aa 100644 --- a/lib/adapters/xhr.js +++ b/lib/adapters/xhr.js @@ -34,20 +34,10 @@ module.exports = function xhrAdapter(config) { // Set the request timeout in MS request.timeout = config.timeout; - // Listen for ready state - request.onreadystatechange = function handleLoad() { - if (!request || request.readyState !== 4) { - return; - } - - // The request errored out and we didn't get a response, this will be - // handled by onerror instead - // With one exception: request that using file: protocol, most browsers - // will return status as 0 even though it's a successful request - if (request.status === 0 && !(request.responseURL && request.responseURL.indexOf('file:') === 0)) { + function onloadend() { + if (!request) { return; } - // Prepare the response var responseHeaders = 'getAllResponseHeaders' in request ? parseHeaders(request.getAllResponseHeaders()) : null; var responseData = !responseType || responseType === 'text' || responseType === 'json' ? @@ -65,7 +55,30 @@ module.exports = function xhrAdapter(config) { // Clean up request request = null; - }; + } + + if ('onloadend' in request) { + // Use onloadend if available + request.onloadend = onloadend; + } else { + // Listen for ready state to emulate onloadend + request.onreadystatechange = function handleLoad() { + if (!request || request.readyState !== 4) { + return; + } + + // The request errored out and we didn't get a response, this will be + // handled by onerror instead + // With one exception: request that using file: protocol, most browsers + // will return status as 0 even though it's a successful request + if (request.status === 0 && !(request.responseURL && request.responseURL.indexOf('file:') === 0)) { + return; + } + // readystate handler is calling before onerror or ontimeout handlers, + // so we should call onloadend on the next 'tick' + setTimeout(onloadend); + }; + } // Handle browser request cancellation (as opposed to a manual cancellation) request.onabort = function handleAbort() { @@ -95,7 +108,10 @@ module.exports = function xhrAdapter(config) { if (config.timeoutErrorMessage) { timeoutErrorMessage = config.timeoutErrorMessage; } - reject(createError(timeoutErrorMessage, config, 'ECONNABORTED', + reject(createError( + timeoutErrorMessage, + config, + config.transitional && config.transitional.clarifyTimeoutError ? 'ETIMEDOUT' : 'ECONNABORTED', request)); // Clean up request diff --git a/lib/core/Axios.js b/lib/core/Axios.js index 5cb91de20b..1d9296c6ee 100644 --- a/lib/core/Axios.js +++ b/lib/core/Axios.js @@ -52,8 +52,9 @@ Axios.prototype.request = function request(config) { if (transitional !== undefined) { validator.assertOptions(transitional, { silentJSONParsing: validators.transitional(validators.boolean, '1.0.0'), - forcedJSONParsing: validators.transitional(validators.boolean, '1.0.0') - }); + forcedJSONParsing: validators.transitional(validators.boolean, '1.0.0'), + clarifyTimeoutError: validators.transitional(validators.boolean, '1.0.0') + }, false); } // filter out skipped interceptors diff --git a/lib/defaults.js b/lib/defaults.js index 42196f8618..04de40c20e 100644 --- a/lib/defaults.js +++ b/lib/defaults.js @@ -30,7 +30,8 @@ var defaults = { transitional: { silentJSONParsing: true, - forcedJSONParsing: true + forcedJSONParsing: true, + clarifyTimeoutError: false }, adapter: getDefaultAdapter(), diff --git a/lib/helpers/validator.js b/lib/helpers/validator.js index e591071c18..7f1bc7dfa9 100644 --- a/lib/helpers/validator.js +++ b/lib/helpers/validator.js @@ -72,7 +72,7 @@ validators.transitional = function transitional(validator, version, message) { * Assert object's properties type * @param {object} options * @param {object} schema - * @param {boolean} allowUnknown + * @param {boolean?} allowUnknown */ function assertOptions(options, schema, allowUnknown) { diff --git a/test/specs/requests.spec.js b/test/specs/requests.spec.js index 692dbb04f3..a5aa81aab4 100644 --- a/test/specs/requests.spec.js +++ b/test/specs/requests.spec.js @@ -63,6 +63,51 @@ describe('requests', function () { }); }); + describe('timeouts', function(){ + beforeEach(function () { + jasmine.clock().install(); + }); + + afterEach(function () { + jasmine.clock().uninstall(); + }); + + it('should handle timeouts', function (done) { + axios({ + url: '/foo', + timeout: 100 + }).then(function () { + fail(new Error('timeout error not caught')); + }, function (err) { + expect(err instanceof Error).toBe(true); + expect(err.code).toEqual('ECONNABORTED'); + done(); + }); + + jasmine.Ajax.requests.mostRecent().responseTimeout(); + }); + + describe('transitional.clarifyTimeoutError', function () { + it('should activate throwing ETIMEDOUT instead of ECONNABORTED on request timeouts', function (done) { + axios({ + url: '/foo', + timeout: 100, + transitional: { + clarifyTimeoutError: true + } + }).then(function () { + fail(new Error('timeout error not caught')); + }, function (err) { + expect(err instanceof Error).toBe(true); + expect(err.code).toEqual('ETIMEDOUT'); + done(); + }); + + jasmine.Ajax.requests.mostRecent().responseTimeout(); + }); + }); + }); + it('should reject on network errors', function (done) { // disable jasmine.Ajax since we're hitting a non-existent server anyway jasmine.Ajax.uninstall(); From 968a7b109d2cf1733cd1d1eebd3b31a11fc2b12c Mon Sep 17 00:00:00 2001 From: DigitalBrainJS Date: Mon, 19 Apr 2021 20:25:59 +0300 Subject: [PATCH 7/7] Removed unnecessary assertion; --- test/specs/transform.spec.js | 1 - 1 file changed, 1 deletion(-) diff --git a/test/specs/transform.spec.js b/test/specs/transform.spec.js index 86d5b5344b..6f81f46944 100644 --- a/test/specs/transform.spec.js +++ b/test/specs/transform.spec.js @@ -64,7 +64,6 @@ describe('transform', function () { setTimeout(function () { expect(thrown).toBeTruthy(); expect(thrown.name).toContain('SyntaxError'); - expect(thrown.message).toContain('JSON'); done(); }, 100); });