fix(cordova): ensure that each cordova command will properly catch wh… (

#1154) * fix(cordova): ensure that each cordova command will properly catch when cordova throws an error. * fix(cordova): show error message to user when iOS is only available on an OSX machine.
ionic-team · Jul 7, 2016 · 142128a · 142128a
1 parent 51469d6
commit 142128a
Show file tree

Hide file tree

Showing 14 changed files with 277 additions and 17 deletions.
diff --git a/lib/ionic/build.js b/lib/ionic/build.js
@@ -5,6 +5,7 @@ var os = require('os');
 var Q = require('q');
 var IonicAppLib = require('ionic-app-lib');
 var ConfigXml = IonicAppLib.configXml;
+var log = IonicAppLib.logging.logger;
 var cordovaUtils = require('../utils/cordova');
 
 var settings = {
@@ -37,7 +38,8 @@ function run(ionic, argv, rawCliArguments) {
   }
 
   if (runPlatform === 'ios' && os.platform() !== 'darwin') {
-    return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
+    log.error('✗ You cannot run iOS unless you are on Mac OSX.');
+    return Q();
   }
 
   var promiseList = []
@@ -59,6 +61,11 @@ function run(ionic, argv, rawCliArguments) {
   .then(function() {
     var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
     return cordovaUtils.execCordovaCommand(optionList);
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/compile.js b/lib/ionic/compile.js
@@ -3,6 +3,7 @@
 var extend = Object.assign || require('util')._extend; // eslint-disable-line no-underscore-dangle
 var IonicAppLib = require('ionic-app-lib');
 var ConfigXml = IonicAppLib.configXml;
+var log = IonicAppLib.logging.logger;
 var cordovaUtils = require('../utils/cordova');
 
 var settings = {
@@ -23,6 +24,11 @@ function run(ionic, argv, rawCliArguments) {
   }).then(function() {
     var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
     return cordovaUtils.execCordovaCommand(optionList);
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/emulate.js b/lib/ionic/emulate.js
@@ -5,6 +5,7 @@ var os = require('os');
 var Q = require('q');
 var IonicAppLib = require('ionic-app-lib');
 var ConfigXml = IonicAppLib.configXml;
+var log = IonicAppLib.logging.logger;
 var cordovaUtils = require('../utils/cordova');
 
 var cordovaRunEmulateOptions = {
@@ -54,7 +55,8 @@ function run(ionic, argv, rawCliArguments) {
   }
 
   if (runPlatform === 'ios' && os.platform() !== 'darwin') {
-    return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
+    log.error('✗ You cannot run iOS unless you are on Mac OSX.');
+    return Q();
   }
 
   var promiseList = []
@@ -79,6 +81,11 @@ function run(ionic, argv, rawCliArguments) {
   .then(function(serveOptions) {
     var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
     return cordovaUtils.execCordovaCommand(optionList, isLiveReload, serveOptions);
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/platform.js b/lib/ionic/platform.js
@@ -71,6 +71,11 @@ function run(ionic, argv, rawCliArguments) {
       log.info('Removing platform from package.json file');
       return State.removePlatform(appDirectory, argumentName);
     }
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/plugin.js b/lib/ionic/plugin.js
@@ -70,6 +70,11 @@ function run(ionic, argv, rawCliArguments) {
       log.info('Removing plugin from package.json file');
       return State.removePlugin(appDirectory, argumentName);
     }
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/prepare.js b/lib/ionic/prepare.js
@@ -3,6 +3,7 @@
 var extend = Object.assign || require('util')._extend; // eslint-disable-line no-underscore-dangle
 var IonicAppLib = require('ionic-app-lib');
 var ConfigXml = IonicAppLib.configXml;
+var log = IonicAppLib.logging.logger;
 var cordovaUtils = require('../utils/cordova');
 
 var settings = {
@@ -24,6 +25,11 @@ function run(ionic, argv, rawCliArguments) {
   .then(function() {
     var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
     return cordovaUtils.execCordovaCommand(optionList);
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/lib/ionic/run.js b/lib/ionic/run.js
@@ -5,6 +5,7 @@ var os = require('os');
 var Q = require('q');
 var IonicAppLib = require('ionic-app-lib');
 var ConfigXml = IonicAppLib.configXml;
+var log = IonicAppLib.logging.logger;
 var cordovaUtils = require('../utils/cordova');
 
 var cordovaRunEmulateOptions = {
@@ -54,7 +55,8 @@ function run(ionic, argv, rawCliArguments) {
   }
 
   if (runPlatform === 'ios' && os.platform() !== 'darwin') {
-    return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
+    log.error('✗ You cannot run iOS unless you are on Mac OSX.');
+    return Q();
   }
 
   var promiseList = []
@@ -79,6 +81,11 @@ function run(ionic, argv, rawCliArguments) {
   .then(function(serveOptions) {
     var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
     return cordovaUtils.execCordovaCommand(optionList, isLiveReload, serveOptions);
+  })
+  .catch(function(ex) {
+    if (ex instanceof Error) {
+      log.error(ex);
+    }
   });
 }
 

diff --git a/spec/tasks/build.spec.js b/spec/tasks/build.spec.js
@@ -70,14 +70,13 @@ describe('build command', function() {
     it('should fail if the system is not Mac and the platform is iOS', function(done) {
       spyOn(os, 'platform').andReturn('windows');
 
-      build.run(null, argv, rawCliArguments).catch(function(error) {
-        expect(error).toEqual('✗ You cannot run iOS unless you are on Mac OSX.');
+      build.run(null, argv, rawCliArguments).then(function() {
+        expect(log.error).toHaveBeenCalledWith('✗ You cannot run iOS unless you are on Mac OSX.');
         done();
       });
     });
   });
 
-
   describe('cordova platform and plugin checks', function() {
 
     var appDirectory = '/ionic/app/path';
@@ -91,11 +90,11 @@ describe('build command', function() {
 
       spyOn(cordovaUtils, 'installPlatform').andReturn(Q(true));
       spyOn(cordovaUtils, 'installPlugins').andReturn(Q(true));
-      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
     });
 
     it('should try to install the cordova platform if it is not installed', function(done) {
       spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(false);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
 
       build.run(null, argv, rawCliArguments).then(function() {
         expect(cordovaUtils.installPlatform).toHaveBeenCalledWith('ios');
@@ -106,6 +105,7 @@ describe('build command', function() {
 
     it('should not try to install the cordova platform if it is installed', function(done) {
       spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
 
       build.run(null, argv, rawCliArguments).then(function() {
         expect(cordovaUtils.installPlatform).not.toHaveBeenCalledWith();
@@ -116,6 +116,7 @@ describe('build command', function() {
     it('should install plugins if they are not installed', function(done) {
       spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
       spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(false);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
 
       build.run(null, argv, rawCliArguments).then(function() {
         expect(cordovaUtils.arePluginsInstalled).toHaveBeenCalledWith(appDirectory);
@@ -127,12 +128,37 @@ describe('build command', function() {
     it('should not install plugins if they are installed', function(done) {
       spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
       spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
 
       build.run(null, argv, rawCliArguments).then(function() {
         expect(cordovaUtils.arePluginsInstalled).toHaveBeenCalledWith(appDirectory);
         expect(cordovaUtils.installPlugins).not.toHaveBeenCalledWith();
         done();
       });
     });
+
+    it('should fail if any functions throw', function(done) {
+      var error = new Error('error occurred');
+      spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
+
+      build.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).toHaveBeenCalledWith(error);
+        done();
+      });
+    });
+
+    it('should fail if any functions throw and not log if not an instance of an Error', function(done) {
+      var error = 1;
+      spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
+
+      build.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).not.toHaveBeenCalled();
+        done();
+      });
+    });
   });
 });
diff --git a/spec/tasks/compile.spec.js b/spec/tasks/compile.spec.js
@@ -31,8 +31,18 @@ describe('compile command', function() {
       var error = new Error('error occurred');
       spyOn(ConfigXml, 'setConfigXml').andReturn(Q.reject(error));
 
-      compile.run({}, argv, rawCliArguments).catch(function(ex) {
-        expect(ex).toEqual(error);
+      compile.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).toHaveBeenCalledWith(error);
+        done();
+      });
+    });
+
+    it('should fail if any functions throw and not log if error is not instanceof Error', function(done) {
+      var error = 1;
+      spyOn(ConfigXml, 'setConfigXml').andReturn(Q.reject(error));
+
+      compile.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).not.toHaveBeenCalled();
         done();
       });
     });

diff --git a/spec/tasks/emulate.spec.js b/spec/tasks/emulate.spec.js
@@ -57,7 +57,7 @@ describe('emulate command', function() {
     it('should default to iOS for the platform', function(done) {
       spyOn(os, 'platform').andReturn('darwin');
 
-      emulate.run(null, argv, rawCliArguments).catch(function() {
+      emulate.run(null, argv, rawCliArguments).then(function() {
         expect(cordovaUtils.isPlatformInstalled).toHaveBeenCalledWith('ios', appDirectory);
         done();
       });
@@ -66,8 +66,8 @@ describe('emulate command', function() {
     it('should fail if the system is not Mac and the platform is iOS', function(done) {
       spyOn(os, 'platform').andReturn('windows');
 
-      emulate.run(null, argv, rawCliArguments).catch(function(error) {
-        expect(error).toEqual('✗ You cannot run iOS unless you are on Mac OSX.');
+      emulate.run(null, argv, rawCliArguments).then(function() {
+        expect(log.error).toHaveBeenCalledWith('✗ You cannot run iOS unless you are on Mac OSX.');
         done();
       });
     });
@@ -140,6 +140,38 @@ describe('emulate command', function() {
       spyOn(os, 'platform').andReturn('darwin');
     });
 
+    it('should fail if any functions throw', function(done) {
+      var processArguments = ['node', 'ionic', 'emulate', '-n'];
+      var rawCliArguments = processArguments.slice(2);
+      var argv = optimist(rawCliArguments).argv;
+
+      var error = new Error('error occurred');
+      spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
+
+      emulate.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).toHaveBeenCalledWith(error);
+        done();
+      });
+    });
+
+    it('should fail if any functions throw and not log if not an instance of an Error', function(done) {
+      var processArguments = ['node', 'ionic', 'emulate', '-n'];
+      var rawCliArguments = processArguments.slice(2);
+      var argv = optimist(rawCliArguments).argv;
+
+      var error = 1;
+      spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
+      spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
+
+      emulate.run({}, argv, rawCliArguments).then(function() {
+        expect(log.error).not.toHaveBeenCalled();
+        done();
+      });
+    });
+
     it('should execute the command against the cordova util', function(done) {
       var processArguments = ['node', 'ionic', 'emulate', '-n'];
       var rawCliArguments = processArguments.slice(2);