Merge pull request #104 from Nexmo/develop

Bump to 1.1.2 - fixes JSON parse error

.travis.yml

@@ -1,5 +1,13 @@
 language: node_js
 node_js:
-  - 4
-  - 5
-  - 6
+- 4
+- 5
+- 6
+deploy:
+  provider: npm
+  email: [email protected]
+  api_key:
+    secure: dIwjf+unoDvAX/oId0TiXZ1/KNgzxZ3sqk/nquKVu0Ln5hEmePiuFEYUlpI7Lz3xVgwJqvlOJwdqGZjTT1xuih9FtOTjYxmAmF+KvfQxlMmHMpQAiOOQryLbEPMJg4lHoCinqOaw9I/cWt0Dad2+OHvzdJ+hBYC9Q54vYguBj1Dq2ENf47Ay0T2w+s6AFDOEvDF7vprFFJFY9xGhTgvJR2AuixaffKMi06CA3+G7WyNCFyYGfZgMdsx+E6rh2RC6ZntWOxR/+DbNROF96RQ4zyPflPk52A5yGeIal15tfegF4ZZ912escYeEfXn2DM2xe98qcnUBexYRGCvQQYI0E3rUDs+wrAI84NeylZEVnDXRnRgBpx8KxAF3vBsh2v+FeWySGEy+w9stISXNuWx7VkhaKd8GDicATILE8jRMivu+LmObslrLWbFIn50j6yNltWZjcioDHAuUavIsHYAb42dtg0STpnAZXFkxv8lrIoJehGURRRl9sjaokHZXM7BoQNL08Jfyi6DCage9JokpRalkg89tlAkYynuM5dBaqJopQrr3+gpLmxUhYBik1nCR9/TF8NxcaoeRzLlam4anerXEz5MECtymnOKH9nYdiCFCftOWSgVJ3pZYmq1SXmMEAAFEVGHs2K8sA/9gmNFfseHAoQq4vMkR0rYnrCYcCkU=
+  on:
+    tags: true
+    repo: Nexmo/nexmo-node

MAINTENANCE.md

@@ -0,0 +1,13 @@
+## A quick guide for maintainers
+
+The following is a quick cheatsheet for maintainers.
+
+* New commits are pushed to the `develop` branch or their own feature branch and merged into `develop`.
+* The `master` branch is to be kept up to date with the latest release on [NPMjs.com](https://www.npmjs.com/).
+* When rolling a new release, update the package version on the `develop` branch and merge `develop` into `master`. Then tag this new release:
+  * `git checkout master`
+  * `git pull master` to make sure you have the merged content
+  * `git tag -v v0.0.0` where `0.0.0` is your release
+  * `git push origin v0.0.0` to push your tag to GitHub
+* **Note:** when merging `develop` into `master` make sure to use `rebase and merge` in order to keep `develop` in sync with `master`.
+* When a new tag is pushed to Github the new release will automatically be pushed to NPM by Travis.

package.json

@@ -1,7 +1,7 @@
 {
   "name": "nexmo",
   "author": "nexmo",
-  "version": "1.1.1",
+  "version": "1.1.2",
   "main": "lib/Nexmo",
   "keywords": [
     "sms",
@@ -51,10 +51,12 @@
     "ngrok": "^2.2.2",
     "nodemon": "^1.10.2",
     "sinon": "^1.17.4",
-    "sinon-chai": "^2.8.0"
+    "sinon-chai": "^2.8.0",
+    "sinon-expect": "^0.3.0"
   },
   "dependencies": {
     "jsonwebtoken": "^7.1.9",
     "uuid": "^2.0.2"
-  }
+  },
+  "license": "MIT"
 }

src/HttpClient.js

@@ -59,35 +59,16 @@ class HttpClient {
 
     request.end(endpoint.body);
 
-    var responseReturn = '';
+    var responseData = '';
     request.on('response', (response) => {
         response.setEncoding('utf8');
         response.on('data', (chunk) => {
-            responseReturn += chunk;
+            responseData += chunk;
         });
         response.on('end', () => {
             this.logger.info('response ended:', response.statusCode);
             if (callback) {
-                var retJson = responseReturn;
-                var err = null;
-                if (method !== 'DELETE') {
-                  try {
-	                    retJson = JSON.parse(responseReturn);
-	                } catch (parsererr) {
-	                    // ignore parser error for now and send raw response to client
-	                    this.logger.error(parsererr);
-	                    this.logger.error('could not convert API response to JSON, above error is ignored and raw API response is returned to client');
-						          this.logger.error('Raw Error message from API ');
-						          this.logger.error(responseReturn);
-	                    err = parsererr;
-	                }
-                }
-
-                if(response.statusCode < 200 || response.statusCode > 299) {
-                  err = retJson;
-                }
-
-                callback(err, retJson);
+              this.__parseReponse(response.statusCode, responseData, method, callback)
             }
         })
         response.on('close', (e) => {
@@ -103,6 +84,35 @@ class HttpClient {
     });
 
   }
+
+  __parseReponse(status, data, method, callback) {
+    var response = null;
+    var error = null;
+
+    try {
+      if (status >= 500) {
+        error = { message: 'Server Error: '+status };
+      } else if (status >= 400 || status < 200) {
+        error = JSON.parse(data);
+      } else if (method !== 'DELETE') {
+        response = JSON.parse(data);
+      } else {
+        response = data;
+      }
+    } catch (parseError) {
+      this.logger.error(parseError);
+      this.logger.error('could not convert API response to JSON, above error is ignored and raw API response is returned to client');
+      this.logger.error('Raw Error message from API ');
+      this.logger.error(data);
+
+      error = {
+        message: "The API response could not be parsed.",
+        parseError: parseError
+      };
+    }
+
+    callback(error, response);
+  }
 }
 
 export default HttpClient;

test/HttpClient-test.js

@@ -1,9 +1,11 @@
 import sinon from 'sinon';
-import expect from 'expect.js';
+import expectjs from 'expect.js';
 
 import HttpClient from '../lib/HttpClient';
 import NullLogger from '../lib/NullLogger';
 
+var expect = require('sinon-expect').enhance(expectjs, sinon, 'was');
+
 var logger = new NullLogger();
 var fakeHttp = {
     request: function() {}
@@ -18,6 +20,8 @@ var defaultHeaders = {
     'Accept': 'application/json'
 };
 
+var client = null;
+
 describe('HttpClient Object', function() {
 
     afterEach(function() {
@@ -240,5 +244,46 @@ describe('HttpClient Object', function() {
             some: 'data'
         });
     });
+});
 
+describe('parseResponse', function() {
+
+  beforeEach(function(){
+    client = new HttpClient({
+      https: fakeHttp,
+      logger: logger
+    });
+  });
+
+  it ('should parse a 500+ status code as an error', function() {
+    var callback = sinon.spy();
+    client.__parseReponse(504, '', 'GET', callback);
+    expect(callback).was.calledWith({ message: 'Server Error: 504' }, null);
+  });
+
+  it ('should parse a 400-499 status code as a JSON error', function() {
+    var callback = sinon.spy();
+    client.__parseReponse(404, '{ "error" : "error" }', 'GET', callback);
+    expect(callback).was.calledWith({ 'error' : 'error' }, null);
+  });
+
+  it ('should parse a 200-299 status code as a JSON object', function() {
+    var callback = sinon.spy();
+    client.__parseReponse(201, '{ "data" : "data" }', 'GET', callback);
+    expect(callback).was.calledWith(null, { 'data' : 'data' });
+  });
+
+  it ('should not try and parse successful DELETE request to JSON', function() {
+    var callback = sinon.spy();
+    client.__parseReponse(201, '', 'DELETE', callback);
+    expect(callback).was.calledWith(null, '');
+  });
+
+  it ('should catch invalid json', function() {
+    var callback = sinon.spy();
+    client.__parseReponse(201, 'not_json', 'GET', callback);
+    expect(callback).was.calledWith(sinon.match({
+      message: 'The API response could not be parsed.'
+    }), null);
+  });
 });