[WIP][Bugfix] Mid-build Retries should not cause spurious errors.

broccolijs · Mar 27, 2020 · e9e4bef · e9e4bef
1 parent 30f9581
commit e9e4bef
Show file tree

Hide file tree

Showing 7 changed files with 160 additions and 112 deletions.
diff --git a/lib/builder.ts b/lib/builder.ts
@@ -8,6 +8,7 @@ import BuilderError from './errors/builder';
 import NodeSetupError from './errors/node-setup';
 import BuildError from './errors/build';
 import CancelationRequest from './cancelation-request';
+import RetryCancellationRequest from './errors/retry-cancelation';
 import filterMap from './utils/filter-map';
 import { EventEmitter } from 'events';
 import { TransformNode, SourceNode, Node } from 'broccoli-node-api';
@@ -53,7 +54,7 @@ class Builder extends EventEmitter {
   watchedPaths: string[];
   _nodeWrappers: Map<TransformNode | SourceNode, NodeWrappers>;
   outputNodeWrapper: NodeWrappers;
-  _cancelationRequest: any;
+  _cancelationRequest?: CancelationRequest;
   outputPath: string;
   buildId: number;
   builderTmpDir!: string;
@@ -143,8 +144,12 @@ class Builder extends EventEmitter {
       //
       // 1. build up a promise chain, which represents the complete build
       pipeline = pipeline.then(async () => {
+        if (this._cancelationRequest) {
+          this._cancelationRequest.throwIfRequested();
+        } else {
+          throw new BuilderError('Broccoli: The current build is missing a CancelationRequest, this is unexpected please report an issue: https://github.com/broccolijs/broccoli/issues/new ');
+        }
         // 3. begin next build step
-        this._cancelationRequest.throwIfRequested();
         this.emit('beginNode', nw);
         try {
           await nw.build();
@@ -166,17 +171,35 @@ class Builder extends EventEmitter {
     // cleanup, or restarting the build itself.
     this._cancelationRequest = new CancelationRequest(pipeline);
 
+    let skipFinally = false;
     try {
       await pipeline;
       this.buildHeimdallTree(this.outputNodeWrapper);
+    } catch (e) {
+      if (RetryCancellationRequest.isRetry(e)) {
+        this._cancelationRequest = undefined;
+        await new Promise((resolve, reject) => {
+          setTimeout(() => {
+            try {
+              resolve(this.build());
+            } catch(e) {
+              reject(e);
+            }
+          }, e.retryIn);
+        })
+        skipFinally = true;
+      } else {
+        throw e;
+      }
     } finally {
+      if (skipFinally) { return; }
       let buildsSkipped = filterMap(
         this._nodeWrappers.values(),
         (nw: NodeWrappers) => nw.buildState.built === false
       ).length;
       logger.debug(`Total nodes skipped: ${buildsSkipped} out of ${this._nodeWrappers.size}`);
 
-      this._cancelationRequest = null;
+      this._cancelationRequest = undefined;
     }
   }
 
@@ -186,6 +209,14 @@ class Builder extends EventEmitter {
     }
   }
 
+  async retry(retryIn: number) {
+    if (this._cancelationRequest) {
+      return this._cancelationRequest.cancel(new RetryCancellationRequest('Retry', retryIn));
+    } else {
+      return this.build();
+    }
+  }
+
   // Destructor-like method. Waits on current node to finish building, then cleans up temp directories
   async cleanup() {
     try {

diff --git a/lib/cancelation-request.ts b/lib/cancelation-request.ts
@@ -1,8 +1,9 @@
 import CancelationError from './errors/cancelation';
 
 export default class CancelationRequest {
-  _pendingWork: Promise<void>;
-  _canceling: Promise<void> | null;
+  private _pendingWork: Promise<void>;
+  private _canceling: Promise<void> | null;
+  private _cancelationError = new CancelationError('Build Canceled');
 
   constructor(pendingWork: Promise<void>) {
     this._pendingWork = pendingWork; // all
@@ -15,19 +16,23 @@ export default class CancelationRequest {
 
   throwIfRequested() {
     if (this.isCanceled) {
-      throw new CancelationError('Build Canceled');
+      throw this._cancelationError;
     }
   }
 
   then() {
     return this._pendingWork.then(...arguments);
   }
 
-  cancel() {
+  cancel(cancelationError?: CancelationError) {
     if (this._canceling) {
       return this._canceling;
     }
 
+    if (cancelationError) {
+      this._cancelationError = cancelationError;
+    }
+
     this._canceling = this._pendingWork.catch(e => {
       if (CancelationError.isCancelationError(e)) {
         return;

diff --git a/lib/errors/retry-cancelation.ts b/lib/errors/retry-cancelation.ts
@@ -0,0 +1,14 @@
+import Cancelation from './cancelation';
+export default class Retry extends Cancelation {
+  isRetry: boolean = true;
+  retryIn: Number;
+
+  static isRetry(e: any): boolean {
+    return typeof e === 'object' && e !== null && e.isRetry === true;
+  }
+
+  constructor(message = 'Retry', retryIn: number) {
+    super(message);
+    this.retryIn = retryIn;
+  }
+}
diff --git a/lib/middleware.ts b/lib/middleware.ts
@@ -141,8 +141,8 @@ export = function getMiddleware(watcher: Watcher, options: MiddlewareOptions = {
   const outputPath = path.resolve(watcher.builder.outputPath);
 
   return async function broccoliMiddleware(request: any, response: any, next: any) {
-    if (watcher.currentBuild == null) {
-      throw new Error('Waiting for initial build to start');
+    if (!watcher.currentBuild) {
+      throw new Error('Broccoli: watcher must have a currentBuild');
     }
 
     try {

diff --git a/lib/server.ts b/lib/server.ts
@@ -86,8 +86,12 @@ class Server extends EventEmitter {
       this.instance.listen(this._port, this._host);
 
       this.instance.on('listening', () => {
-        this.ui.writeLine(`Serving on ${this._url}\n`);
-        resolve(this._watcher.start());
+        try {
+          this.ui.writeLine(`Serving on ${this._url}\n`);
+          this._watcher.start().then(resolve, reject);
+        } catch(e) {
+          reject(e);
+        }
       });
 
       this.instance.on('error', (error: any) => {