Skip to content

Commit

Permalink
Merge branch 'feature/callback-error-handling' of https://github.com/…
Browse files Browse the repository at this point in the history 
…orionhealth/shifter
  • Loading branch information
@caridy
caridy committed Apr 15, 2014
2 parents 4199a12 + bfc36cb commit 930273f
Show file tree
Hide file tree
Showing 22 changed files with 427 additions and 109 deletions.
2 changes: 2 additions & 0 deletions .gitignore
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ CVS/
*~
.com.apple.timemachine.supported
tests/assets/yql/build/*/*
tests/assets/badmodule/build/*/*
tests/assets/badrollup/build/*/*
tests/assets-global
tests/assets/freestyle/build
coverage
6 changes: 6 additions & 0 deletions conf/docs/index.mustache
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,3 +485,9 @@ I've added a `--lint-stderr` config option which forces all lint output to `stde
<p>
Adding `--recursive` to the `--walk` command will tell `shifter` to walk the directories recursively looking for `build.json` files.
</p>

<h3 id="exp.err.hand.prog">Error handling when shifter is called programmatically</h3>

<p>
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.
</p>
2 changes: 1 addition & 1 deletion lib/ant.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ exports.process = function (options, callback) {
}
});
} else {
log.error('no .properties files located, hit the brakes');
callback('no .properties files located, hit the brakes');
}

});
Expand Down
34 changes: 25 additions & 9 deletions lib/builder.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ var prebuild = function (jobs, options, callback) {
child.on('exit', stack.add(function (code) {
log.info('build for ' + job + ' complete, downshifting');
if (code) {
log.error('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!');
callback('prebuild ' + job + ' failed, exited with code ' + code + ', hitting the brakes, fix it and try again!');
}
}));
});
Expand All @@ -81,18 +81,22 @@ exports.start = function (json, options, buildCallback) {
start = new Date(),
hasSkin = false,
buildStat,
end = function () {
end = function (err) {
var end = new Date();
log.info('done racing, the gears are toast');
log.info('finished in ' + timer.calc(start, end) + ', pretty fast huh?');
if (buildCallback) {
buildCallback();
buildCallback(err);
}
},
post = function (json, callback) {
if (json.postbuilds && options.exec) {
log.info('found a postbuild, shifting it');
prebuild(json.postbuilds, options, function () {
prebuild(json.postbuilds, options, function (err) {
if (err) {
return buildCallback(err);
}

delete json.postbuilds;
post(json, callback);
});
Expand All @@ -108,7 +112,11 @@ exports.start = function (json, options, buildCallback) {

if (json.prebuilds && options.exec) {
log.info('found a prebuild, shifting it');
prebuild(json.prebuilds, options, function () {
prebuild(json.prebuilds, options, function (err) {
if (err) {
return buildCallback(err);
}

delete json.prebuilds;
exports.start(json, options, buildCallback);
});
Expand All @@ -129,17 +137,22 @@ exports.start = function (json, options, buildCallback) {
json2 = JSON.parse(fs.readFileSync(options.buildFile, 'utf8'));
} catch (e) {
console.log(e.stack);
log.error('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?');
buildCallback('hitting the brakes! failed to parse ' + options.buildFileName + ', syntax error?');
}

if (pack.valid(json2)) {
pack.munge(json2, options, function (json2, options) {
pack.munge(json2, options, function (err, json2, options) {
delete json2.exec;
delete json2.prebuilds;

if (err) {
return buildCallback(err);
}

exports.start(json2, options, buildCallback);
});
} else {
log.error('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!');
buildCallback('hitting the brakes, your ' + options.buildFileName + ' file is invalid, please fix it!');
}
} else {
exports.start(json, options, buildCallback);
Expand Down Expand Up @@ -168,7 +181,10 @@ exports.start = function (json, options, buildCallback) {
}
});

stack.done(function () {
stack.done(function (err) {
if (err) {
return end(err);
}
var rollups = [];
if (json.rollups) {
log.info('build has rollup builds, shifting them now');
Expand Down
56 changes: 37 additions & 19 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,29 @@ var runQueue = function() {
var item = queue.pop();
if (item) {
buildRunning = true;
exports.init(item.opts, function() {
exports.init(item.opts, function(err) {
buildRunning = false;
if (err) {
return item.callback(err);
}
item.callback();
runQueue();
});
}
}
};

function logAndExit(err) {
if (err) {
log.err(err);
process.exit(1);
}
}

exports.add = function(opts, callback) {
queue.push({
opts: opts,
callback: callback
callback: callback || logAndExit
});
runQueue();
};
Expand All @@ -59,6 +69,10 @@ exports.init = function (opts, initCallback) {

log.reset(options);

if (!initCallback) {
initCallback = logAndExit;
}

if (options.cwd) {
CWD = options.cwd;
}
Expand All @@ -82,15 +96,15 @@ exports.init = function (opts, initCallback) {
require('./help');
return;
}

if (options.quiet) {
log.quiet();
}

if (options.silent) {
log.silent();
}

if (options['global-config']) {
log.info('racing to find the closest .shifter.json file');
find(CWD, '.shifter.json', function(err, file) {
Expand Down Expand Up @@ -129,19 +143,23 @@ exports.init = function (opts, initCallback) {
var json, walk, ant, mods, builder;
if (yes) {
if (options.ant) {
log.error('already has a ' + buildFileName + ' file, hitting the brakes');
return initCallback('already has a ' + buildFileName + ' file, hitting the brakes');
}
log.info('found ' + buildFileName + ' file, shifting');
if (path.extname(buildFileName) === '.json') {
try {
json = require(buildFile);
} catch (e) {
console.log(e.stack);
log.error('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?');
return initCallback('hitting the brakes! failed to parse ' + buildFileName + ', syntax error?');
}
if (pack.valid(json)) {
log.info('putting the hammer down, let\'s build this thing!');
pack.munge(json, options, function (json, options) {
pack.munge(json, options, function (err, json, options) {
if (err) {
return initCallback(err);
}

if (options.list) {
mods = Object.keys(json.builds).sort();
log.info('This module includes these builds:');
Expand All @@ -153,16 +171,14 @@ exports.init = function (opts, initCallback) {
} else {
builder = require('./builder');
builder.reset();
builder.start(json, options, function() {
builder.start(json, options, function(err) {
buildRunning = false;
if (initCallback) {
initCallback();
}
initCallback(err);
});
}
});
} else {
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
}
} else if (path.extname(buildFileName) === '.js') {
// probably a row module
Expand All @@ -179,23 +195,21 @@ exports.init = function (opts, initCallback) {
vm.runInContext(fs.readFileSync(buildFile, 'utf8'),
contextForRunInContext, buildFile);
} catch (e) {
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
}
if (mods) {
// raw yui module without build.json
builder = require('./builder');
builder.reset();
builder.start({
builds: mods
}, options, function() {
}, options, function(err) {
buildRunning = false;
if (initCallback) {
initCallback();
}
initCallback(err);
});
}
} else {
log.error('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
return initCallback('hitting the brakes, your ' + buildFileName + ' file is invalid, please fix it!');
}
} else {
if (options.walk) {
Expand All @@ -204,7 +218,11 @@ exports.init = function (opts, initCallback) {
} else {
log.warn('no ' + buildFileName + ' file, downshifting to convert ant files');
ant = require('./ant');
ant.process(options, function () {
ant.process(options, function (err) {
if (err) {
return initCallback(err);
}

if (!options.ant) {
exports.init(options, initCallback);
}
Expand Down
8 changes: 1 addition & 7 deletions lib/log.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,19 +57,13 @@ exports.warn = function (str) {
}
};

exports.error = function (str) {
if (!silent) {
console.error(prefix, exports.color('[error]', 'red'), str);
}
process.exit(1);
};

exports.err = function (str) {
if (!silent) {
console.error(prefix, exports.color('[err]', 'red'), str);
}
};

exports.error = exports.err;

exports.console = {
log: function() {
Expand Down
31 changes: 18 additions & 13 deletions lib/module.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ var Stack = require('./stack').Stack,
callback: function (e) {
log.err('compression failed');
log.console.log(' ' + String(e.message).trim() + log.color(' // line ' + e.line + ', pos ' + e.col, 'white'));
log.error('dropped the clutch, build failed');
log.err('dropped the clutch, build failed');
// Not calling back here, the jsminify task will callback with an error, failing the queue
}
};
}
Expand Down Expand Up @@ -91,7 +92,8 @@ var Stack = require('./stack').Stack,
}
});
if (lintFail) {
log.error('lint failed, aborting build');
// Return an error to callback with, which should fail the build
return 'lint failed, aborting build';
}
} else {
log.info('css lint passed for ' + file);
Expand Down Expand Up @@ -141,7 +143,8 @@ var Stack = require('./stack').Stack,
}
});
if (lintFail) {
log.error('lint failed, aborting build');
// Return an error to callback with, which should fail the build
return 'lint failed, aborting build';
}
}
}
Expand Down Expand Up @@ -362,13 +365,13 @@ var buildJS = function (mod, name, callback) {
if (err) {
if (/file has not changed/.test(err)) {
log.warn(name + ': ' + err);
} else if (/ENOENT/.test(err)) {
log.err('Failed to open file: ' + err.path);
} else {
if (/ENOENT/.test(err)) {
log.error('Failed to open file: ' + err.path);
}
log.err(name + ': ' + err);
}
}

callback(err, result);
});

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

//Walk the skins and write them out
Expand Down Expand Up @@ -541,7 +545,8 @@ var buildSkin = function (mod, name, callback) {
if (err) {
log.err(err);
if (err.code === 'ENOENT') {
log.error('skin file is missing: ' + err.path);
log.err('skin file is missing: ' + err.path);
return;
}
}

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



stack.done(function () {
stack.done(function (errs) {
if (!stack.complete) {
stack.complete = true;
callback();
callback(errs);
}
});
};
Expand Down Expand Up @@ -811,11 +816,11 @@ exports.build = function (mod, name, options, callback) {

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

var end = function () {
var end = function (err) {
if (mod.postexec) {
exec(mod.postexec, name, callback);
} else {
callback();
callback(err);
}
};
if (mod.exec) {
Expand Down Expand Up @@ -880,7 +885,7 @@ var _rollup = function (mod, name, options, callback) {
.write(path.join(mod.buildDir, fileName, fileName + '-min.js'))
.run(function (err) {
if (err) {
log.error(name + ' rollup: ' + err);
return callback(name + ' rollup: ' + err);
}
callback();
});
Expand Down
Loading

0 comments on commit 930273f

Please sign in to comment.