Fix cli crash when oc dev reads a broken package.json

Gruntfile.js

@@ -57,7 +57,7 @@ module.exports = function(grunt) {
     fs.writeFileSync(path.join(__dirname, 'client/oc-client.min.js'), compressedClientLibrary);
 
     var Local = require('./cli/domain/local'),
-        local = new Local();
+        local = new Local({ logger: { log: grunt.log.writeln }});
 
     local.package(path.join(__dirname, 'components/oc-client'), function(err, res){
       grunt.log[!!err ? 'error' : 'ok'](!!err ? err : 'Client has been built and packaged');

cli/domain/get-components-by-dir.js

@@ -0,0 +1,50 @@
+'use strict';
+
+var fs = require('fs-extra');
+var path = require('path');
+var _ = require('underscore');
+
+module.exports = function(dependencies){
+  var logger = dependencies.logger;
+  return function(componentsDir, callback){
+
+    try {
+      var components = fs.readdirSync(componentsDir).filter(function(file){
+
+        var filePath = path.resolve(componentsDir, file),
+            isDir = fs.lstatSync(filePath).isDirectory(),
+            packagePath = path.join(filePath, 'package.json');
+
+        if(!isDir || !fs.existsSync(packagePath)){
+          return false;
+        }
+
+        var content;
+
+        try {
+          content = fs.readJsonSync(packagePath);
+        }
+        catch(err)
+        {
+          logger.log(('error reading ' + packagePath + ' ' + err.toString()).red);
+          return false;
+        }
+
+        if(!content.oc || !!content.oc.packaged){
+          return false;
+        }
+
+        return true;
+      });
+
+      var fullPathComponents = _.map(components, function(component){
+        return path.resolve(componentsDir, component);
+      });
+
+      callback(null, fullPathComponents);
+
+    } catch(err){
+      return callback(err);
+    }
+  };
+};

cli/domain/local.js

@@ -3,6 +3,7 @@
 var async = require('async');
 var format = require('stringformat');
 var fs = require('fs-extra');
+var colors = require('colors');
 var path = require('path');
 var Targz = require('tar.gz');
 var _ = require('underscore');
@@ -11,12 +12,13 @@ var getUnixUtcTimestamp = require('../../utils/get-unix-utc-timestamp');
 var packageServerScript = require('./package-server-script');
 var packageStaticFiles = require('./package-static-files');
 var packageTemplate = require('./package-template');
+var getComponentsByDir = require('./get-components-by-dir');
 var request = require('../../utils/request');
 var settings = require('../../resources/settings');
 var validator = require('../../registry/domain/validators');
 
-module.exports = function(){
-
+module.exports = function(dependencies){
+  var logger = dependencies.logger;
   var targz = new Targz();
 
   return _.extend(this, {
@@ -26,38 +28,7 @@ module.exports = function(){
     compress: function(input, output, callback){
       return targz.compress(input, output, callback);
     },
-    getComponentsByDir: function(componentsDir, callback){
-
-      try {
-        var components = fs.readdirSync(componentsDir).filter(function(file){
-
-          var filePath = path.resolve(componentsDir, file),
-              isDir = fs.lstatSync(filePath).isDirectory(),
-              packagePath = path.join(filePath, 'package.json');
-
-          if(!isDir || !fs.existsSync(packagePath)){
-            return false;
-          }
-
-          var content = fs.readJsonSync(packagePath);
-
-          if(!content.oc || !!content.oc.packaged){
-            return false;
-          }
-
-          return true;
-        });
-
-        var fullPathComponents = _.map(components, function(component){
-          return path.resolve(componentsDir, component);
-        });
-
-        callback(null, fullPathComponents);
-
-      } catch(err){
-        return callback(err);
-      }
-    },
+    getComponentsByDir: getComponentsByDir(dependencies),
     getLocalNpmModules: function(componentsDir){
 
       var nodeFolder = path.join(componentsDir, 'node_modules');

cli/index.js

@@ -9,14 +9,16 @@ var Local = require('./domain/local');
 var Registry = require('./domain/registry');
 var strings = require('../resources');
 
+var logger = {
+  log: console.log,
+  logNoNewLine: function(msg){
+    return process.stdout.write(msg.toString());
+  }
+};
+
 var dependencies = {
-  local: new Local(),
-  logger: {
-    log: console.log,
-    logNoNewLine: function(msg){
-      return process.stdout.write(msg.toString());
-    }
-  },
+  local: new Local({ logger: logger }),
+  logger: logger,
   registry: new Registry()
 };
 

test/unit/cli-domain-local-get-components-by-dir.js

@@ -0,0 +1,126 @@
+'use strict';
+
+var expect = require('chai').expect;
+var injectr = require('injectr');
+var path = require('path');
+var sinon = require('sinon');
+var _ = require('underscore');
+
+var initialise = function(){
+
+  var loggerMock = {
+    log: sinon.stub()
+  };
+
+  var fsMock = {
+    existsSync: sinon.stub(),
+    lstatSync: sinon.stub(),
+    mkdirSync: sinon.spy(),
+    readdirSync: sinon.stub(),
+    readFileSync: sinon.stub(),
+    readJson: sinon.stub(),
+    readJsonSync: sinon.stub(),
+    writeFile: sinon.stub().yields(null, 'ok'),
+    writeJson: sinon.stub().yields(null, 'ok')
+  };
+
+  var pathMock = {
+    extname: path.extname,
+    join: path.join,
+    resolve: function(){
+      return _.toArray(arguments).join('/');
+    }
+  };
+
+  var GetComponentsByDir = injectr('../../cli/domain/get-components-by-dir.js', {
+    'fs-extra': fsMock,
+    path: pathMock
+  }, { __dirname: '' });
+
+  var local = new GetComponentsByDir({ logger: loggerMock });
+
+  return { local: local, fs: fsMock, logger: loggerMock };
+};
+
+var executeComponentsListingByDir = function(local, callback){
+  return local('.', callback);
+};
+
+describe('cli : domain : get-components-by-dir', function(){
+
+  describe('when getting components from dir', function(){
+
+    var error, result;
+    beforeEach(function(done){
+
+      var data = initialise();
+
+      data.fs.readdirSync.onCall(0).returns([
+        'a-component',
+        'a-not-component-dir',
+        'a-file.json',
+        '_package'
+      ]);
+
+      data.fs.lstatSync.onCall(0).returns({ isDirectory: function(){ return true; }});
+      data.fs.existsSync.onCall(0).returns(true);
+      data.fs.readJsonSync.onCall(0).returns({ oc: {}});
+
+      data.fs.lstatSync.onCall(1).returns({ isDirectory: function(){ return true; }});
+      data.fs.existsSync.onCall(1).returns(false);
+
+      data.fs.lstatSync.onCall(2).returns({ isDirectory: function(){ return false; }});
+
+      data.fs.lstatSync.onCall(3).returns({ isDirectory: function(){ return true; }});
+      data.fs.existsSync.onCall(2).returns(true);
+      data.fs.readJsonSync.onCall(1).returns({ oc: { packaged: true }});
+
+      executeComponentsListingByDir(data.local, function(err, res){
+        error = err;
+        result = res;
+        done();
+      });
+    });
+
+    it('should add version to package.json file', function(){
+      expect(result).to.eql(['./a-component']);
+    });
+  });
+
+  describe('when reading a broken package.json', function(){
+
+    var error, result, logger;
+    beforeEach(function(done){
+
+      var data = initialise();
+
+      data.fs.readdirSync.onCall(0).returns([
+        'a-broken-component',
+        'another-component'
+      ]);
+
+      data.fs.lstatSync.onCall(0).returns({ isDirectory: function(){ return true; }});
+      data.fs.existsSync.onCall(0).returns(true);
+      data.fs.readJsonSync.onCall(0).throws(new Error('syntax error: fubar'));
+
+      data.fs.lstatSync.onCall(1).returns({ isDirectory: function(){ return true; }});
+      data.fs.existsSync.onCall(1).returns(true);
+      data.fs.readJsonSync.onCall(1).returns({ oc: { }});
+
+      executeComponentsListingByDir(data.local, function(err, res){
+        error = err;
+        result = res;
+        logger = data.logger;
+        done();
+      });
+    });
+
+    it('should handle the error and continue loading other components', function(){
+      expect(result).to.eql(['./another-component']);
+    });
+
+    it('should log the error', function(){
+      expect(logger.log.called).to.eql(true);
+    });
+  });
+});

test/unit/cli-domain-local.js

@@ -20,6 +20,14 @@ var initialise = function(){
     writeJson: sinon.stub().yields(null, 'ok')
   };
 
+  var pathMock = {
+    extname: path.extname,
+    join: path.join,
+    resolve: function(){
+      return _.toArray(arguments).join('/');
+    }
+  };
+
   var Local = injectr('../../cli/domain/local.js', {
     'fs-extra': fsMock,
     'uglify-js': {
@@ -29,18 +37,12 @@ var initialise = function(){
         };
       }
     },
-    path: {
-      extname: path.extname,
-      join: path.join,
-      resolve: function(){
-        return _.toArray(arguments).join('/');
-      }
-    },
+    path: pathMock,
     './package-static-files': sinon.stub().yields(null, 'ok'),
     './package-template': sinon.stub().yields(null, { type: 'jade', src: 'template.js', hashKey: '123456'})
   }, { __dirname: '' });
 
-  var local = new Local();
+  var local = new Local({ logger: { log: console.log } });
 
   return { local: local, fs: fsMock };
 };
@@ -57,10 +59,6 @@ var executeMocking = function(local, type, name, value, cb){
   }, cb);
 };
 
-var executeComponentsListingByDir = function(local, callback){
-  return local.getComponentsByDir('.', callback);
-};
-
 describe('cli : domain : local', function(){
 
   describe('when packaging', function(){
@@ -106,45 +104,6 @@ describe('cli : domain : local', function(){
     });
   });
 
-  describe('when getting components from dir', function(){
-
-    var error, result;
-    beforeEach(function(done){
-
-      var data = initialise();
-
-      data.fs.readdirSync.onCall(0).returns([
-        'a-component',
-        'a-not-component-dir',
-        'a-file.json',
-        '_package'
-      ]);
-
-      data.fs.lstatSync.onCall(0).returns({ isDirectory: function(){ return true; }});
-      data.fs.existsSync.onCall(0).returns(true);
-      data.fs.readJsonSync.onCall(0).returns({ oc: {}});
-
-      data.fs.lstatSync.onCall(1).returns({ isDirectory: function(){ return true; }});
-      data.fs.existsSync.onCall(1).returns(false);
-
-      data.fs.lstatSync.onCall(2).returns({ isDirectory: function(){ return false; }});
-
-      data.fs.lstatSync.onCall(3).returns({ isDirectory: function(){ return true; }});
-      data.fs.existsSync.onCall(2).returns(true);
-      data.fs.readJsonSync.onCall(1).returns({ oc: { packaged: true }});
-
-      executeComponentsListingByDir(data.local, function(err, res){
-        error = err;
-        result = res;
-        done();
-      });
-    });
-
-    it('should add version to package.json file', function(){
-      expect(result).to.eql(['./a-component']);
-    });
-  });
-
   describe('when mocking a static plugin', function(){
 
     var data;

test/unit/cli-facade-dev.js

@@ -9,7 +9,7 @@ describe('cli : facade : dev', function(){
   var logSpy = {},
       DevFacade = require('../../cli/facade/dev'),
       Local = require('../../cli/domain/local'),
-      local = new Local(),
+      local = new Local({ logger: { log: function(){} } }),
       npm = require('npm'),
       devFacade = new DevFacade({ local: local, logger: logSpy });