From 1b6c18863b38171f13d2fb9f648478d797ca4a75 Mon Sep 17 00:00:00 2001 From: Gabriel Andretta Date: Tue, 1 Sep 2015 15:15:52 -0300 Subject: [PATCH 1/3] Change XMLHttpRequest error handling - Listen for 'load' event instead of 'readystatechange' because the later can be triggered along with the 'error' event. - Check the response status code, and invoke the callback with an error argument when it is not in the 20x range. --- index.js | 6 ++++-- test/test.js | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/index.js b/index.js index 93768a8..a325071 100644 --- a/index.js +++ b/index.js @@ -62,13 +62,15 @@ function json(url, obj, headers, fn){ var req = new XMLHttpRequest; req.onerror = fn; - req.onreadystatechange = done; + req.onload = done; req.open('POST', url, true); for (var k in headers) req.setRequestHeader(k, headers[k]); req.send(JSON.stringify(obj)); function done(){ - if (4 == req.readyState) return fn(null, req); + // sometimes IE returns 1223 when it should be 204 + // see http://stackoverflow.com/questions/10046972/msie-returns-status-code-of-1223-for-ajax-request + /^(20\d|1223)$/.test(req.status) ? fn(null, req) : fn(req); } } diff --git a/test/test.js b/test/test.js index 6c7fecc..c6f3888 100644 --- a/test/test.js +++ b/test/test.js @@ -20,6 +20,28 @@ describe('send-json', function(){ done(); }); }) + + it('should invoke the callback with an error when connection is refused', function(done){ + if ('xhr' != send.type) return done(); + var url = protocol() + '//localhost:4567/data'; + var headers = { 'Content-Type': 'application/json' }; + send.json(url, [1, 2, 3], headers, function(err, req){ + assert(err); + assert(err.status === undefined); + done(); + }); + }) + + it('should invoke the callback with an error when the request does not succeed', function(done){ + if ('xhr' != send.type) return done(); + var url = protocol() + '//localhost:3001/not-found'; + var headers = { 'Content-Type': 'application/json' }; + send.json(url, [1, 2, 3], headers, function(err, req){ + assert(err); + assert(err.status === 404); + done(); + }); + }) }) describe('#base64', function(){ From 13ccd0de0fbd159b7b38073a096f61cf68df4316 Mon Sep 17 00:00:00 2001 From: Gabriel Andretta Date: Wed, 2 Sep 2015 11:04:31 -0300 Subject: [PATCH 2/3] Bubble up actual Error instances --- index.js | 16 ++++++++++++++-- test/test.js | 5 +++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index a325071..274db23 100644 --- a/index.js +++ b/index.js @@ -61,7 +61,7 @@ function json(url, obj, headers, fn){ if (3 == arguments.length) fn = headers, headers = {}; var req = new XMLHttpRequest; - req.onerror = fn; + req.onerror = error; req.onload = done; req.open('POST', url, true); for (var k in headers) req.setRequestHeader(k, headers[k]); @@ -70,7 +70,19 @@ function json(url, obj, headers, fn){ function done(){ // sometimes IE returns 1223 when it should be 204 // see http://stackoverflow.com/questions/10046972/msie-returns-status-code-of-1223-for-ajax-request - /^(20\d|1223)$/.test(req.status) ? fn(null, req) : fn(req); + if (/^(20\d|1223)$/.test(req.status)) { + fn(null, req) + } else { + var err = new Error("Request wasn't successful") + err.req = req; + fn(err); + } + } + + function error(evt){ + var err = new Error("A network error ocurred"); + err.evt = evt; + fn(err); } } diff --git a/test/test.js b/test/test.js index c6f3888..0a847aa 100644 --- a/test/test.js +++ b/test/test.js @@ -27,7 +27,7 @@ describe('send-json', function(){ var headers = { 'Content-Type': 'application/json' }; send.json(url, [1, 2, 3], headers, function(err, req){ assert(err); - assert(err.status === undefined); + assert(typeof err.evt === "object"); done(); }); }) @@ -38,7 +38,8 @@ describe('send-json', function(){ var headers = { 'Content-Type': 'application/json' }; send.json(url, [1, 2, 3], headers, function(err, req){ assert(err); - assert(err.status === 404); + assert(typeof err.req === "object"); + assert(err.req.status === 404); done(); }); }) From b24664241d3a7e7b1831875ae26460fa22681b94 Mon Sep 17 00:00:00 2001 From: Gabriel Andretta Date: Wed, 2 Sep 2015 12:23:20 -0300 Subject: [PATCH 3/3] Update callback error arguments - Rename error.evt to error.event for the network error case. - Pass the request as the second argument instead of as an event property for the unsuccessful request case. --- index.js | 11 ++++------- test/test.js | 6 +++--- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/index.js b/index.js index 274db23..32028b7 100644 --- a/index.js +++ b/index.js @@ -71,17 +71,14 @@ function json(url, obj, headers, fn){ // sometimes IE returns 1223 when it should be 204 // see http://stackoverflow.com/questions/10046972/msie-returns-status-code-of-1223-for-ajax-request if (/^(20\d|1223)$/.test(req.status)) { - fn(null, req) - } else { - var err = new Error("Request wasn't successful") - err.req = req; - fn(err); + return fn(null, req) } + fn(new Error("Request wasn't successful"), req); } - function error(evt){ + function error(event){ var err = new Error("A network error ocurred"); - err.evt = evt; + err.event = event; fn(err); } } diff --git a/test/test.js b/test/test.js index 0a847aa..7d10f3e 100644 --- a/test/test.js +++ b/test/test.js @@ -27,7 +27,7 @@ describe('send-json', function(){ var headers = { 'Content-Type': 'application/json' }; send.json(url, [1, 2, 3], headers, function(err, req){ assert(err); - assert(typeof err.evt === "object"); + assert(typeof err.event === "object"); done(); }); }) @@ -38,8 +38,8 @@ describe('send-json', function(){ var headers = { 'Content-Type': 'application/json' }; send.json(url, [1, 2, 3], headers, function(err, req){ assert(err); - assert(typeof err.req === "object"); - assert(err.req.status === 404); + assert(typeof req === "object"); + assert(req.status === 404); done(); }); })