Skip to content

Commit

Permalink
fix(aws-cdk): continue after exceptions in stack monitor (#791)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
rix0rrr authored Sep 28, 2018
1 parent 4980c97 commit 88b599d
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
4 changes: 2 additions & 2 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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! };
}
Expand Down Expand Up @@ -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}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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() {
Expand All @@ -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();
}

Expand Down Expand Up @@ -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`) {
Expand Down

0 comments on commit 88b599d

Please sign in to comment.