Handle 429 and 5xx HTTP errors

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);
+      }
     }
   }
 };

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",

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');
+        });
       });
     });
   });