avoid calling timed destroy too early before app instance created

Author: bekzod

src/ember-app.js

@@ -230,13 +230,13 @@ class EmberApp {
     return this.buildAppInstance()
       .then(instance => {
         result.instance = instance;
+        result._startDestroyTimer();
         registerFastBootInfo(fastbootInfo, instance);
         return instance.boot(bootOptions);
       })
       .then(() => result.instanceBooted = true)
       .then(() => result.instance.visit(path, bootOptions))
-      .then(() => fastbootInfo.deferredPromise)
-      .then(() => result);
+      .then(() => fastbootInfo.deferredPromise);
   }
 
   /**
@@ -271,29 +271,13 @@ class EmberApp {
     let shouldRender = (options.shouldRender !== undefined) ? options.shouldRender : true;
     let bootOptions = buildBootOptions(shouldRender);
     let fastbootInfo = new FastBootInfo(
-      req,
-      res,
+      req, res,
       { hostWhitelist: this.hostWhitelist, metadata: options.metadata }
     );
 
     let doc = bootOptions.document;
 
-    let result = new Result({
-      doc: doc,
-      html: html,
-      fastbootInfo: fastbootInfo
-    });
-
-    let destroyAppInstanceTimer;
-    if (destroyAppInstanceInMs > 0) {
-      // start a timer to destroy the appInstance forcefully in the given ms.
-      // This is a failure mechanism so that node process doesn't get wedged if the `visit` never completes.
-      destroyAppInstanceTimer = setTimeout(function() {
-        if (result._destroyAppInstance()) {
-          result.error = new Error('App instance was forcefully destroyed in ' + destroyAppInstanceInMs + 'ms');
-        }
-      }, destroyAppInstanceInMs);
-    }
+    let result = new Result({ doc, html, fastbootInfo, destroyAppInstanceInMs });
 
     return this.visitRoute(path, fastbootInfo, bootOptions, result)
       .then(() => {
@@ -304,13 +288,7 @@ class EmberApp {
       })
       .catch(error => result.error = error)
       .then(() => result._finalize())
-      .finally(() => {
-        if (result._destroyAppInstance()) {
-          if (destroyAppInstanceTimer) {
-            clearTimeout(destroyAppInstanceTimer);
-          }
-        }
-      });
+      .finally(() => result._destroyAppInstance());
   }
 
   /**

src/result.js

@@ -15,6 +15,8 @@ class Result {
     this._doc = options.doc;
     this._html = options.html;
     this._fastbootInfo = options.fastbootInfo;
+    this._destroyAppInstanceInMs = options.destroyAppInstanceInMs;
+    this._destroyAppInstanceTimer = undefined;
   }
 
   /**
@@ -97,10 +99,23 @@ class Result {
     }
   }
 
+  _startDestroyTimer() {
+    if (this._destroyAppInstanceInMs > 0 && !this._instanceDestroyed) {
+      // start a timer to destroy the appInstance forcefully in the given ms.
+      // This is a failure mechanism so that node process doesn't get wedged if the `visit` never completes.
+      this._destroyAppInstanceTimer = setTimeout(()=> {
+        if (this._destroyAppInstance()) {
+          this.error = new Error('App instance was forcefully destroyed in ' + this._destroyAppInstanceInMs + 'ms');
+        }
+      }, this._destroyAppInstanceInMs);
+    }
+  }
+
   _destroyAppInstance() {
     if (this.instance && !this._instanceDestroyed) {
       this._instanceDestroyed = true;
       this.instance.destroy();
+      clearTimeout(this._destroyAppInstanceTimer);
       return true;
     }
     return false;

test/fastboot-test.js

@@ -103,23 +103,12 @@ describe("FastBoot", function() {
 
   it("can forcefully destroy the app instance using destroyAppInstanceInMs", function(done) {
     var fastboot = new FastBoot({
-      distPath: fixture('basic-app')
+      distPath: fixture('long-resolving-app')
     });
 
-    // delaying `visitRoute` to forcefully destroy app instance
-    let originalVisitRoute = fastboot._app.visitRoute;
-    fastboot._app.visitRoute = function() {
-      return originalVisitRoute.apply(this, arguments)
-        .then(function() {
-          return new Promise(function(resolve) {
-            setTimeout(resolve, 2000);
-          });
-        });
-    };
-
-    fastboot.visit('/', { destroyAppInstanceInMs: 5 })
+    fastboot.visit('/', { destroyAppInstanceInMs: 300 })
       .catch((e) => {
-        expect(e.message).to.equal('App instance was forcefully destroyed in 5ms');
+        expect(e.message).to.equal('App instance was forcefully destroyed in 300ms');
         done();
       });
   });