diff --git a/lib/index.js b/lib/index.js index e4499c0..caa2cf8 100644 --- a/lib/index.js +++ b/lib/index.js @@ -121,7 +121,12 @@ exports.sendJsonWithTimeout = function(url, obj, headers, timeout, fn) { function done() { if (req.readyState === 4) { - return fn(null, req); + // Fail on 429 and 5xx HTTP errors + if (req.status === 429 || req.status >= 500 && req.status < 600) { + fn(new Error('HTTP Error ' + req.status + ' (' + req.statusText + ')')); + } else { + fn(null, req); + } } } }; diff --git a/package.json b/package.json index 4a4ce50..8f0bdc6 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "karma-phantomjs-launcher": "^1.0.0", "karma-sauce-launcher": "^1.0.0", "karma-spec-reporter": "0.0.26", + "lolex": "^2.7.1", "mocha": "^2.2.5", "npm-check": "^5.2.1", "phantomjs-prebuilt": "^2.1.7", diff --git a/test/index.test.js b/test/index.test.js index b00ec9d..e5c0c88 100644 --- a/test/index.test.js +++ b/test/index.test.js @@ -13,6 +13,8 @@ var tester = require('@segment/analytics.js-integration-tester'); var type = require('component-type'); var sinon = require('sinon'); var send = require('@segment/send-json'); +var Schedule = require('@segment/localstorage-retry/lib/schedule'); +var lolex = require('lolex'); // FIXME(ndhoule): clear-env's AJAX request clearing interferes with PhantomJS 2 // Detect Phantom env and use it to disable affected tests. We should use a @@ -1017,6 +1019,23 @@ describe('Segment.io', function() { var body = JSON.parse(req.requestBody); assert.equal(body.userId, '1'); }); + + it('should retry on HTTP errors', function() { + var clock = lolex.createClock(0); + var spy = sinon.spy(); + + Schedule.setClock(clock); + xhr.onCreate = spy; + + segment.enqueue('/i', { userId: '1' }); + assert(spy.calledOnce); + + var req = spy.getCall(0).args[0]; + req.respond(500, null, 'segment machine broke'); + + clock.tick(segment._lsqueue.getDelay(1)); + assert(spy.calledTwice); + }); }); describe('sendJsonWithTimeout', function() { @@ -1045,6 +1064,45 @@ describe('Segment.io', function() { Segment.sendJsonWithTimeout(url, [1, 2, 3], headers, 1, function(err) { assert(err.type === 'timeout'); done(); + }); + }); + + describe('error handling', function() { + var xhr; + var req; + + beforeEach(function() { + xhr = sinon.useFakeXMLHttpRequest(); + xhr.onCreate = function(_req) { + req = _req; + }; + }); + + afterEach(function() { + xhr.restore(); + }); + + [429, 500, 503].forEach(function(code) { + it('should throw on ' + code + ' HTTP errors', function(done) { + if (send.type !== 'xhr') return done(); + + Segment.sendJsonWithTimeout(url + '/null', [1, 2, 3], headers, 10 * 1000, function(err) { + assert(RegExp('^HTTP Error ' + code + ' (.+)$').test(err.message)); + done(); + }); + + req.respond(code, null, 'nope'); + }); + }); + + [200, 204, 300, 302, 400, 404].forEach(function(code) { + it('should not throw on ' + code + ' HTTP errors', function(done) { + if (send.type !== 'xhr') return done(); + + Segment.sendJsonWithTimeout(url + '/null', [1, 2, 3], headers, 10 * 1000, done); + + req.respond(code, null, 'ok'); + }); }); }); });