Skip to content

Commit 930273f

Browse files
caridycaridy
committed
Merge branch 'feature/callback-error-handling' of https://github.com/orionhealth/shifter 
2 parents 4199a12 + bfc36cb commit 930273f

22 files changed

+427
-109
lines changed

‎.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ CVS/
99
*~
1010
.com.apple.timemachine.supported
1111
tests/assets/yql/build/*/*
12+
tests/assets/badmodule/build/*/*
13+
tests/assets/badrollup/build/*/*
1214
tests/assets-global
1315
tests/assets/freestyle/build
1416
coverage

‎conf/docs/index.mustache

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -485,3 +485,9 @@ I've added a `--lint-stderr` config option which forces all lint output to `stde
485485
<p>
486486
Adding `--recursive` to the `--walk` command will tell `shifter` to walk the directories recursively looking for `build.json` files.
487487
</p>
488+
489+
<h3 id="exp.err.hand.prog">Error handling when shifter is called programmatically</h3>
490+
491+
<p>
492+
When `shifter` is called programmatically, build errors such as UglifyJS parsing errors, are propagated through callback routines. This allows another external build process that uses `shifter` to handle the errors accordingly.
493+
</p>

‎lib/ant.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ exports.process = function (options, callback) {
137137
}
138138
});
139139
} else {
140-
log.error('no .properties files located, hit the brakes');
140+
callback('no .properties files located, hit the brakes');
141141
}
142142

143143
});

‎lib/builder.js

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ var prebuild = function (jobs, options, callback) {
6767
child.on('exit', stack.add(function (code) {
6868
log.info('build for ' + job + ' complete, downshifting');
6969
if (code) {
70-
log.error('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!');
70+
callback('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!');
7171
}
7272
}));
7373
});
@@ -81,18 +81,22 @@ exports.start = function (json, options, buildCallback) {
8181
start = new Date(),
8282
hasSkin = false,
8383
buildStat,
84-
end = function () {
84+
end = function (err) {
8585
var end = new Date();
8686
log.info('done racing, the gears are toast');
8787
log.info('finished in ' + timer.calc(start, end) + ', pretty fast huh?');
8888
if (buildCallback) {
89-
buildCallback();
89+
buildCallback(err);
9090
}
9191
},
9292
post = function (json, callback) {
9393
if (json.postbuilds && options.exec) {
9494
log.info('found a postbuild, shifting it');
95-
prebuild(json.postbuilds, options, function () {
95+
prebuild(json.postbuilds, options, function (err) {
96+
if (err) {
97+
return buildCallback(err);
98+
}
99+
96100
delete json.postbuilds;
97101
post(json, callback);
98102
});
@@ -108,7 +112,11 @@ exports.start = function (json, options, buildCallback) {
108112

109113
if (json.prebuilds && options.exec) {
110114
log.info('found a prebuild, shifting it');
111-
prebuild(json.prebuilds, options, function () {
115+
prebuild(json.prebuilds, options, function (err) {
116+
if (err) {
117+
return buildCallback(err);
118+
}
119+
112120
delete json.prebuilds;
113121
exports.start(json, options, buildCallback);
114122
});
@@ -129,17 +137,22 @@ exports.start = function (json, options, buildCallback) {
129137
json2 = JSON.parse(fs.readFileSync(options.buildFile, 'utf8'));
130138
} catch (e) {
131139
console.log(e.stack);
132-
log.error('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?');
140+
buildCallback('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?');
133141
}
134142

135143
if (pack.valid(json2)) {
136-
pack.munge(json2, options, function (json2, options) {
144+
pack.munge(json2, options, function (err, json2, options) {
137145
delete json2.exec;
138146
delete json2.prebuilds;
147+
148+
if (err) {
149+
return buildCallback(err);
150+
}
151+
139152
exports.start(json2, options, buildCallback);
140153
});
141154
} else {
142-
log.error('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!');
155+
buildCallback('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!');
143156
}
144157
} else {
145158
exports.start(json, options, buildCallback);
@@ -168,7 +181,10 @@ exports.start = function (json, options, buildCallback) {
168181
}
169182
});
170183

171-
stack.done(function () {
184+
stack.done(function (err) {
185+
if (err) {
186+
return end(err);
187+
}
172188
var rollups = [];
173189
if (json.rollups) {
174190
log.info('build has rollup builds, shifting them now');

‎lib/index.js

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,29 @@ var runQueue = function() {
3434
var item = queue.pop();
3535
if (item) {
3636
buildRunning = true;
37-
exports.init(item.opts, function() {
37+
exports.init(item.opts, function(err) {
3838
buildRunning = false;
39+
if (err) {
40+
return item.callback(err);
41+
}
3942
item.callback();
4043
runQueue();
4144
});
4245
}
4346
}
4447
};
4548

49+
function logAndExit(err) {
50+
if (err) {
51+
log.err(err);
52+
process.exit(1);
53+
}
54+
}
55+
4656
exports.add = function(opts, callback) {
4757
queue.push({
4858
opts: opts,
49-
callback: callback
59+
callback: callback || logAndExit
5060
});
5161
runQueue();
5262
};
@@ -59,6 +69,10 @@ exports.init = function (opts, initCallback) {
5969

6070
log.reset(options);
6171

72+
if (!initCallback) {
73+
initCallback = logAndExit;
74+
}
75+
6276
if (options.cwd) {
6377
CWD = options.cwd;
6478
}
@@ -82,15 +96,15 @@ exports.init = function (opts, initCallback) {
8296
require('./help');
8397
return;
8498
}
85-
99+
86100
if (options.quiet) {
87101
log.quiet();
88102
}
89103

90104
if (options.silent) {
91105
log.silent();
92106
}
93-
107+
94108
if (options['global-config']) {
95109
log.info('racing to find the closest .shifter.json file');
96110
find(CWD, '.shifter.json', function(err, file) {
@@ -129,19 +143,23 @@ exports.init = function (opts, initCallback) {
129143
var json, walk, ant, mods, builder;
130144
if (yes) {
131145
if (options.ant) {
132-
log.error('already has a ' + buildFileName + ' file, hitting the brakes');
146+
return initCallback('already has a ' + buildFileName + ' file, hitting the brakes');
133147
}
134148
log.info('found ' + buildFileName + ' file, shifting');
135149
if (path.extname(buildFileName) === '.json') {
136150
try {
137151
json = require(buildFile);
138152
} catch (e) {
139153
console.log(e.stack);
140-
log.error('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?');
154+
return initCallback('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?');
141155
}
142156
if (pack.valid(json)) {
143157
log.info('putting the hammer down, let\'s build this thing!');
144-
pack.munge(json, options, function (json, options) {
158+
pack.munge(json, options, function (err, json, options) {
159+
if (err) {
160+
return initCallback(err);
161+
}
162+
145163
if (options.list) {
146164
mods = Object.keys(json.builds).sort();
147165
log.info('This module includes these builds:');
@@ -153,16 +171,14 @@ exports.init = function (opts, initCallback) {
153171
} else {
154172
builder = require('./builder');
155173
builder.reset();
156-
builder.start(json, options, function() {
174+
builder.start(json, options, function(err) {
157175
buildRunning = false;
158-
if (initCallback) {
159-
initCallback();
160-
}
176+
initCallback(err);
161177
});
162178
}
163179
});
164180
} else {
165-
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
181+
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
166182
}
167183
} else if (path.extname(buildFileName) === '.js') {
168184
// probably a row module
@@ -179,23 +195,21 @@ exports.init = function (opts, initCallback) {
179195
vm.runInContext(fs.readFileSync(buildFile, 'utf8'),
180196
contextForRunInContext, buildFile);
181197
} catch (e) {
182-
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
198+
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
183199
}
184200
if (mods) {
185201
// raw yui module without build.json
186202
builder = require('./builder');
187203
builder.reset();
188204
builder.start({
189205
builds: mods
190-
}, options, function() {
206+
}, options, function(err) {
191207
buildRunning = false;
192-
if (initCallback) {
193-
initCallback();
194-
}
208+
initCallback(err);
195209
});
196210
}
197211
} else {
198-
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
212+
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
199213
}
200214
} else {
201215
if (options.walk) {
@@ -204,7 +218,11 @@ exports.init = function (opts, initCallback) {
204218
} else {
205219
log.warn('no ' + buildFileName + ' file, downshifting to convert ant files');
206220
ant = require('./ant');
207-
ant.process(options, function () {
221+
ant.process(options, function (err) {
222+
if (err) {
223+
return initCallback(err);
224+
}
225+
208226
if (!options.ant) {
209227
exports.init(options, initCallback);
210228
}

‎lib/log.js

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,19 +57,13 @@ exports.warn = function (str) {
5757
}
5858
};
5959

60-
exports.error = function (str) {
61-
if (!silent) {
62-
console.error(prefix, exports.color('[error]', 'red'), str);
63-
}
64-
process.exit(1);
65-
};
66-
6760
exports.err = function (str) {
6861
if (!silent) {
6962
console.error(prefix, exports.color('[err]', 'red'), str);
7063
}
7164
};
7265

66+
exports.error = exports.err;
7367

7468
exports.console = {
7569
log: function() {

‎lib/module.js

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ var Stack = require('./stack').Stack,
5555
callback: function (e) {
5656
log.err('compression failed');
5757
log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white'));
58-
log.error('dropped the clutch, build failed');
58+
log.err('dropped the clutch, build failed');
59+
// Not calling back here, the jsminify task will callback with an error, failing the queue
5960
}
6061
};
6162
}
@@ -91,7 +92,8 @@ var Stack = require('./stack').Stack,
9192
}
9293
});
9394
if (lintFail) {
94-
log.error('lint failed, aborting build');
95+
// Return an error to callback with, which should fail the build
96+
return 'lint failed, aborting build';
9597
}
9698
} else {
9799
log.info('css lint passed for ' + file);
@@ -141,7 +143,8 @@ var Stack = require('./stack').Stack,
141143
}
142144
});
143145
if (lintFail) {
144-
log.error('lint failed, aborting build');
146+
// Return an error to callback with, which should fail the build
147+
return 'lint failed, aborting build';
145148
}
146149
}
147150
}
@@ -362,13 +365,13 @@ var buildJS = function (mod, name, callback) {
362365
if (err) {
363366
if (/file has not changed/.test(err)) {
364367
log.warn(name + ': ' + err);
368+
} else if (/ENOENT/.test(err)) {
369+
log.err('Failed to open file: ' + err.path);
365370
} else {
366-
if (/ENOENT/.test(err)) {
367-
log.error('Failed to open file: ' + err.path);
368-
}
369371
log.err(name + ': ' + err);
370372
}
371373
}
374+
372375
callback(err, result);
373376
});
374377

@@ -499,7 +502,8 @@ var buildSkin = function (mod, name, callback) {
499502
fs.readdir(path.join(shifter.cwd(), 'assets', subMod, 'skins'), stack.add(function (err, skins) {
500503
if (err) {
501504
log.console.log(err);
502-
log.error('skin files are not right!');
505+
log.err('skin files are not right!');
506+
return;
503507
}
504508

505509
//Walk the skins and write them out
@@ -541,7 +545,8 @@ var buildSkin = function (mod, name, callback) {
541545
if (err) {
542546
log.err(err);
543547
if (err.code === 'ENOENT') {
544-
log.error('skin file is missing: ' + err.path);
548+
log.err('skin file is missing: ' + err.path);
549+
return;
545550
}
546551
}
547552

@@ -739,10 +744,10 @@ var build = function (mod, name, options, callback) {
739744

740745

741746

742-
stack.done(function () {
747+
stack.done(function (errs) {
743748
if (!stack.complete) {
744749
stack.complete = true;
745-
callback();
750+
callback(errs);
746751
}
747752
});
748753
};
@@ -811,11 +816,11 @@ exports.build = function (mod, name, options, callback) {
811816

812817
mod.buildDir = options['build-dir'];
813818

814-
var end = function () {
819+
var end = function (err) {
815820
if (mod.postexec) {
816821
exec(mod.postexec, name, callback);
817822
} else {
818-
callback();
823+
callback(err);
819824
}
820825
};
821826
if (mod.exec) {
@@ -880,7 +885,7 @@ var _rollup = function (mod, name, options, callback) {
880885
.write(path.join(mod.buildDir, fileName, fileName + '-min.js'))
881886
.run(function (err) {
882887
if (err) {
883-
log.error(name + ' rollup: ' + err);
888+
return callback(name + ' rollup: ' + err);
884889
}
885890
callback();
886891
});

0 commit comments

Comments
 (0)