Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON improvements: throw if JSON parsing failed; number, boolean can be passed directly as payload for encoding to JSON #2613, #61, #907 #3688

Merged
merged 14 commits into from Apr 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions README.md
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
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
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
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);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this setTimeout appears to be causing test failures for me when using sinon/nise FakeServer with FakeTimers in Jest. And possibly with nock.
replacing this line with onloadend() causes my tests to pass again.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for me. Thx for pointing it out. :)
Will this be fixed in a new release, because this bug appears since v0.21.2. Is it already planned? :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For one project we ended up putting flushPromises and await sandbox.clock.tickAsync() / sandbox.clock.tick in the appropriate places in our custom wrappers for sinon FakeServer and requests to workaround this change so that we could use the latest version of axios in the project.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the response :) ... the await clock.tickAsync(); worked for me. Thanks a lot!

};
}

// 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
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
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
@@ -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
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')) {
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