fix(core): use correct formatting for aggregate errors in aws-cdk (#…

…32817) Closes #32237 ### Reason for this change Sometimes when we print `e.message`, `e` is an `AggegateError` - so the message text is incomplete/not formatted correctly. This PR adds a new util function, `formatErrorMessage` which returns `e.message` if it exists, or a correctly formatted string of errors if `e` is an `AggregateError`. ### Description of changes See `formatErrorMessage` function in the newly created file, `packages/aws-cdk/lib/util/error.ts`. All other changes are grunt work replacing `e.message` with `formateErrorMessage(e)`. This PR only does the finding and replacing in the `aws-cdk` package, TBD whether we need to do the same for the rest of the repo. ### Describe any new or updated permissions being added None ### Description of how you validated changes See unit tests in `packages/aws-cdk/test/api/util/error.test.ts` ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --------- Signed-off-by: Sumu <[email protected]>
aws · Jan 13, 2025 · 97af31b · 97af31b
1 parent a5bd76e
commit 97af31b
Show file tree

Hide file tree

Showing 21 changed files with 91 additions and 27 deletions.
diff --git a/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts b/packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts
@@ -4,6 +4,7 @@ import type { AwsCredentialIdentity, AwsCredentialIdentityProvider } from '@smit
 import { credentialsAboutToExpire, makeCachingProvider } from './provider-caching';
 import { debug, warning } from '../../logging';
 import { AuthenticationError } from '../../toolkit/error';
+import { formatErrorMessage } from '../../util/error';
 import { Mode } from '../plugin/mode';
 import { PluginHost } from '../plugin/plugin';
 
@@ -48,7 +49,7 @@ export class CredentialPlugins {
         available = await source.isAvailable();
       } catch (e: any) {
         // This shouldn't happen, but let's guard against it anyway
-        warning(`Uncaught exception in ${source.name}: ${e.message}`);
+        warning(`Uncaught exception in ${source.name}: ${formatErrorMessage(e)}`);
         available = false;
       }
 
@@ -62,7 +63,7 @@ export class CredentialPlugins {
         canProvide = await source.canProvideCredentials(awsAccountId);
       } catch (e: any) {
         // This shouldn't happen, but let's guard against it anyway
-        warning(`Uncaught exception in ${source.name}: ${e.message}`);
+        warning(`Uncaught exception in ${source.name}: ${formatErrorMessage(e)}`);
         canProvide = false;
       }
       if (!canProvide) {

diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts b/packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
@@ -12,6 +12,7 @@ import { makeCachingProvider } from './provider-caching';
 import { SDK } from './sdk';
 import { debug, warning } from '../../logging';
 import { AuthenticationError } from '../../toolkit/error';
+import { formatErrorMessage } from '../../util/error';
 import { traceMethods } from '../../util/tracing';
 import { Mode } from '../plugin/mode';
 
@@ -281,7 +282,7 @@ export class SdkProvider {
           return undefined;
         }
 
-        debug(`Unable to determine the default AWS account (${e.name}): ${e.message}`);
+        debug(`Unable to determine the default AWS account (${e.name}): ${formatErrorMessage(e)}`);
         return undefined;
       }
     });

diff --git a/packages/aws-cdk/lib/api/aws-auth/sdk.ts b/packages/aws-cdk/lib/api/aws-auth/sdk.ts
@@ -320,6 +320,7 @@ import { Account } from './sdk-provider';
 import { defaultCliUserAgent } from './user-agent';
 import { debug } from '../../logging';
 import { AuthenticationError } from '../../toolkit/error';
+import { formatErrorMessage } from '../../util/error';
 import { traceMethods } from '../../util/tracing';
 
 export interface S3ClientOptions {
@@ -903,7 +904,7 @@ export class SDK {
 
           return upload.done();
         } catch (e: any) {
-          throw new AuthenticationError(`Upload failed: ${e.message}`);
+          throw new AuthenticationError(`Upload failed: ${formatErrorMessage(e)}`);
         }
       },
     };

diff --git a/packages/aws-cdk/lib/api/deploy-stack.ts b/packages/aws-cdk/lib/api/deploy-stack.ts
@@ -33,6 +33,7 @@ import { AssetManifestBuilder } from '../util/asset-manifest-builder';
 import { determineAllowCrossAccountAssetPublishing } from './util/checks';
 import { publishAssets } from '../util/asset-publishing';
 import { StringWithoutPlaceholders } from './util/placeholders';
+import { formatErrorMessage } from '../util/error';
 
 export type DeployStackResult =
   | SuccessfulDeployStackResult
@@ -388,7 +389,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
       }
       print(
         'Could not perform a hotswap deployment, because the CloudFormation template could not be resolved: %s',
-        e.message,
+        formatErrorMessage(e),
       );
     }
 
@@ -672,7 +673,7 @@ class FullCloudFormationDeployment {
       }
       finalState = successStack;
     } catch (e: any) {
-      throw new Error(suffixWithErrors(e.message, monitor?.errors));
+      throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
     } finally {
       await monitor?.stop();
     }
@@ -750,7 +751,7 @@ export async function destroyStack(options: DestroyStackOptions) {
       throw new Error(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
     }
   } catch (e: any) {
-    throw new Error(suffixWithErrors(e.message, monitor?.errors));
+    throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
   } finally {
     if (monitor) {
       await monitor.stop();

diff --git a/packages/aws-cdk/lib/api/deployments.ts b/packages/aws-cdk/lib/api/deployments.ts
@@ -38,6 +38,7 @@ import {
   type PublishAssetsOptions,
   PublishingAws,
 } from '../util/asset-publishing';
+import { formatErrorMessage } from '../util/error';
 
 const BOOTSTRAP_STACK_VERSION_FOR_ROLLBACK = 23;
 
@@ -588,7 +589,7 @@ export class Deployments {
           stackErrorMessage = errors;
         }
       } catch (e: any) {
-        stackErrorMessage = suffixWithErrors(e.message, monitor?.errors);
+        stackErrorMessage = suffixWithErrors(formatErrorMessage(e), monitor?.errors);
       } finally {
         await monitor?.stop();
       }
@@ -756,7 +757,7 @@ export class Deployments {
     try {
       await envResources.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
     } catch (e: any) {
-      throw new Error(`${stackName}: ${e.message}`);
+      throw new Error(`${stackName}: ${formatErrorMessage(e)}`);
     }
   }
 

diff --git a/packages/aws-cdk/lib/api/environment-access.ts b/packages/aws-cdk/lib/api/environment-access.ts
@@ -5,6 +5,7 @@ import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/s
 import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
 import { Mode } from './plugin/mode';
 import { replaceEnvPlaceholders, StringWithoutPlaceholders } from './util/placeholders';
+import { formatErrorMessage } from '../util/error';
 
 /**
  * Access particular AWS resources, based on information from the CX manifest
@@ -130,7 +131,7 @@ export class EnvironmentAccess {
     try {
       return await this.accessStackForLookup(stack);
     } catch (e: any) {
-      warning(`${e.message}`);
+      warning(`${formatErrorMessage(e)}`);
     }
     return this.accessStackForStackOperations(stack, Mode.ForReading);
   }

diff --git a/packages/aws-cdk/lib/api/environment-resources.ts b/packages/aws-cdk/lib/api/environment-resources.ts
@@ -3,6 +3,7 @@ import type { SDK } from './aws-auth';
 import { type EcrRepositoryInfo, ToolkitInfo } from './toolkit-info';
 import { debug, warning } from '../logging';
 import { Notices } from '../notices';
+import { formatErrorMessage } from '../util/error';
 
 /**
  * Registry class for `EnvironmentResources`.
@@ -94,7 +95,7 @@ export class EnvironmentResources {
         const bootstrapStack = await this.lookupToolkit();
         if (bootstrapStack.found && bootstrapStack.version < BOOTSTRAP_TEMPLATE_VERSION_INTRODUCING_GETPARAMETER) {
           warning(
-            `Could not read SSM parameter ${ssmParameterName}: ${e.message}, falling back to version from ${bootstrapStack}`,
+            `Could not read SSM parameter ${ssmParameterName}: ${formatErrorMessage(e)}, falling back to version from ${bootstrapStack}`,
           );
           doValidate(bootstrapStack.version, this.environment);
           return;

diff --git a/packages/aws-cdk/lib/api/hotswap-deployments.ts b/packages/aws-cdk/lib/api/hotswap-deployments.ts
@@ -29,6 +29,7 @@ import { isHotswappableStateMachineChange } from './hotswap/stepfunctions-state-
 import { NestedStackTemplates, loadCurrentTemplateWithNestedStacks } from './nested-stack-helpers';
 import { Mode } from './plugin/mode';
 import { CloudFormationStack } from './util/cloudformation';
+import { formatErrorMessage } from '../util/error';
 
 // Must use a require() otherwise esbuild complains about calling a namespace
 // eslint-disable-next-line @typescript-eslint/no-require-imports
@@ -423,7 +424,7 @@ async function applyHotswappableChange(sdk: SDK, hotswapOperation: HotswappableC
     await hotswapOperation.apply(sdk);
   } catch (e: any) {
     if (e.name === 'TimeoutError' || e.name === 'AbortError') {
-      const result: WaiterResult = JSON.parse(e.message);
+      const result: WaiterResult = JSON.parse(formatErrorMessage(e));
       const error = new Error([
         `Resource is not in the expected state due to waiter status: ${result.state}`,
         result.reason ? `${result.reason}.` : '',

diff --git a/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts b/packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts
@@ -1,6 +1,7 @@
 import type { CloudFormationStackArtifact, Environment } from '@aws-cdk/cx-api';
 import type { StackResourceSummary } from '@aws-sdk/client-cloudformation';
 import { debug } from '../../logging';
+import { formatErrorMessage } from '../../util/error';
 import type { SDK, SdkProvider } from '../aws-auth';
 import { EnvironmentAccess } from '../environment-access';
 import { EvaluateCloudFormationTemplate, LazyListStackResources } from '../evaluate-cloudformation-template';
@@ -44,7 +45,7 @@ export async function findCloudWatchLogGroups(
   try {
     sdk = (await new EnvironmentAccess(sdkProvider, DEFAULT_TOOLKIT_STACK_NAME).accessStackForLookup(stackArtifact)).sdk;
   } catch (e: any) {
-    debug(`Failed to access SDK environment: ${e.message}`);
+    debug(`Failed to access SDK environment: ${formatErrorMessage(e)}`);
     sdk = (await sdkProvider.forEnvironment(resolvedEnv, Mode.ForReading)).sdk;
   }
 

diff --git a/packages/aws-cdk/lib/api/nested-stack-helpers.ts b/packages/aws-cdk/lib/api/nested-stack-helpers.ts
@@ -4,6 +4,7 @@ import * as fs from 'fs-extra';
 import type { SDK } from './aws-auth';
 import { LazyListStackResources, type ListStackResources } from './evaluate-cloudformation-template';
 import { CloudFormationStack, type Template } from './util/cloudformation';
+import { formatErrorMessage } from '../util/error';
 
 export interface NestedStackTemplates {
   readonly physicalName: string | undefined;
@@ -129,7 +130,7 @@ async function getNestedStackArn(
     const stackResources = await listStackResources?.listStackResources();
     return stackResources?.find((sr) => sr.LogicalResourceId === nestedStackLogicalId)?.PhysicalResourceId;
   } catch (e: any) {
-    if (e.message.startsWith('Stack with id ') && e.message.endsWith(' does not exist')) {
+    if (formatErrorMessage(e).startsWith('Stack with id ') && formatErrorMessage(e).endsWith(' does not exist')) {
       return;
     }
     throw e;

diff --git a/packages/aws-cdk/lib/api/util/cloudformation.ts b/packages/aws-cdk/lib/api/util/cloudformation.ts
@@ -15,6 +15,7 @@ import { makeBodyParameter, TemplateBodyParameter } from './template-body-parame
 import { debug } from '../../logging';
 import { deserializeStructure } from '../../serialize';
 import { AssetManifestBuilder } from '../../util/asset-manifest-builder';
+import { formatErrorMessage } from '../../util/error';
 import type { ICloudFormationClient, SdkProvider } from '../aws-auth';
 import type { Deployments } from '../deployments';
 
@@ -50,7 +51,7 @@ export class CloudFormationStack {
       const response = await cfn.describeStacks({ StackName: stackName });
       return new CloudFormationStack(cfn, stackName, response.Stacks && response.Stacks[0], retrieveProcessedTemplate);
     } catch (e: any) {
-      if (e.name === 'ValidationError' && e.message === `Stack with id ${stackName} does not exist`) {
+      if (e.name === 'ValidationError' && formatErrorMessage(e) === `Stack with id ${stackName} does not exist`) {
         return new CloudFormationStack(cfn, stackName, undefined);
       }
       throw e;

diff --git a/packages/aws-cdk/lib/api/util/cloudformation/stack-event-poller.ts b/packages/aws-cdk/lib/api/util/cloudformation/stack-event-poller.ts
@@ -1,4 +1,5 @@
 import type { StackEvent } from '@aws-sdk/client-cloudformation';
+import { formatErrorMessage } from '../../../util/error';
 import type { ICloudFormationClient } from '../../aws-auth';
 
 export interface StackEventPollerProps {
@@ -141,7 +142,7 @@ export class StackEventPoller {
 
       }
     } catch (e: any) {
-      if (!(e.name === 'ValidationError' && e.message === `Stack [${this.props.stackName}] does not exist`)) {
+      if (!(e.name === 'ValidationError' && formatErrorMessage(e) === `Stack [${this.props.stackName}] does not exist`)) {
         throw e;
       }
     }

diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -50,6 +50,7 @@ import { deserializeStructure, serializeStructure } from './serialize';
 import { Configuration, PROJECT_CONFIG } from './settings';
 import { ToolkitError } from './toolkit/error';
 import { numberFromBool, partition } from './util';
+import { formatErrorMessage } from './util/error';
 import { validateSnsTopicArn } from './util/validate-notification-arn';
 import { Concurrency, WorkGraph } from './util/work-graph';
 import { WorkGraphBuilder } from './util/work-graph-builder';
@@ -201,7 +202,7 @@ export class CdkToolkit {
               tryLookupRole: true,
             });
           } catch (e: any) {
-            debug(e.message);
+            debug(formatErrorMessage(e));
             if (!quiet) {
               stream.write(
                 `Checking if the stack ${stack.stackName} exists before creating the changeset has failed, will base the diff on template differences (run again with -v to see the reason)\n`,
@@ -511,7 +512,7 @@ export class CdkToolkit {
         // It has to be exactly this string because an integration test tests for
         // "bold(stackname) failed: ResourceNotReady: <error>"
         throw new ToolkitError(
-          [`❌  ${chalk.bold(stack.stackName)} failed:`, ...(e.name ? [`${e.name}:`] : []), e.message].join(' '),
+          [`❌  ${chalk.bold(stack.stackName)} failed:`, ...(e.name ? [`${e.name}:`] : []), formatErrorMessage(e)].join(' '),
         );
       } finally {
         if (options.cloudWatchLogMonitor) {
@@ -603,7 +604,7 @@ export class CdkToolkit {
         const elapsedRollbackTime = new Date().getTime() - startRollbackTime;
         print('\n✨  Rollback time: %ss\n', formatTime(elapsedRollbackTime));
       } catch (e: any) {
-        error('\n ❌  %s failed: %s', chalk.bold(stack.displayName), e.message);
+        error('\n ❌  %s failed: %s', chalk.bold(stack.displayName), formatErrorMessage(e));
         throw new ToolkitError('Rollback failed (use --force to orphan failing resources)');
       }
     }

diff --git a/packages/aws-cdk/lib/context-providers/index.ts b/packages/aws-cdk/lib/context-providers/index.ts
@@ -15,6 +15,7 @@ import { ContextProviderPlugin } from '../api/plugin/context-provider-plugin';
 import { replaceEnvPlaceholders } from '../api/util/placeholders';
 import { debug } from '../logging';
 import { Context, TRANSIENT_CONTEXT_KEY } from '../settings';
+import { formatErrorMessage } from '../util/error';
 
 export type ContextProviderFactory = ((sdk: SdkProvider) => ContextProviderPlugin);
 export type ProviderMap = {[name: string]: ContextProviderFactory};
@@ -72,7 +73,7 @@ export async function provideContextValues(
     } catch (e: any) {
       // Set a specially formatted provider value which will be interpreted
       // as a lookup failure in the toolkit.
-      value = { [cxapi.PROVIDER_ERROR_KEY]: e.message, [TRANSIENT_CONTEXT_KEY]: true };
+      value = { [cxapi.PROVIDER_ERROR_KEY]: formatErrorMessage(e), [TRANSIENT_CONTEXT_KEY]: true };
     }
     context.set(key, value);
     debug(`Setting "${key}" context to ${JSON.stringify(value)}`);

diff --git a/packages/aws-cdk/lib/init-hooks.ts b/packages/aws-cdk/lib/init-hooks.ts
@@ -1,6 +1,7 @@
 import * as path from 'path';
 import { shell } from './os';
 import { ToolkitError } from './toolkit/error';
+import { formatErrorMessage } from './util/error';
 
 export type SubstitutePlaceholders = (...fileNames: string[]) => Promise<void>;
 
@@ -86,6 +87,6 @@ async function dotnetAddProject(targetDirectory: string, context: HookContext, e
   try {
     await shell(['dotnet', 'sln', slnPath, 'add', csprojPath]);
   } catch (e: any) {
-    throw new ToolkitError(`Could not add project ${pname}.${ext} to solution ${pname}.sln. ${e.message}`);
+    throw new ToolkitError(`Could not add project ${pname}.${ext} to solution ${pname}.sln. ${formatErrorMessage(e)}`);
   }
 };
diff --git a/packages/aws-cdk/lib/init.ts b/packages/aws-cdk/lib/init.ts
@@ -6,6 +6,7 @@ import { invokeBuiltinHooks } from './init-hooks';
 import { error, print, warning } from './logging';
 import { ToolkitError } from './toolkit/error';
 import { cdkHomeDir, rootDir } from './util/directories';
+import { formatErrorMessage } from './util/error';
 import { rangeFromSemver } from './util/version-range';
 
 /* eslint-disable @typescript-eslint/no-var-requires */ // Packages don't have @types module
@@ -388,7 +389,7 @@ async function postInstallTypescript(canUseNetwork: boolean, cwd: string) {
   try {
     await execute(command, ['install'], { cwd });
   } catch (e: any) {
-    warning(`${command} install failed: ` + e.message);
+    warning(`${command} install failed: ` + formatErrorMessage(e));
   }
 }
 

diff --git a/packages/aws-cdk/lib/notices.ts b/packages/aws-cdk/lib/notices.ts
@@ -13,6 +13,7 @@ import { ToolkitError } from './toolkit/error';
 import { loadTreeFromDir, some } from './tree';
 import { flatMap } from './util';
 import { cdkCacheDir } from './util/directories';
+import { formatErrorMessage } from './util/error';
 import { versionNumber } from './version';
 
 const CACHE_FILE_PATH = path.join(cdkCacheDir(), 'notices.json');
@@ -429,19 +430,19 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
                   debug('Notices refreshed');
                   resolve(data ?? []);
                 } catch (e: any) {
-                  reject(new ToolkitError(`Failed to parse notices: ${e.message}`));
+                  reject(new ToolkitError(`Failed to parse notices: ${formatErrorMessage(e)}`));
                 }
               });
               res.on('error', e => {
-                reject(new ToolkitError(`Failed to fetch notices: ${e.message}`));
+                reject(new ToolkitError(`Failed to fetch notices: ${formatErrorMessage(e)}`));
               });
             } else {
               reject(new ToolkitError(`Failed to fetch notices. Status code: ${res.statusCode}`));
             }
           });
         req.on('error', reject);
       } catch (e: any) {
-        reject(new ToolkitError(`HTTPS 'get' call threw an error: ${e.message}`));
+        reject(new ToolkitError(`HTTPS 'get' call threw an error: ${formatErrorMessage(e)}`));
       }
     });
   }

diff --git a/packages/aws-cdk/lib/util/archive.ts b/packages/aws-cdk/lib/util/archive.ts
@@ -2,6 +2,7 @@ import { error } from 'console';
 import { createWriteStream, promises as fs } from 'fs';
 import * as path from 'path';
 import * as glob from 'glob';
+import { formatErrorMessage } from './error';
 
 // eslint-disable-next-line @typescript-eslint/no-require-imports
 const archiver = require('archiver');
@@ -76,7 +77,7 @@ async function moveIntoPlace(source: string, target: string) {
       if (e.code !== 'EPERM' || attempts-- <= 0) {
         throw e;
       }
-      error(e.message);
+      error(formatErrorMessage(e));
       await sleep(Math.floor(Math.random() * delay));
       delay *= 2;
     }

diff --git a/packages/aws-cdk/lib/util/error.ts b/packages/aws-cdk/lib/util/error.ts
@@ -0,0 +1,19 @@
+/**
+ * Takes in an error and returns a correctly formatted string of its error message.
+ * If it is an AggregateError, it will return a string with all the inner errors
+ * formatted and separated by a newline.
+ *
+ * @param error The error to format
+ * @returns A string with the error message(s) of the error
+ */
+export function formatErrorMessage(error: any): string {
+  if (error && Array.isArray(error.errors)) {
+    const innerMessages = error.errors
+      .map((innerError: { message: any; toString: () => any }) => (innerError?.message || innerError?.toString()))
+      .join('\n');
+    return `AggregateError: ${innerMessages}`;
+  }
+
+  // Fallback for regular Error or other types
+  return error?.message || error?.toString() || 'Unknown error';
+}