Node.JS Twitter Count Module

Question

I would like someone review this code and tell if it can be done better, or if the code is elegant and simple enough for the task at hand.

I used code climate and I got 4/4 and my test coverage is 96%, yet I would like a professional opinion about it.

'use strict';
var PromisePolyfill = require('promise');
var http = require('http');
var twitterCountURL = 'http://urls.api.twitter.com/1/urls/count.json?url=';
/**
 * Gets the numbers of tweets of a given url.
 * @param {string} validUrl - The url to query
 * @returns {Number} Number of tweets
*/
exports.getCount = function(validUrl) {
 return new PromisePolyfill(function(resolve, reject) {
 if (validUrl) {
 http.get(twitterCountURL + validUrl, function(res) {
 var body = '';
 res.on('data', function(chunk) {
 body += chunk;
 });
 res.on('end', function() {
 var twResponse = JSON.parse(body);
 if (!twResponse.count) {
 twResponse.count = 0;
 }
 // console.log(twResponse.count);
 resolve(twResponse.count);
 });
 }).on('error', function(e) {
 reject(e.message);
 });
 }else {
 resolve(0);
 }
 });
};

And its respective test file:

'use strict';
var chai = require('chai');
var chaiAsPromised = require('chai-as-promised');
var sinon = require('sinon');
var http = require('http');
var PassThrough = require('stream').PassThrough;
var twitterCount = require('../../modules/twitterCount');
var response,
 responsePT;
chai.should();
chai.use(chaiAsPromised);
describe('twitterCount Module', function() {
 beforeEach(function(done) {
 response = new PassThrough();
 responsePT = new PassThrough();
 sinon.stub(http, 'get');
 done();
 });
 afterEach(function(done) {
 http.get.restore();
 done();
 });
 it('should return valid count when a valid url is passed', function(done) {
 var expected = {'count': 234453234, 'url': 'http:\/\/www.google.com\/'};
 response.write(JSON.stringify(expected));
 response.end();
 http.get.callsArgWith(1, response)
 .returns(responsePT);
 twitterCount.getCount('http://www.google.com').should.eventually.equal(234453234).notify(done);
 });
 it('should return 0 when a invalid url is passed', function(done) {
 var expected = {'count': 0, 'url': 'http:\/\/invalidurl.invalid\/'};
 response.write(JSON.stringify(expected));
 response.end();
 http.get.callsArgWith(1, response)
 .returns(responsePT);
 twitterCount.getCount('http://invalidurl.invalid').should.eventually.equal(0).notify(done);
 });
 it('should return 0 when an empty url is passed', function(done) {
 var expected = {'count': 0, 'url': ''};
 response.write(JSON.stringify(expected));
 response.end();
 http.get.callsArgWith(1, response)
 .returns(responsePT);
 twitterCount.getCount('').should.eventually.equal(0).notify(done);
 });
});

Answer 1

JSON.parse must always be wrapped in a try catch, because you cannot guarantee that the body will be in proper format. This will crash your server on failure.

You should add a test case to confirm this, just by responding with an empty string.

• Andres Osorio: Thanks @duane, I added the try / catch and added the test as well to the github repo. If there is anything else, please let me know. I appreciate your review.