Merge pull request #178 from desmondmorris/http-errors

Allow 2XX responses to be ok and test it a lot

 - Bonus! Max complexity and max statements have fallen
 - exercises the REST api capabilities

.eslintrc.js

@@ -11,11 +11,11 @@ module.exports = {
     'indent': [2, 2, {SwitchCase: 1 }],
     'brace-style': [2, "stroustrup", {'allowSingleLine': true} ],
     'comma-dangle': [0],
-    'complexity': [2, 32],
+    'complexity': [2, 19],
     'curly': 2,
     'eqeqeq': 2,
     'max-depth': 2,
-    'max-statements': [2, 41],
+    'max-statements': [2, 16],
     'new-cap': 2,
     'no-caller': 2,
     'no-cond-assign': 2,

lib/twitter.js

@@ -133,36 +133,42 @@ Twitter.prototype.__request = function(method, path, params, callback) {
   }
 
   this.request(options, function(error, response, data) {
+    // request error
     if (error) {
-      callback(error, data, response);
+      return callback(error, data, response);
     }
-    else {
-      try {
-        data = JSON.parse(data);
-      }
-      catch(parseError) {
-        return callback(
-          new Error('Status Code: ' + response.statusCode),
-          data,
-          response
-        );
 
-      }
-      if (typeof data.errors !== 'undefined') {
-        callback(data.errors, data, response);
-      }
-      else if(response.statusCode !== 200) {
-        callback(
-          new Error('Status Code: ' + response.statusCode),
-          data,
-          response
-        );
-      }
-      else {
-        callback(null, data, response);
-      }
+    // JSON parse error
+    try {
+      data = JSON.parse(data);
+    }
+    catch(parseError) {
+      return callback(
+        new Error('JSON parseError with HTTP Status: ' + response.statusCode + ' ' + response.statusMessage),
+        data,
+        response
+      );
     }
+
+    // response object errors
+    // This should return an error object not an array of errors
+    if (data.errors !== undefined) {
+      return callback(data.errors, data, response);
+    }
+
+    // status code errors
+    if(response.statusCode < 200 || response.statusCode > 299) {
+      return callback(
+        new Error('HTTP Error: ' + response.statusCode + ' ' + response.statusMessage),
+        data,
+        response
+      );
+    }
+
+    // no errors
+    callback(null, data, response);
   });
+
 };
 
 /**

test/twitter.js

@@ -120,8 +120,8 @@ describe('Twitter', function() {
     });
   });
 
-  describe('Prototypes', function() {
-    describe('prototype.__buildEndpoint();', function() {
+  describe('Methods', function() {
+    describe('__buildEndpoint()', function() {
       var client;
 
       before(function() {
@@ -178,16 +178,63 @@ describe('Twitter', function() {
       });
     });
 
-    describe('prototype.__request();', function() {
+    describe('__request()', function(){
+      before(function(){
+        this.nock = nock('https://api.twitter.com');
+        this.twitter = new Twitter();
+      });
+
+      it('accepts a 2xx response', function(done) {
+        var jsonResponse = {favorites: [] };
+        this.nock.get(/.*/).reply(201, jsonResponse);
+        this.twitter.__request('get', '/tweet', function(error, data, response){
+          assert.equal(error, null);
+          assert.deepEqual(data, jsonResponse);
+          assert.notEqual(response, null);
+          done();
+        });
+      });
+
+      it('errors on non 4xx or 5xx responses', function(done){
+        var jsonResponse = {errors: ['nope']};
+        this.nock.get(/.*/).reply(403, jsonResponse);
+        this.twitter.__request('get', '/tweet', function(error, data, response){
+          assert.deepEqual(error, ['nope']);
+          assert.deepEqual(data, jsonResponse);
+          assert.notEqual(response, null);
+          done();
+        });
+      });
+
+      it('errors on bad json', function(done) {
+        this.nock.get(/.*/).reply(500, 'fail whale');
+        this.twitter.__request('get', '/tweet', function(error, data, response){
+          assert(error instanceof Error);
+          assert.equal(data, 'fail whale');
+          assert.notEqual(response, null);
+          done();
+        });
+      });
+
+      it('errors on a request or network error', function(done) {
+        this.nock.get(/.*/).replyWithError('something bad happened');
+        this.twitter.__request('get', '/tweet', function(error, data, response){
+          assert(error instanceof Error);
+          assert.equal(error.message, 'something bad happened');
+          assert.equal(data, undefined);
+          assert.equal(response, undefined);
+          done();
+        });
+      });
     });
 
-    describe('prototype.__get();', function() {
+    describe('get();', function() {
     });
 
-    describe('prototype.__post();', function() {
+    describe('post()', function() {
     });
 
-    describe('prototype.__stream();', function() {
+    describe('stream()', function() {
     });
   });