Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Jan 13, 2025
5 changes: 3 additions & 2 deletions packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
}

Expand All @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
}
});
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)}`);
}
},
};
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk/lib/api/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
type PublishAssetsOptions,
PublishingAws,
} from '../util/asset-publishing';
import { formatErrorMessage } from '../util/error';

const BOOTSTRAP_STACK_VERSION_FOR_ROLLBACK = 23;

Expand Down Expand Up @@ -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();
}
Expand Down Expand Up @@ -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)}`);
}
}

Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/environment-access.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/environment-resources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/hotswap-deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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}.` : '',
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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;
}

Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/nested-stack-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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;
}
}
Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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`,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)');
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/context-providers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)}`);
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/init-hooks.ts
Original file line number Diff line number Diff line change
@@ -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>;

Expand Down Expand Up @@ -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)}`);
}
};
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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));
}
}

Expand Down
7 changes: 4 additions & 3 deletions packages/aws-cdk/lib/notices.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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)}`));
}
});
}
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/util/archive.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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;
}
Expand Down
19 changes: 19 additions & 0 deletions packages/aws-cdk/lib/util/error.ts
Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose do add a test for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just did see packages/aws-cdk/test/api/util/error.test.ts

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';
}
Loading
Loading