fix(aws-cdk): continue after exceptions in stack monitor (#791)

rix0rrr · web-flow · commit 88b599d81fcd · 2018-09-28T15:52:27.000+02:00
If exceptions occur in the stack monitor, a new invocation wouldn't be scheduled. We're now catching the exception and continuing. This will not fix #787, but it will reduce the frequency and impact of it occurring. Also fixes a race condition where the stack monitor might print more lines after it has already been stopped and the CDK final output has already been printed.
diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -68,7 +68,7 @@ export async function deployStack(stack: cxapi.SynthesizedStack,
   const monitor = quiet ? undefined : new StackActivityMonitor(cfn, deployName, stack.metadata, changeSetDescription.Changes.length).start();
   debug('Execution of changeset %s on stack %s has started; waiting for the update to complete...', changeSetName, deployName);
   await waitForStack(cfn, deployName);
-  if (monitor) { monitor.stop(); }
+  if (monitor) { await monitor.stop(); }
   debug('Stack %s has completed updating', deployName);
   return { noOp: false, outputs: await getStackOutputs(cfn, deployName), stackArn: changeSet.StackId! };
 }
@@ -127,7 +127,7 @@ export async function destroyStack(stack: cxapi.StackInfo, sdk: SDK, deployName?
   const monitor = quiet ? undefined : new StackActivityMonitor(cfn, deployName).start();
   await cfn.deleteStack({ StackName: deployName }).promise().catch(e => { throw e; });
   const destroyedStack = await waitForStack(cfn, deployName, false);
-  if (monitor) { monitor.stop(); }
+  if (monitor) { await monitor.stop(); }
   if (destroyedStack && destroyedStack.StackStatus !== 'DELETE_COMPLETE') {
     const status = StackStatus.fromStackDescription(destroyedStack);
     throw new Error(`Failed to destroy ${deployName}: ${status}`);
diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-activity-monitor.ts
@@ -2,6 +2,7 @@ import cxapi = require('@aws-cdk/cx-api');
 import aws = require('aws-sdk');
 import colors = require('colors/safe');
 import util = require('util');
+import { error } from '../../../logging';
 
 interface StackActivity {
   readonly event: aws.CloudFormation.StackEvent;
@@ -59,6 +60,11 @@ export class StackActivityMonitor {
    */
   private lastPrintTime = Date.now();
 
+  /**
+   * Set to the activity of reading the current events
+   */
+  private readPromise?: Promise<AWS.CloudFormation.StackEvent[]>;
+
   constructor(private readonly cfn: aws.CloudFormation,
               private readonly stackName: string,
               private readonly metadata?: cxapi.StackMetadata,
@@ -80,11 +86,17 @@ export class StackActivityMonitor {
     return this;
   }
 
-  public stop() {
+  public async stop() {
     this.active = false;
     if (this.tickTimer) {
       clearTimeout(this.tickTimer);
     }
+
+    if (this.readPromise) {
+      // We're currently reading events, wait for it to finish and print them before continuing.
+      await this.readPromise;
+      this.flushEvents();
+    }
   }
 
   private scheduleNextTick() {
@@ -99,8 +111,18 @@ export class StackActivityMonitor {
       return;
     }
 
-    await this.readEvents();
-    this.flushEvents();
+    try {
+      this.readPromise = this.readEvents();
+      await this.readPromise;
+      this.readPromise = undefined;
+
+      // We might have been stop()ped while the network call was in progress.
+      if (!this.active) { return; }
+
+      this.flushEvents();
+    } catch (e) {
+      error("Error occurred while monitoring stack: %s", e);
+    }
     this.scheduleNextTick();
   }
 
@@ -220,7 +242,7 @@ export class StackActivityMonitor {
     return colors.reset;
   }
 
-  private async readEvents(nextToken?: string) {
+  private async readEvents(nextToken?: string): Promise<AWS.CloudFormation.StackEvent[]> {
     const output = await this.cfn.describeStackEvents({ StackName: this.stackName, NextToken: nextToken }).promise()
       .catch( e => {
         if (e.code === 'ValidationError' && e.message === `Stack [${this.stackName}] does not exist`) {
