fix(cli): deployment errors are printed 3 times (#31389)

The CLI prints deployment errors 3 times. This is caused by an catching an error, printing it, and then throwing it again; to another `catch` statement that catches the error, prints it, and then throws it again.

In this PR, get rid of one catch and change the error that gets rethrown in a different case.

Also in this PR: fix the inconsistency of printing the progress of asset publishing. Compared to the progress of stack deployments, the stack name isn't bold and there is a single space offset.

(A little work to change the printing, a LOT of work to get the integration+regression tests to pass, that all assert way too many specifics about the error messages that get printed to the screen)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -13,7 +13,7 @@ import { shell, ShellOptions, ShellHelper, rimraf } from './shell';
 import { AwsContext, withAws } from './with-aws';
 import { withTimeout } from './with-timeout';
 
-export const DEFAULT_TEST_TIMEOUT_S = 10 * 60;
+export const DEFAULT_TEST_TIMEOUT_S = 20 * 60;
 export const EXTENDED_TEST_TIMEOUT_S = 30 * 60;
 
 /**

packages/@aws-cdk-testing/cli-integ/resources/cli-regression-patches/v2.160.0/skip-tests.txt

@@ -0,0 +1,2 @@
+# This test asserts too much about the output of the CLI.
+security related changes without a CLI are expected to fail when approval is required

packages/@aws-cdk-testing/cli-integ/skip-tests.txt

@@ -1,7 +1,7 @@
 # This file is empty on purpose. Leave it here as documentation
 # and an example.
 #
-# Copy this file to cli-regression-patches/vX.Y.Z/skip-tests.txt
+# Copy this file to resources/cli-regression-patches/vX.Y.Z/skip-tests.txt
 # and edit it there if you want to exclude certain tests from running
 # when performing a certain version's regression tests.
 #

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli-lib.integtest.ts

@@ -75,7 +75,7 @@ integTest(
       'This deployment will make potentially sensitive changes according to your current security approval level',
     );
     expect(stdErr).toContain(
-      'Deployment failed: Error: "--require-approval" is enabled and stack includes security-sensitive updates',
+      '"--require-approval" is enabled and stack includes security-sensitive updates',
     );
 
     // Ensure stack was not deployed

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts

@@ -19,7 +19,6 @@ import {
 import { InvokeCommand } from '@aws-sdk/client-lambda';
 import { CreateTopicCommand, DeleteTopicCommand } from '@aws-sdk/client-sns';
 import { AssumeRoleCommand, GetCallerIdentityCommand } from '@aws-sdk/client-sts';
-import * as chalk from 'chalk';
 import {
   integTest,
   cloneDirectory,
@@ -2198,8 +2197,7 @@ integTest(
       allowErrExit: true,
     });
 
-    const stackName = `${fixture.stackNamePrefix}-ecs-hotswap`;
-    const expectedSubstring = `❌  ${chalk.bold(stackName)} failed: ResourceNotReady: Resource is not in the state deploymentCompleted`;
+    const expectedSubstring = 'Resource is not in the state deploymentCompleted';
 
     expect(deployOutput).toContain(expectedSubstring);
     expect(deployOutput).not.toContain('hotswapped!');

packages/aws-cdk/lib/api/deployments.ts

@@ -1,6 +1,7 @@
 import * as cxapi from '@aws-cdk/cx-api';
 import * as cdk_assets from 'cdk-assets';
 import { AssetManifest, IManifestEntry } from 'cdk-assets';
+import * as chalk from 'chalk';
 import { Tag } from '../cdk-toolkit';
 import { debug, warning, error } from '../logging';
 import { Mode } from './aws-auth/credentials';
@@ -700,7 +701,7 @@ export class Deployments {
     if (existing) {
       return existing;
     }
-    const prefix = stackName ? `${stackName}: ` : '';
+    const prefix = stackName ? `${chalk.bold(stackName)}: ` : '';
     const publisher = new cdk_assets.AssetPublishing(assetManifest, {
       aws: new PublishingAws(this.sdkProvider, env),
       progressListener: new ParallelSafeAssetProgress(prefix, this.props.quiet ?? false),
@@ -719,7 +720,7 @@ class ParallelSafeAssetProgress implements cdk_assets.IPublishProgressListener {
 
   public onPublishEvent(type: cdk_assets.EventType, event: cdk_assets.IPublishProgress): void {
     const handler = this.quiet && type !== 'fail' ? debug : EVENT_TO_LOGGER[type];
-    handler(`${this.prefix} ${type}: ${event.message}`);
+    handler(`${this.prefix}${type}: ${event.message}`);
   }
 }
 

packages/aws-cdk/lib/cdk-toolkit.ts

@@ -371,9 +371,14 @@ export class CdkToolkit {
         print('Stack ARN:');
 
         data(result.stackArn);
-      } catch (e) {
-        error('\n ❌  %s failed: %s', chalk.bold(stack.displayName), e);
-        throw e;
+      } catch (e: any) {
+        // It has to be exactly this string because an integration test tests for
+        // "bold(stackname) failed: ResourceNotReady: <error>"
+        throw new Error([
+          `❌  ${chalk.bold(stack.stackName)} failed:`,
+          ...e.code ? [`${e.code}:`] : [],
+          e.message,
+        ].join(' '));
       } finally {
         if (options.cloudWatchLogMonitor) {
           const foundLogGroupsResult = await findCloudWatchLogGroups(this.props.sdkProvider, stack);
@@ -401,33 +406,28 @@ export class CdkToolkit {
       warning('⚠️ The --concurrency flag only supports --progress "events". Switching to "events".');
     }
 
-    try {
-      const stacksAndTheirAssetManifests = stacks.flatMap(stack => [
-        stack,
-        ...stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact),
-      ]);
-      const workGraph = new WorkGraphBuilder(prebuildAssets).build(stacksAndTheirAssetManifests);
-
-      // Unless we are running with '--force', skip already published assets
-      if (!options.force) {
-        await this.removePublishedAssets(workGraph, options);
-      }
+    const stacksAndTheirAssetManifests = stacks.flatMap(stack => [
+      stack,
+      ...stack.dependencies.filter(cxapi.AssetManifestArtifact.isAssetManifestArtifact),
+    ]);
+    const workGraph = new WorkGraphBuilder(prebuildAssets).build(stacksAndTheirAssetManifests);
 
-      const graphConcurrency: Concurrency = {
-        'stack': concurrency,
-        'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds
-        'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable
-      };
-
-      await workGraph.doParallel(graphConcurrency, {
-        deployStack,
-        buildAsset,
-        publishAsset,
-      });
-    } catch (e) {
-      error('\n ❌ Deployment failed: %s', e);
-      throw e;
+    // Unless we are running with '--force', skip already published assets
+    if (!options.force) {
+      await this.removePublishedAssets(workGraph, options);
     }
+
+    const graphConcurrency: Concurrency = {
+      'stack': concurrency,
+      'asset-build': 1, // This will be CPU-bound/memory bound, mostly matters for Docker builds
+      'asset-publish': (options.assetParallelism ?? true) ? 8 : 1, // This will be I/O-bound, 8 in parallel seems reasonable
+    };
+
+    await workGraph.doParallel(graphConcurrency, {
+      deployStack,
+      buildAsset,
+      publishAsset,
+    });
   }
 
   public async watch(options: WatchOptions) {