Skip to content

Commit 142128a

Browse files
jthoms1jthoms1
authored
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.
1 parent 51469d6 commit 142128a

File tree

14 files changed

+277
-17
lines changed

14 files changed

+277
-17
lines changed

‎lib/ionic/build.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ var os = require('os');
55
var Q = require('q');
66
var IonicAppLib = require('ionic-app-lib');
77
var ConfigXml = IonicAppLib.configXml;
8+
var log = IonicAppLib.logging.logger;
89
var cordovaUtils = require('../utils/cordova');
910

1011
var settings = {
@@ -37,7 +38,8 @@ function run(ionic, argv, rawCliArguments) {
3738
}
3839

3940
if (runPlatform === 'ios' && os.platform() !== 'darwin') {
40-
return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
41+
log.error('✗ You cannot run iOS unless you are on Mac OSX.');
42+
return Q();
4143
}
4244

4345
var promiseList = []
@@ -59,6 +61,11 @@ function run(ionic, argv, rawCliArguments) {
5961
.then(function() {
6062
var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
6163
return cordovaUtils.execCordovaCommand(optionList);
64+
})
65+
.catch(function(ex) {
66+
if (ex instanceof Error) {
67+
log.error(ex);
68+
}
6269
});
6370
}
6471

‎lib/ionic/compile.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
var extend = Object.assign || require('util')._extend; // eslint-disable-line no-underscore-dangle
44
var IonicAppLib = require('ionic-app-lib');
55
var ConfigXml = IonicAppLib.configXml;
6+
var log = IonicAppLib.logging.logger;
67
var cordovaUtils = require('../utils/cordova');
78

89
var settings = {
@@ -23,6 +24,11 @@ function run(ionic, argv, rawCliArguments) {
2324
}).then(function() {
2425
var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
2526
return cordovaUtils.execCordovaCommand(optionList);
27+
})
28+
.catch(function(ex) {
29+
if (ex instanceof Error) {
30+
log.error(ex);
31+
}
2632
});
2733
}
2834

‎lib/ionic/emulate.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ var os = require('os');
55
var Q = require('q');
66
var IonicAppLib = require('ionic-app-lib');
77
var ConfigXml = IonicAppLib.configXml;
8+
var log = IonicAppLib.logging.logger;
89
var cordovaUtils = require('../utils/cordova');
910

1011
var cordovaRunEmulateOptions = {
@@ -54,7 +55,8 @@ function run(ionic, argv, rawCliArguments) {
5455
}
5556

5657
if (runPlatform === 'ios' && os.platform() !== 'darwin') {
57-
return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
58+
log.error('✗ You cannot run iOS unless you are on Mac OSX.');
59+
return Q();
5860
}
5961

6062
var promiseList = []
@@ -79,6 +81,11 @@ function run(ionic, argv, rawCliArguments) {
7981
.then(function(serveOptions) {
8082
var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
8183
return cordovaUtils.execCordovaCommand(optionList, isLiveReload, serveOptions);
84+
})
85+
.catch(function(ex) {
86+
if (ex instanceof Error) {
87+
log.error(ex);
88+
}
8289
});
8390
}
8491

‎lib/ionic/platform.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ function run(ionic, argv, rawCliArguments) {
7171
log.info('Removing platform from package.json file');
7272
return State.removePlatform(appDirectory, argumentName);
7373
}
74+
})
75+
.catch(function(ex) {
76+
if (ex instanceof Error) {
77+
log.error(ex);
78+
}
7479
});
7580
}
7681

‎lib/ionic/plugin.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ function run(ionic, argv, rawCliArguments) {
7070
log.info('Removing plugin from package.json file');
7171
return State.removePlugin(appDirectory, argumentName);
7272
}
73+
})
74+
.catch(function(ex) {
75+
if (ex instanceof Error) {
76+
log.error(ex);
77+
}
7378
});
7479
}
7580

‎lib/ionic/prepare.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
var extend = Object.assign || require('util')._extend; // eslint-disable-line no-underscore-dangle
44
var IonicAppLib = require('ionic-app-lib');
55
var ConfigXml = IonicAppLib.configXml;
6+
var log = IonicAppLib.logging.logger;
67
var cordovaUtils = require('../utils/cordova');
78

89
var settings = {
@@ -24,6 +25,11 @@ function run(ionic, argv, rawCliArguments) {
2425
.then(function() {
2526
var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
2627
return cordovaUtils.execCordovaCommand(optionList);
28+
})
29+
.catch(function(ex) {
30+
if (ex instanceof Error) {
31+
log.error(ex);
32+
}
2733
});
2834
}
2935

‎lib/ionic/run.js‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ var os = require('os');
55
var Q = require('q');
66
var IonicAppLib = require('ionic-app-lib');
77
var ConfigXml = IonicAppLib.configXml;
8+
var log = IonicAppLib.logging.logger;
89
var cordovaUtils = require('../utils/cordova');
910

1011
var cordovaRunEmulateOptions = {
@@ -54,7 +55,8 @@ function run(ionic, argv, rawCliArguments) {
5455
}
5556

5657
if (runPlatform === 'ios' && os.platform() !== 'darwin') {
57-
return Q.reject('✗ You cannot run iOS unless you are on Mac OSX.');
58+
log.error('✗ You cannot run iOS unless you are on Mac OSX.');
59+
return Q();
5860
}
5961

6062
var promiseList = []
@@ -79,6 +81,11 @@ function run(ionic, argv, rawCliArguments) {
7981
.then(function(serveOptions) {
8082
var optionList = cordovaUtils.filterArgumentsForCordova(cmdName, argv, rawArgs);
8183
return cordovaUtils.execCordovaCommand(optionList, isLiveReload, serveOptions);
84+
})
85+
.catch(function(ex) {
86+
if (ex instanceof Error) {
87+
log.error(ex);
88+
}
8289
});
8390
}
8491

‎spec/tasks/build.spec.js‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,13 @@ describe('build command', function() {
7070
it('should fail if the system is not Mac and the platform is iOS', function(done) {
7171
spyOn(os, 'platform').andReturn('windows');
7272

73-
build.run(null, argv, rawCliArguments).catch(function(error) {
74-
expect(error).toEqual('✗ You cannot run iOS unless you are on Mac OSX.');
73+
build.run(null, argv, rawCliArguments).then(function() {
74+
expect(log.error).toHaveBeenCalledWith('✗ You cannot run iOS unless you are on Mac OSX.');
7575
done();
7676
});
7777
});
7878
});
7979

80-
8180
describe('cordova platform and plugin checks', function() {
8281

8382
var appDirectory = '/ionic/app/path';
@@ -91,11 +90,11 @@ describe('build command', function() {
9190

9291
spyOn(cordovaUtils, 'installPlatform').andReturn(Q(true));
9392
spyOn(cordovaUtils, 'installPlugins').andReturn(Q(true));
94-
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
9593
});
9694

9795
it('should try to install the cordova platform if it is not installed', function(done) {
9896
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(false);
97+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
9998

10099
build.run(null, argv, rawCliArguments).then(function() {
101100
expect(cordovaUtils.installPlatform).toHaveBeenCalledWith('ios');
@@ -106,6 +105,7 @@ describe('build command', function() {
106105

107106
it('should not try to install the cordova platform if it is installed', function(done) {
108107
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
108+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
109109

110110
build.run(null, argv, rawCliArguments).then(function() {
111111
expect(cordovaUtils.installPlatform).not.toHaveBeenCalledWith();
@@ -116,6 +116,7 @@ describe('build command', function() {
116116
it('should install plugins if they are not installed', function(done) {
117117
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
118118
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(false);
119+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
119120

120121
build.run(null, argv, rawCliArguments).then(function() {
121122
expect(cordovaUtils.arePluginsInstalled).toHaveBeenCalledWith(appDirectory);
@@ -127,12 +128,37 @@ describe('build command', function() {
127128
it('should not install plugins if they are installed', function(done) {
128129
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
129130
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
131+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q(0));
130132

131133
build.run(null, argv, rawCliArguments).then(function() {
132134
expect(cordovaUtils.arePluginsInstalled).toHaveBeenCalledWith(appDirectory);
133135
expect(cordovaUtils.installPlugins).not.toHaveBeenCalledWith();
134136
done();
135137
});
136138
});
139+
140+
it('should fail if any functions throw', function(done) {
141+
var error = new Error('error occurred');
142+
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
143+
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
144+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
145+
146+
build.run({}, argv, rawCliArguments).then(function() {
147+
expect(log.error).toHaveBeenCalledWith(error);
148+
done();
149+
});
150+
});
151+
152+
it('should fail if any functions throw and not log if not an instance of an Error', function(done) {
153+
var error = 1;
154+
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
155+
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
156+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
157+
158+
build.run({}, argv, rawCliArguments).then(function() {
159+
expect(log.error).not.toHaveBeenCalled();
160+
done();
161+
});
162+
});
137163
});
138164
});

‎spec/tasks/compile.spec.js‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,18 @@ describe('compile command', function() {
3131
var error = new Error('error occurred');
3232
spyOn(ConfigXml, 'setConfigXml').andReturn(Q.reject(error));
3333

34-
compile.run({}, argv, rawCliArguments).catch(function(ex) {
35-
expect(ex).toEqual(error);
34+
compile.run({}, argv, rawCliArguments).then(function() {
35+
expect(log.error).toHaveBeenCalledWith(error);
36+
done();
37+
});
38+
});
39+
40+
it('should fail if any functions throw and not log if error is not instanceof Error', function(done) {
41+
var error = 1;
42+
spyOn(ConfigXml, 'setConfigXml').andReturn(Q.reject(error));
43+
44+
compile.run({}, argv, rawCliArguments).then(function() {
45+
expect(log.error).not.toHaveBeenCalled();
3646
done();
3747
});
3848
});

‎spec/tasks/emulate.spec.js‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('emulate command', function() {
5757
it('should default to iOS for the platform', function(done) {
5858
spyOn(os, 'platform').andReturn('darwin');
5959

60-
emulate.run(null, argv, rawCliArguments).catch(function() {
60+
emulate.run(null, argv, rawCliArguments).then(function() {
6161
expect(cordovaUtils.isPlatformInstalled).toHaveBeenCalledWith('ios', appDirectory);
6262
done();
6363
});
@@ -66,8 +66,8 @@ describe('emulate command', function() {
6666
it('should fail if the system is not Mac and the platform is iOS', function(done) {
6767
spyOn(os, 'platform').andReturn('windows');
6868

69-
emulate.run(null, argv, rawCliArguments).catch(function(error) {
70-
expect(error).toEqual('✗ You cannot run iOS unless you are on Mac OSX.');
69+
emulate.run(null, argv, rawCliArguments).then(function() {
70+
expect(log.error).toHaveBeenCalledWith('✗ You cannot run iOS unless you are on Mac OSX.');
7171
done();
7272
});
7373
});
@@ -140,6 +140,38 @@ describe('emulate command', function() {
140140
spyOn(os, 'platform').andReturn('darwin');
141141
});
142142

143+
it('should fail if any functions throw', function(done) {
144+
var processArguments = ['node', 'ionic', 'emulate', '-n'];
145+
var rawCliArguments = processArguments.slice(2);
146+
var argv = optimist(rawCliArguments).argv;
147+
148+
var error = new Error('error occurred');
149+
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
150+
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
151+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
152+
153+
emulate.run({}, argv, rawCliArguments).then(function() {
154+
expect(log.error).toHaveBeenCalledWith(error);
155+
done();
156+
});
157+
});
158+
159+
it('should fail if any functions throw and not log if not an instance of an Error', function(done) {
160+
var processArguments = ['node', 'ionic', 'emulate', '-n'];
161+
var rawCliArguments = processArguments.slice(2);
162+
var argv = optimist(rawCliArguments).argv;
163+
164+
var error = 1;
165+
spyOn(cordovaUtils, 'isPlatformInstalled').andReturn(true);
166+
spyOn(cordovaUtils, 'arePluginsInstalled').andReturn(true);
167+
spyOn(cordovaUtils, 'execCordovaCommand').andReturn(Q.reject(error));
168+
169+
emulate.run({}, argv, rawCliArguments).then(function() {
170+
expect(log.error).not.toHaveBeenCalled();
171+
done();
172+
});
173+
});
174+
143175
it('should execute the command against the cordova util', function(done) {
144176
var processArguments = ['node', 'ionic', 'emulate', '-n'];
145177
var rawCliArguments = processArguments.slice(2);

0 commit comments

Comments
 (0)