From a5ca1e989b318b0f3796a1ebf2798e7d70370e4d Mon Sep 17 00:00:00 2001 From: Dmitriy Mozgovoy Date: Mon, 19 Apr 2021 19:55:34 +0300 Subject: [PATCH] JSON improvements: throw if JSON parsing failed; number, boolean can be passed directly as payload for encoding to JSON #2613, #61, #907 (#3688) * Draft * 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; * Fixed isOlderVersion helper; Fixed typo; Added validator.spec.js; * Added forcedJSONParsing transitional option #2791 * `transformData` is now called in the default configuration context if the function context is not specified (for tests compatibility); * 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 | 13 +++ index.d.ts | 7 ++ lib/adapters/http.js | 7 +- lib/adapters/xhr.js | 60 ++++++++------ lib/core/Axios.js | 12 +++ lib/core/dispatchRequest.js | 9 ++- lib/core/transformData.js | 4 +- lib/defaults.js | 33 ++++++-- lib/helpers/validator.js | 105 +++++++++++++++++++++++++ test/specs/defaults.spec.js | 2 +- test/specs/helpers/validator.spec.js | 58 ++++++++++++++ test/specs/requests.spec.js | 45 +++++++++++ test/specs/transform.spec.js | 84 ++++++++++++++++++++ test/unit/defaults/transformReponse.js | 2 +- 14 files changed, 403 insertions(+), 38 deletions(-) create mode 100644 lib/helpers/validator.js create mode 100644 test/specs/helpers/validator.spec.js diff --git a/README.md b/README.md index 82447780b4..5def9d52b2 100755 --- a/README.md +++ b/README.md @@ -458,6 +458,19 @@ 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 + + // 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 c74e93ca92..df8c808844 100644 --- a/index.d.ts +++ b/index.d.ts @@ -41,6 +41,12 @@ export type ResponseType = | 'text' | 'stream' +export interface TransitionalOptions{ + silentJSONParsing: boolean; + forcedJSONParsing: boolean; + clarifyTimeoutError: boolean; +} + export interface AxiosRequestConfig { url?: string; method?: Method; @@ -71,6 +77,7 @@ export interface AxiosRequestConfig { proxy?: AxiosProxyConfig | false; cancelToken?: CancelToken; decompress?: boolean; + transitional?: TransitionalOptions } export interface AxiosResponse { diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 02be4651f9..5fb5ccd692 100755 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -284,7 +284,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 3027752e1a..a386dd24aa 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 @@ -33,23 +34,14 @@ 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 = !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, @@ -63,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() { @@ -93,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 @@ -133,16 +151,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..1d9296c6ee 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,16 @@ 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'), + forcedJSONParsing: validators.transitional(validators.boolean, '1.0.0'), + clarifyTimeoutError: validators.transitional(validators.boolean, '1.0.0') + }, false); + } + // 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..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,9 +12,10 @@ var utils = require('./../utils'); * @returns {*} The resulting transformed data */ module.exports = function transformData(data, headers, fns) { + var context = this || defaults; /*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 c7492a5d06..04de40c20e 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,11 +27,19 @@ function getDefaultAdapter() { } var defaults = { + + transitional: { + silentJSONParsing: true, + forcedJSONParsing: true, + clarifyTimeoutError: false + }, + adapter: getDefaultAdapter(), transformRequest: [function transformRequest(data, headers) { normalizeHeaderName(headers, 'Accept'); normalizeHeaderName(headers, 'Content-Type'); + if (utils.isFormData(data) || utils.isArrayBuffer(data) || utils.isBuffer(data) || @@ -47,7 +56,7 @@ var defaults = { setContentTypeIfUnset(headers, 'application/x-www-form-urlencoded;charset=utf-8'); return data.toString(); } - if (utils.isObject(data)) { + if (utils.isObject(data) || (headers && headers['Content-Type'] === 'application/json')) { setContentTypeIfUnset(headers, 'application/json;charset=utf-8'); return JSON.stringify(data); } @@ -55,13 +64,25 @@ var defaults = { }], transformResponse: [function transformResponse(data) { - var result = data; - if (utils.isString(result) && result.length) { + var transitional = this.transitional; + var silentJSONParsing = transitional && transitional.silentJSONParsing; + var forcedJSONParsing = transitional && transitional.forcedJSONParsing; + var strictJSONParsing = !silentJSONParsing && this.responseType === 'json'; + + if (strictJSONParsing || (forcedJSONParsing && utils.isString(data) && data.length)) { try { - result = JSON.parse(result); - } catch (e) { /* Ignore */ } + return JSON.parse(data); + } catch (e) { + if (strictJSONParsing) { + if (e.name === 'SyntaxError') { + throw enhanceError(e, this, 'E_JSON_PARSE'); + } + throw e; + } + } } - return result; + + return data; }], /** diff --git a/lib/helpers/validator.js b/lib/helpers/validator.js new file mode 100644 index 0000000000..7f1bc7dfa9 --- /dev/null +++ b/lib/helpers/validator.js @@ -0,0 +1,105 @@ +'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 currentVerArr = pkg.version.split('.'); + +/** + * Compare package versions + * @param {string} version + * @param {string?} thanVersion + * @returns {boolean} + */ +function isOlderVersion(version, thanVersion) { + var pkgVersionArr = thanVersion ? thanVersion.split('.') : currentVerArr; + var destVer = version.split('.'); + for (var i = 0; i < 3; i++) { + if (pkgVersionArr[i] > destVer[i]) { + return true; + } else if (pkgVersionArr[i] < destVer[i]) { + return false; + } + } + return false; +} + +/** + * 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, + ' 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 = { + isOlderVersion: isOlderVersion, + assertOptions: assertOptions, + validators: validators +}; 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/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(); + }); +}); 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(); diff --git a/test/specs/transform.spec.js b/test/specs/transform.spec.js index 5199ecbbe3..86d5b5344b 100644 --- a/test/specs/transform.spec.js +++ b/test/specs/transform.spec.js @@ -41,6 +41,90 @@ 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 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' 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 +});