fix(cli): cdk deploy hangs when stack deployment fails (#6433)

* fix(cli): `cdk deploy` hangs when stack deployment fails

Previously, when a stack would fail deployment we were using process.exit() which exited as quickly as possible
even if async operations were pending.

We rectified that by setting process.exitCode = 1 to allow all output to be flushed before a graceful exit.
However, since we were not handling deployment errors, the Stack Activity Monitor was still polling every 5 seconds
as the monitor is never explicitly halted.

With this change, when a stack deployment fails, we halt the monitor before throwing the error. This ensures that
there are no open handles when we call process.exitCode = 1

* move stoppage of the monitor into a finally block to clean the code up

* remove unnecessary comment

* added integ test

* oops - added new file

* use '?' instead of 'if'

* added removalPolicy to destroy bucket when orphaned + use a unique name

* verify the deployment actually failed

* use an empty policy to fail deployment

* remove unused s3

Co-authored-by: Eli Polonsky <Eli.polonsky@gmail.com>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

packages/aws-cdk/lib/api/deploy-stack.ts

@@ -151,9 +151,10 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
     // tslint:disable-next-line:max-line-length
     const monitor = options.quiet ? undefined : new StackActivityMonitor(cfn, deployName, options.stack, (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) {
-      await monitor.stop();
+    try {
+      await waitForStack(cfn, deployName);
+    } finally {
+      await monitor?.stop();
     }
     debug('Stack %s has completed updating', deployName);
   } else {

packages/aws-cdk/test/integ/cli/app/app.js

@@ -6,6 +6,7 @@ const iam = require('@aws-cdk/aws-iam');
 const sns = require('@aws-cdk/aws-sns');
 const lambda = require('@aws-cdk/aws-lambda');
 const docker = require('@aws-cdk/aws-ecr-assets');
+const core = require('@aws-cdk/core')
 const { StackWithNestedStack } = require('./nested-stack');
 
 const stackPrefix = process.env.STACK_NAME_PREFIX || 'cdk-toolkit-integration';
@@ -117,6 +118,20 @@ class DockerStackWithCustomFile extends cdk.Stack {
 
 }
 
+class FailedStack extends cdk.Stack {
+
+  constructor(parent, id, props) {
+    super(parent, id, props);
+
+    // fails on 'Property PolicyDocument cannot be empty'.
+    new core.CfnResource(this, 'EmptyPolicy', {
+      type: 'AWS::IAM::Policy'
+    })
+
+  }
+
+}
+
 const VPC_TAG_NAME = 'custom-tag';
 const VPC_TAG_VALUE = `${stackPrefix}-bazinga!`;
 
@@ -169,6 +184,7 @@ new MissingSSMParameterStack(app, `${stackPrefix}-missing-ssm-parameter`, { env:
 new LambdaStack(app, `${stackPrefix}-lambda`);
 new DockerStack(app, `${stackPrefix}-docker`);
 new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`);
+new FailedStack(app, `${stackPrefix}-failed`)
 
 if (process.env.ENABLE_VPC_TESTING) { // Gating so we don't do context fetching unless that's what we are here for
   const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION };

packages/aws-cdk/test/integ/cli/test-cdk-failed-deploy-doesnt-hang.sh

@@ -0,0 +1,22 @@
+#!/bin/bash
+set -euo pipefail
+scriptdir=$(cd $(dirname $0) && pwd)
+source ${scriptdir}/common.bash
+# ----------------------------------------------------------
+
+setup
+
+set +e
+# this will hang if we introduce https://github.com/aws/aws-cdk/issues/6403 again.
+cdk deploy -v ${STACK_NAME_PREFIX}-failed
+deploy_status=$?
+set -e
+
+# destroy
+cdk destroy -f ${STACK_NAME_PREFIX}-failed
+
+if [ "${deploy_status}" -eq 0 ]; then
+  fail "stack deployment should have failed"
+fi
+
+echo "✅  success"