diff --git a/lib/builder.js b/lib/builder.js index 3a326009..3361caab 100644 --- a/lib/builder.js +++ b/lib/builder.js @@ -15,6 +15,7 @@ const SourceNodeWrapper = require('./wrappers/source-node'); const BuilderError = require('./errors/builder'); const NodeSetupError = require('./errors/node-setup'); const BuildError = require('./errors/build'); +const CancelationRequest = require('./cancellation-request'); // Clean up left-over temporary directories on uncaught exception. tmp.setGracefulCleanup(); @@ -61,6 +62,7 @@ module.exports = class Builder { this.checkInputPathsExist(); this.setupTmpDirs(); + this._cancelationRequest = undefined; // Now that temporary directories are set up, we need to run the rest of the // constructor in a try/catch block to clean them up if necessary. @@ -81,8 +83,16 @@ module.exports = class Builder { // This method will never throw, and it will never be rejected with anything // other than a BuildError. build() { + if (this._cancelationRequest) { + return RSVP.Promise.reject( + new BuilderError('Cannot start a build if one is already running') + ); + } + + let promise = RSVP.Promise.resolve(); + this.buildId++; - let promise = RSVP.resolve(); + this.nodeWrappers.forEach(nw => { // We use `.forEach` instead of `for` to close nested functions over `nw` @@ -93,16 +103,36 @@ module.exports = class Builder { // We use a nested .then/.catch so that the .catch can only catch errors // from this node, but not from previous nodes. return RSVP.resolve() + .then(() => this._cancelationRequest.throwIfRequested()) .then(() => this.trigger('beginNode', nw)) .then(() => nw.build()) - .finally(() => this.trigger('endNode', nw)) + .finally(() => { + if (this._cancelationRequest.isCancelled) { + return; + } + this.trigger('endNode', nw); + }) + .then(() => this._cancelationRequest.throwIfRequested()) .catch(err => { throw new BuildError(err, nw); }); }); }); - return promise; + this._cancelationRequest = new CancelationRequest(promise); + + return promise.finally(() => { + this._cancelationRequest = null; + }); + } + + cancel() { + if (!this._cancelationRequest) { + // No current build, so no cancellation + return RSVP.Promise.resolve(); + } + + return this._cancelationRequest.cancel(); } // Destructor-like method. Cleanup is synchronous at the moment, but in the diff --git a/lib/cancellation-request.js b/lib/cancellation-request.js new file mode 100644 index 00000000..261c31e3 --- /dev/null +++ b/lib/cancellation-request.js @@ -0,0 +1,37 @@ +'use strict'; + +const CancelationError = require('./errors/cancelation'); +const BuilderError = require('./errors/builder'); + +module.exports = class CancelationRequest { + constructor(pendingWork) { + this._isCancelled = false; + this._pendingWork = pendingWork; // all + } + + get isCancelled() { + return this._isCancelled; + } + + throwIfRequested() { + if (this._isCancelled) { + throw new CancelationError('BUILD CANCELLED'); + } + } + + cancel() { + // if we ever expose the cancellation to plugins, we should not expose + // cancel(), rather it should be expose similarity to the Promise + // executor + this._isCancelled = true; + + return this._pendingWork.catch(e => { + // a rejection with a cancel cancel promise + if (BuilderError.isBuilderError(e)) { + return; + } + + throw e; + }); + } +}; diff --git a/lib/errors/build.js b/lib/errors/build.js index 9c8805d4..495ad3e6 100644 --- a/lib/errors/build.js +++ b/lib/errors/build.js @@ -44,7 +44,10 @@ module.exports = class BuildError extends BuilderError { instantiationStack; super(message); + // consider for x in y this.stack = originalError.stack; + this.isSilent = originalError.isSilent; + this.isCancellation = originalError.isCancellation; // This error API can change between minor Broccoli version bumps this.broccoliPayload = { diff --git a/lib/errors/builder.js b/lib/errors/builder.js index 0914f465..6b46cfb0 100644 --- a/lib/errors/builder.js +++ b/lib/errors/builder.js @@ -2,6 +2,10 @@ // Base class for builder errors module.exports = class BuilderError extends Error { + static isBuilderError(error) { + return error !== null && typeof error === 'object' && error.isBuilderError; + } + constructor(a, b, c) { // Subclassing Error in ES5 is non-trivial because reasons, so we need this // extra constructor logic from http://stackoverflow.com/a/17891099/525872. @@ -12,5 +16,6 @@ module.exports = class BuilderError extends Error { temp.name = this.name = this.constructor.name; this.stack = temp.stack; this.message = temp.message; + this.isBuilderError = true; } }; diff --git a/lib/errors/cancelation.js b/lib/errors/cancelation.js new file mode 100644 index 00000000..ce0a8cd5 --- /dev/null +++ b/lib/errors/cancelation.js @@ -0,0 +1,23 @@ +'use strict'; + +// Base class for builder errors +module.exports = class Cancellation extends Error { + static isCancellation(e) { + return typeof e === 'object' && e !== null && e.isCancellation == true; + } + + constructor(a, b, c) { + // Subclassing Error in ES5 is non-trivial because reasons, so we need this + // extra constructor logic from http://stackoverflow.com/a/17891099/525872. + // Note that ES5 subclasses of BuilderError don't in turn need any special + // code. + let temp = super(a, b, c); + // Need to assign temp.name for correct error class in .stack and .message + temp.name = this.name = this.constructor.name; + this.stack = temp.stack; + this.message = temp.message; + + this.isCancellation = true; + this.isSilent = true; + } +}; diff --git a/test/builder_test.js b/test/builder_test.js index 9acabec2..91861e85 100644 --- a/test/builder_test.js +++ b/test/builder_test.js @@ -29,6 +29,9 @@ RSVP.on('error', error => { throw error; }); +function wait(time) { + return new RSVP.Promise(resolve => setTimeout(resolve, time || 0)); +} // Make a default set of plugins with the latest Plugin version. In some tests // we'll shadow this `plugins` variable with one created with different versions. const plugins = makePlugins(Plugin); @@ -718,4 +721,178 @@ describe('Builder', function() { expect(Builder.prototype).to.have.nested.property('features.persistentOutputFlag', true); }); }); + + describe('cancel()', function() { + it('handles a cancel without an active build (has no affect)', function() { + let stepA = new plugins.Noop(); + let pipeline = new Builder(stepA); + + pipeline.cancel(); + pipeline.cancel(); // ensure double cancel is always safe here + + expect(stepA.buildCount).to.eql(0); + + return pipeline.build().then(() => { + expect(stepA.buildCount).to.eql(1); + }); + }); + + it('returns a promise which waits until cancellation is complete', function() { + let pipeline; + let stepAIsComplete = false; + let cancellingIsComplete = false; + + class StepA extends plugins.Deferred { + build() { + resolveCancel(pipeline.cancel()); + + setTimeout(() => this.resolve(), 50); + + return super.build().then(() => { + stepAIsComplete = true; + expect(cancellingIsComplete).to.eql( + false, + 'expected cancelling to not complete before stepA' + ); + }); + } + } + + const stepA = new StepA(); + const stepB = new plugins.Deferred([stepA]); + + pipeline = new Builder(stepB); + + const building = expect(pipeline.build()).to.eventually.be.rejectedWith('BUILD CANCELLED'); + + let resolveCancel; + const cancelling = new RSVP.Promise(resolve => (resolveCancel = resolve)); + + return RSVP.Promise.all([ + building, + cancelling.then(() => { + cancellingIsComplete = true; + expect(stepAIsComplete).to.eql( + true, + 'expected stepA to be complete before cancel completes' + ); + }), + ]); + }); + + it('errors if a new build is attempted during a cancellation', function() { + const stepA = new plugins.Deferred(); + const pipeline = new Builder(stepA); + const build = pipeline.build(); + + return wait().then(() => { + let wait = RSVP.Promise.all([ + expect(build).to.eventually.be.rejectedWith('BUILD CANCELLED'), + pipeline.cancel(), + expect(pipeline.build()).to.eventually.be.rejectedWith( + 'Cannot start a build if one is already running' + ), + expect(pipeline.build()).to.eventually.be.rejectedWith( + 'Cannot start a build if one is already running' + ), + expect(pipeline.build()).to.eventually.be.rejectedWith( + 'Cannot start a build if one is already running' + ), + ]); + + stepA.resolve(); + + return wait; + }); + }); + + it('build before cancel completes', function() { + let stepA = new plugins.Noop(); + let pipeline = new Builder(stepA); + + pipeline.cancel(); + pipeline.cancel(); // ensure double cancel is always safe here + + expect(stepA.buildCount).to.eql(0); + + return pipeline.build().then(() => { + expect(stepA.buildCount).to.eql(1); + }); + }); + + it('it cancels immediately if cancelled immediately after build', function() { + const step = new plugins.Deferred(); + let pipeline = new Builder(step); + step.resolve(); + let build = pipeline.build(); + pipeline.cancel(); + + return RSVP.Promise.resolve( + expect(build).to.eventually.be.rejectedWith('BUILD CANCELLED') + ).finally(() => { + expect(step.buildCount).to.eql(0); + }); + }); + + it('completes the current task before cancelling, and can be resumed', function() { + let pipeline; + + class SometimesBuildCanceller extends plugins.Noop { + build() { + super.build(); + + // cancel the first build, but allow the rest to proceed + if (this.buildCount === 1 || this.buildCount === 3) { + pipeline.cancel(); + } + } + } + + const stepA = new SometimesBuildCanceller(); + + const stepB = new plugins.Noop([stepA]); + const stepC = new plugins.Noop([stepB]); + + pipeline = new Builder(stepC); + + // build #1 + let build = pipeline.build(); + + // once the build has begun: + // 1. allow StepA to complete + // 2. cancel the build + // 3. wait for the build settle + // (stepB and stepC should not have run) + + return expect( + build.finally(() => { + expect(stepA.buildCount).to.eql(1, 'stepA.buildCount'); + expect(stepB.buildCount).to.eql(0, 'stepB.buildCount'); + expect(stepC.buildCount).to.eql(0, 'stepC.buildCount'); + }) + ) + .to.eventually.be.rejectedWith('BUILD CANCELLED') + .then(() => { + // build #2 + let build = pipeline.build(); + + return build.then(() => { + expect(stepA.buildCount).to.eql(2, 'stepA.buildCount'); + expect(stepB.buildCount).to.eql(1, 'stepB.buildCount'); + expect(stepC.buildCount).to.eql(1, 'stepC.buildCount'); + + // build #3 + return expect(pipeline.build()) + .to.eventually.be.rejectedWith('BUILD CANCELLED') + .then(() => { + // build will cancel again during stepA (before the stepB) so + // only stepA should have made progress + expect(stepA.buildCount).to.eql(3, 'stepA.buildCount'); + expect(stepB.buildCount).to.eql(1, 'stepB.buildCount'); + expect(stepC.buildCount).to.eql(1, 'stepC.buildCount'); + }); + }); + }); + }); + }); }); diff --git a/test/cancellation-request-test.js b/test/cancellation-request-test.js new file mode 100644 index 00000000..7ed061c2 --- /dev/null +++ b/test/cancellation-request-test.js @@ -0,0 +1,52 @@ +'use strict'; + +const chai = require('chai'); +const expect = chai.expect; +const chaiAsPromised = require('chai-as-promised'); +const CancelationRequest = require('../lib/cancellation-request'); +const RSVP = require('rsvp'); +const BuilderError = require('../lib/errors/builder'); + +chai.use(chaiAsPromised); + +describe('cancellation-request', function() { + it('.isCancelled / .cancel', function() { + let request = new CancelationRequest(RSVP.Promise.resolve()); + + expect(request.isCancelled).to.eql(false); + let wait = request.cancel(); + expect(request.isCancelled).to.eql(true); + + return wait.then(() => { + expect(request.isCancelled).to.eql(true); + }); + }); + + it('.throwIfRequested (requested)', function() { + let request = new CancelationRequest(RSVP.Promise.resolve()); + + request.throwIfRequested(); + + return request.cancel().then(() => { + expect(() => { + request.throwIfRequested(); + }).to.throw('BUILD CANCELLED'); + + expect(() => { + request.throwIfRequested(); + }).to.throw('BUILD CANCELLED'); + }); + }); + + it('.cancel (with builder rejection)', function() { + let request = new CancelationRequest(RSVP.Promise.reject(new BuilderError())); + + return request.cancel(); + }); + + it('.cancel (with non-builder rejection)', function() { + let request = new CancelationRequest(RSVP.Promise.reject(new Error('OOPS'))); + + return expect(request.cancel()).to.eventually.be.rejectedWith('OOPS'); + }); +}); diff --git a/test/plugins.js b/test/plugins.js index f3f5ab60..3f333c2c 100644 --- a/test/plugins.js +++ b/test/plugins.js @@ -9,36 +9,48 @@ const symlinkOrCopySync = require('symlink-or-copy').sync; module.exports = function(Plugin) { const plugins = {}; - plugins.Noop = class NoopPlugin extends Plugin { - build() {} - }; - - // This plugin writes foo.js into its outputPath - plugins.Veggies = class VeggiesPlugin extends Plugin { + class CountingPlugin extends Plugin { constructor(inputNodes, options) { super(inputNodes || [], options); + this._count = 0; } build() { + this._count++; + } + + get buildCount() { + return this._count; + } + } + + plugins.Noop = class NoopPlugin extends CountingPlugin {}; + + // This plugin writes foo.js into its outputPath + plugins.Veggies = class VeggiesPlugin extends CountingPlugin { + build() { + super.build(); fs.writeFileSync(this.outputPath + '/veggies.txt', 'tasty'); } }; - plugins.Merge = class MergePlugin extends Plugin { + plugins.Merge = class MergePlugin extends CountingPlugin { build() { + super.build(); for (let i = 0; i < this.inputPaths.length; i++) { symlinkOrCopySync(this.inputPaths[i], this.outputPath + '/' + i); } } }; - plugins.Failing = class FailingPlugin extends Plugin { + plugins.Failing = class FailingPlugin extends CountingPlugin { constructor(errorObject, options) { super([], options); this.errorObject = errorObject; } build() { + super.build(); throw this.errorObject; } }; @@ -47,7 +59,7 @@ module.exports = function(Plugin) { // The build will stall until you call node.finishBuild(). // To wait until the build starts, chain on node.buildStarted. // Don't build more than once. - plugins.Async = class AsyncPlugin extends Plugin { + plugins.Async = class AsyncPlugin extends CountingPlugin { constructor(inputNodes, options) { super(inputNodes || [], options); this.buildFinishedDeferred = RSVP.defer(); @@ -56,6 +68,7 @@ module.exports = function(Plugin) { } build() { + super.build(); this.buildStartedDeferred.resolve(); return this.buildFinishedDeferred.promise; } @@ -69,15 +82,31 @@ module.exports = function(Plugin) { } }; - plugins.Sleeping = class SleepingPlugin extends Plugin { + plugins.Sleeping = class SleepingPlugin extends CountingPlugin { constructor(inputNodes, options) { super(inputNodes || [], options); } build() { + super.build(); return new RSVP.Promise(resolve => setTimeout(resolve, 10)); } }; + plugins.Deferred = class DeferredPlugin extends CountingPlugin { + constructor(inputNodes, options) { + super(inputNodes || [], options); + this.promise = new RSVP.Promise((resolve, reject) => { + this.resolve = resolve; + this.reject = reject; + }); + } + + build() { + super.build(); + return this.promise; + } + }; + return plugins; };