Skip to content

Commit

Permalink
JSON improvements: throw if JSON parsing failed; number, boolean can …
Browse files Browse the repository at this point in the history
…be passed directly as payload for encoding to JSON axios#2613, axios#61, axios#907 (axios#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 axios#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);
  • Loading branch information
DigitalBrainJS committed Apr 19, 2021
1 parent d99d5fa commit 5ad6994
Show file tree
Hide file tree
Showing 14 changed files with 403 additions and 38 deletions.
13 changes: 13 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
```

Expand Down
7 changes: 7 additions & 0 deletions index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -71,6 +77,7 @@ export interface AxiosRequestConfig {
proxy?: AxiosProxyConfig | false;
cancelToken?: CancelToken;
decompress?: boolean;
transitional?: TransitionalOptions
}

export interface AxiosResponse<T = any> {
Expand Down
7 changes: 6 additions & 1 deletion lib/adapters/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
));
});
}

Expand Down
60 changes: 35 additions & 25 deletions lib/adapters/xhr.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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() {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions lib/core/Axios.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
Expand Down
9 changes: 6 additions & 3 deletions lib/core/dispatchRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 3 additions & 1 deletion lib/core/transformData.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

var utils = require('./../utils');
var defaults = require('./../defaults');

/**
* Transform the data for a request or a response
Expand All @@ -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;
Expand Down
33 changes: 27 additions & 6 deletions lib/defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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) ||
Expand All @@ -47,21 +56,33 @@ 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')) {

This comment has been minimized.

Copy link
@burgalon

burgalon Sep 5, 2021

@timemachine3030 this is causing a bug where GET requests which have content-type: application/json add body null because now it's being parsed. This was not the case for axios 0.21.1 and is causing failures on such requests

This comment has been minimized.

Copy link
@timemachine3030

timemachine3030 Sep 7, 2021

Owner

Is the remote server setting the Content-Type to application/json but not including a body? If that's the case then line 61 needs updated to only parse the JSON if it exists (which is probably a valid update regardless of the remote server action).

I'm asking so that I can understand the behavior and write a test to reproduce. If you can an example of a failure that would be helpful.

setContentTypeIfUnset(headers, 'application/json;charset=utf-8');
return JSON.stringify(data);
}
return data;
}],

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;
}],

/**
Expand Down
Loading

0 comments on commit 5ad6994

Please sign in to comment.