From 91dae3c4ad4f75c16bb077fec0a3914d63730659 Mon Sep 17 00:00:00 2001 From: Nick Uraltsev Date: Mon, 13 Jun 2016 13:56:08 -0700 Subject: [PATCH 1/2] Improve error handling --- README.md | 30 ++++++++++++++-------------- examples/get/index.html | 6 +++--- examples/post/index.html | 4 ++-- examples/upload/index.html | 4 ++-- lib/adapters/http.js | 18 +++++++++-------- lib/adapters/xhr.js | 8 +++----- lib/core/createError.js | 16 +++++++++++++++ lib/core/enhanceError.js | 17 ++++++++++++++++ lib/core/settle.js | 6 +++++- test/specs/core/createError.spec.js | 11 ++++++++++ test/specs/core/enhanceError.spec.js | 15 ++++++++++++++ test/specs/core/settle.spec.js | 7 ++++++- test/specs/requests.spec.js | 11 ++++++++++ test/unit/adapters/http.js | 8 ++++---- 14 files changed, 120 insertions(+), 41 deletions(-) create mode 100644 lib/core/createError.js create mode 100644 lib/core/enhanceError.js create mode 100644 test/specs/core/createError.spec.js create mode 100644 test/specs/core/enhanceError.spec.js diff --git a/README.md b/README.md index 2d39982..b22cc48 100644 --- a/README.md +++ b/README.md @@ -56,8 +56,8 @@ axios.get('/user?ID=12345') .then(function (response) { console.log(response); }) - .catch(function (response) { - console.log(response); + .catch(function (error) { + console.log(error); }); // Optionally the request above could also be done as @@ -69,8 +69,8 @@ axios.get('/user', { .then(function (response) { console.log(response); }) - .catch(function (response) { - console.log(response); + .catch(function (error) { + console.log(error); }); ``` @@ -84,8 +84,8 @@ axios.post('/user', { .then(function (response) { console.log(response); }) - .catch(function (response) { - console.log(response); + .catch(function (error) { + console.log(error); }); ``` @@ -414,18 +414,18 @@ instance.interceptors.request.use(function () {/*...*/}); ```js axios.get('/user/12345') - .catch(function (response) { - if (response instanceof Error) { - // Something happened in setting up the request that triggered an Error - console.log('Error', response.message); - } else { + .catch(function (error) { + if (error.response) { // The request was made, but the server responded with a status code // that falls out of the range of 2xx - console.log(response.data); - console.log(response.status); - console.log(response.headers); - console.log(response.config); + console.log(error.response.data); + console.log(error.response.status); + console.log(error.response.headers); + } else { + // Something happened in setting up the request that triggered an Error + console.log('Error', error.message); } + console.log(error.config); }); ``` diff --git a/examples/get/index.html b/examples/get/index.html index b0ab807..dc8e51b 100644 --- a/examples/get/index.html +++ b/examples/get/index.html @@ -20,13 +20,13 @@ '' + person.name + '' + '
Github: ' + person.github + '
' + '
Twitter: ' + person.twitter + '
' + - '' + + '' + '
' ); }).join(''); }) - .catch(function(data) { - document.getElementById('people').innerHTML = '
  • ' + data + '
    • '; + .catch(function (err) { + document.getElementById('people').innerHTML = '
  • ' + err.message + '
    • '; }); diff --git a/examples/post/index.html b/examples/post/index.html index 5d8e670..11f9ce3 100644 --- a/examples/post/index.html +++ b/examples/post/index.html @@ -29,9 +29,9 @@ output.className = 'container'; output.innerHTML = res.data; }) - .catch(function (res) { + .catch(function (err) { output.className = 'container text-danger'; - output.innerHTML = res.data; + output.innerHTML = err.message; }); }; })(); diff --git a/examples/upload/index.html b/examples/upload/index.html index 57125d5..9e72c1a 100644 --- a/examples/upload/index.html +++ b/examples/upload/index.html @@ -37,9 +37,9 @@ output.className = 'container'; output.innerHTML = res.data; }) - .catch(function (res) { + .catch(function (err) { output.className = 'container text-danger'; - output.innerHTML = res.data; + output.innerHTML = err.message; }); }; })(); diff --git a/lib/adapters/http.js b/lib/adapters/http.js index 464e19f..265b008 100644 --- a/lib/adapters/http.js +++ b/lib/adapters/http.js @@ -12,6 +12,8 @@ var url = require('url'); var zlib = require('zlib'); var pkg = require('./../../package.json'); var Buffer = require('buffer').Buffer; +var createError = require('../core/createError'); +var enhanceError = require('../core/enhanceError'); /*eslint consistent-return:0*/ module.exports = function httpAdapter(resolve, reject, config) { @@ -33,7 +35,10 @@ module.exports = function httpAdapter(resolve, reject, config) { } else if (utils.isString(data)) { data = new Buffer(data, 'utf-8'); } else { - return reject(new Error('Data after transformation must be a string, an ArrayBuffer, or a Stream')); + return reject(createError( + 'Data after transformation must be a string, an ArrayBuffer, or a Stream', + config + )); } // Add Content-Length header if data exists @@ -122,13 +127,13 @@ module.exports = function httpAdapter(resolve, reject, config) { // make sure the content length is not over the maxContentLength if specified if (config.maxContentLength > -1 && Buffer.concat(responseBuffer).length > config.maxContentLength) { - reject(new Error('maxContentLength size of ' + config.maxContentLength + ' exceeded')); + reject(createError('maxContentLength size of ' + config.maxContentLength + ' exceeded', config)); } }); stream.on('error', function handleStreamError(err) { if (aborted) return; - reject(err); + reject(enhanceError(err, config)); }); stream.on('end', function handleStreamEnd() { @@ -145,17 +150,14 @@ module.exports = function httpAdapter(resolve, reject, config) { // Handle errors req.on('error', function handleRequestError(err) { if (aborted) return; - reject(err); + reject(enhanceError(err, config)); }); // Handle request timeout if (config.timeout && !timer) { timer = setTimeout(function handleRequestTimeout() { - var err = new Error('timeout of ' + config.timeout + 'ms exceeded'); - err.timeout = config.timeout; - err.code = 'ECONNABORTED'; req.abort(); - reject(err); + reject(createError('timeout of ' + config.timeout + 'ms exceeded', config, 'ECONNABORTED')); aborted = true; }, config.timeout); } diff --git a/lib/adapters/xhr.js b/lib/adapters/xhr.js index 5d08dee..2cfc31a 100644 --- a/lib/adapters/xhr.js +++ b/lib/adapters/xhr.js @@ -6,6 +6,7 @@ var transformData = require('./../core/transformData'); var buildURL = require('./../helpers/buildURL'); var parseHeaders = require('./../helpers/parseHeaders'); var isURLSameOrigin = require('./../helpers/isURLSameOrigin'); +var createError = require('../core/createError'); var btoa = (typeof window !== 'undefined' && window.btoa) || require('./../helpers/btoa'); module.exports = function xhrAdapter(resolve, reject, config) { @@ -82,7 +83,7 @@ module.exports = function xhrAdapter(resolve, reject, config) { request.onerror = function handleError() { // Real errors are hidden from us by the browser // onerror should only fire if it's a network error - reject(new Error('Network Error')); + reject(createError('Network Error', config)); // Clean up request request = null; @@ -90,10 +91,7 @@ module.exports = function xhrAdapter(resolve, reject, config) { // Handle timeout request.ontimeout = function handleTimeout() { - var err = new Error('timeout of ' + config.timeout + 'ms exceeded'); - err.timeout = config.timeout; - err.code = 'ECONNABORTED'; - reject(err); + reject(createError('timeout of ' + config.timeout + 'ms exceeded', config, 'ECONNABORTED')); // Clean up request request = null; diff --git a/lib/core/createError.js b/lib/core/createError.js new file mode 100644 index 0000000..f27439d --- /dev/null +++ b/lib/core/createError.js @@ -0,0 +1,16 @@ +'use strict'; + +var enhanceError = require('./enhanceError'); + +/** + * Create an Error with the specified message, config, and optional error code. + * + * @param {string} message The error message. + * @param {Object} config The config object. + * @param {string} [code] The error code (for example, 'ECONNABORTED'). + * @returns {Error} The created error. + */ +module.exports = function createError(message, config, code) { + var error = new Error(message); + return enhanceError(error, config, code); +}; diff --git a/lib/core/enhanceError.js b/lib/core/enhanceError.js new file mode 100644 index 0000000..baef48a --- /dev/null +++ b/lib/core/enhanceError.js @@ -0,0 +1,17 @@ +'use strict'; + +/** + * Update an Error with the specified config and optional error code. + * + * @param {Error} error The error to update. + * @param {Object} config The config object. + * @param {string} [code] The error code (for example, 'ECONNABORTED'). + * @returns {Error} The error. + */ +module.exports = function enhanceError(error, config, code) { + error.config = config; + if (code) { + error.code = code; + } + return error; +}; diff --git a/lib/core/settle.js b/lib/core/settle.js index 35027f5..f2571f0 100644 --- a/lib/core/settle.js +++ b/lib/core/settle.js @@ -1,5 +1,7 @@ 'use strict'; +var createError = require('./createError'); + /** * Resolve or reject a Promise based on response status. * @@ -13,6 +15,8 @@ module.exports = function settle(resolve, reject, response) { if (!response.status || !validateStatus || validateStatus(response.status)) { resolve(response); } else { - reject(response); + var error = createError('Request failed with status code ' + response.status, response.config); + error.response = response; + reject(error); } }; diff --git a/test/specs/core/createError.spec.js b/test/specs/core/createError.spec.js new file mode 100644 index 0000000..a1e445b --- /dev/null +++ b/test/specs/core/createError.spec.js @@ -0,0 +1,11 @@ +var createError = require('../../../lib/core/createError'); + +describe('core::createError', function() { + it('should create an Error with message, config, and code', function() { + var error = createError('Boom!', { foo: 'bar' }, 'ESOMETHING'); + expect(error instanceof Error).toBe(true); + expect(error.message).toBe('Boom!'); + expect(error.config).toEqual({ foo: 'bar' }); + expect(error.code).toBe('ESOMETHING'); + }); +}); diff --git a/test/specs/core/enhanceError.spec.js b/test/specs/core/enhanceError.spec.js new file mode 100644 index 0000000..bb8ba22 --- /dev/null +++ b/test/specs/core/enhanceError.spec.js @@ -0,0 +1,15 @@ +var enhanceError = require('../../../lib/core/enhanceError'); + +describe('core::enhanceError', function() { + it('should add config and code to error', function() { + var error = new Error('Boom!'); + enhanceError(error, { foo: 'bar' }, 'ESOMETHING'); + expect(error.config).toEqual({ foo: 'bar' }); + expect(error.code).toBe('ESOMETHING'); + }); + + it('should return error', function() { + var error = new Error('Boom!'); + expect(enhanceError(error, { foo: 'bar' }, 'ESOMETHING')).toBe(error); + }); +}); diff --git a/test/specs/core/settle.spec.js b/test/specs/core/settle.spec.js index 14e507f..cc5c15e 100644 --- a/test/specs/core/settle.spec.js +++ b/test/specs/core/settle.spec.js @@ -58,7 +58,12 @@ describe('core::settle', function() { }; settle(resolve, reject, response); expect(resolve).not.toHaveBeenCalled(); - expect(reject).toHaveBeenCalledWith(response); + expect(reject).toHaveBeenCalled(); + var reason = reject.calls.first().args[0]; + expect(reason instanceof Error).toBe(true); + expect(reason.message).toBe('Request failed with status code 500'); + expect(reason.config).toBe(response.config); + expect(reason.response).toBe(response); }); it('should pass status to validateStatus', function() { diff --git a/test/specs/requests.spec.js b/test/specs/requests.spec.js index 47be0b0..0a81d16 100644 --- a/test/specs/requests.spec.js +++ b/test/specs/requests.spec.js @@ -46,6 +46,10 @@ describe('requests', function () { var finish = function () { expect(resolveSpy).not.toHaveBeenCalled(); expect(rejectSpy).toHaveBeenCalled(); + var reason = rejectSpy.calls.first().args[0]; + expect(reason instanceof Error).toBe(true); + expect(reason.config.method).toBe('get'); + expect(reason.config.url).toBe('http://thisisnotaserver'); done(); }; @@ -68,6 +72,13 @@ describe('requests', function () { .then(function () { expect(resolveSpy).not.toHaveBeenCalled(); expect(rejectSpy).toHaveBeenCalled(); + var reason = rejectSpy.calls.first().args[0]; + expect(reason instanceof Error).toBe(true); + expect(reason.message).toBe('Request failed with status code 500'); + expect(reason.config.method).toBe('get'); + expect(reason.config.url).toBe('/foo'); + expect(reason.response.status).toBe(500); + done(); }); diff --git a/test/unit/adapters/http.js b/test/unit/adapters/http.js index f1c1abe..4ef91b7 100644 --- a/test/unit/adapters/http.js +++ b/test/unit/adapters/http.js @@ -25,8 +25,8 @@ module.exports = { timeout: 250 }).then(function (res) { success = true; - }).catch(function (res) { - error = res; + }).catch(function (err) { + error = err; failure = true; }); @@ -189,8 +189,8 @@ module.exports = { maxContentLength: 2000 }).then(function (res) { success = true; - }).catch(function (res) { - error = res; + }).catch(function (err) { + error = err; failure = true; }); From 6f2186d863be5c886e6e78d4c96cba8c9f232541 Mon Sep 17 00:00:00 2001 From: Nick Uraltsev Date: Mon, 13 Jun 2016 18:58:55 -0700 Subject: [PATCH 2/2] Modify createError and enhanceError functions to accept response as parameter --- lib/core/createError.js | 9 +++++---- lib/core/enhanceError.js | 8 +++++--- lib/core/settle.js | 9 ++++++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/core/createError.js b/lib/core/createError.js index f27439d..aa9abfd 100644 --- a/lib/core/createError.js +++ b/lib/core/createError.js @@ -3,14 +3,15 @@ var enhanceError = require('./enhanceError'); /** - * Create an Error with the specified message, config, and optional error code. + * Create an Error with the specified message, config, error code, and response. * * @param {string} message The error message. - * @param {Object} config The config object. + * @param {Object} config The config. * @param {string} [code] The error code (for example, 'ECONNABORTED'). + @ @param {Object} [response] The response. * @returns {Error} The created error. */ -module.exports = function createError(message, config, code) { +module.exports = function createError(message, config, code, response) { var error = new Error(message); - return enhanceError(error, config, code); + return enhanceError(error, config, code, response); }; diff --git a/lib/core/enhanceError.js b/lib/core/enhanceError.js index baef48a..adf836f 100644 --- a/lib/core/enhanceError.js +++ b/lib/core/enhanceError.js @@ -1,17 +1,19 @@ 'use strict'; /** - * Update an Error with the specified config and optional error code. + * Update an Error with the specified config, error code, and response. * * @param {Error} error The error to update. - * @param {Object} config The config object. + * @param {Object} config The config. * @param {string} [code] The error code (for example, 'ECONNABORTED'). + @ @param {Object} [response] The response. * @returns {Error} The error. */ -module.exports = function enhanceError(error, config, code) { +module.exports = function enhanceError(error, config, code, response) { error.config = config; if (code) { error.code = code; } + error.response = response; return error; }; diff --git a/lib/core/settle.js b/lib/core/settle.js index f2571f0..00ed636 100644 --- a/lib/core/settle.js +++ b/lib/core/settle.js @@ -15,8 +15,11 @@ module.exports = function settle(resolve, reject, response) { if (!response.status || !validateStatus || validateStatus(response.status)) { resolve(response); } else { - var error = createError('Request failed with status code ' + response.status, response.config); - error.response = response; - reject(error); + reject(createError( + 'Request failed with status code ' + response.status, + response.config, + null, + response + )); } };