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 { 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 @@
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)}`);

Check warning on line 52 in packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts#L52

Added line #L52 was not covered by tests
available = false;
}

Expand All @@ -62,7 +63,7 @@
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)}`);

Check warning on line 66 in packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/aws-auth/credential-plugins.ts#L66

Added line #L66 was not covered by tests
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 { 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 @@
}
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 @@
}
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 @@
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));

Check warning on line 754 in packages/aws-cdk/lib/api/deploy-stack.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L754 was not covered by tests
} 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 @@
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 @@
stackErrorMessage = errors;
}
} catch (e: any) {
stackErrorMessage = suffixWithErrors(e.message, monitor?.errors);
stackErrorMessage = suffixWithErrors(formatErrorMessage(e), monitor?.errors);

Check warning on line 592 in packages/aws-cdk/lib/api/deployments.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L592 was not covered by tests
} finally {
await monitor?.stop();
}
Expand Down Expand Up @@ -756,7 +757,7 @@
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 { 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 @@
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));

Check warning on line 427 in packages/aws-cdk/lib/api/hotswap-deployments.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/hotswap-deployments.ts#L427

Added line #L427 was not covered by tests
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 @@
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)}`);

Check warning on line 48 in packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/api/logs/find-cloudwatch-logs.ts#L48

Added line #L48 was not covered by tests
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 @@ -51,6 +51,7 @@
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 @@ -202,7 +203,7 @@
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 @@ -512,7 +513,7 @@
// 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 @@ -604,7 +605,7 @@
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));

Check warning on line 608 in packages/aws-cdk/lib/cdk-toolkit.ts

View check run for this annotation

Codecov / codecov/patch

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

Added line #L608 was not covered by tests
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 @@
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)}`);

Check warning on line 90 in packages/aws-cdk/lib/init-hooks.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/init-hooks.ts#L90

Added line #L90 was not covered by tests
}
};
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 { 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 @@
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)}`));

Check warning on line 437 in packages/aws-cdk/lib/notices.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/notices.ts#L437

Added line #L437 was not covered by tests
});
} 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 { 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 @@
if (e.code !== 'EPERM' || attempts-- <= 0) {
throw e;
}
error(e.message);
error(formatErrorMessage(e));

Check warning on line 80 in packages/aws-cdk/lib/util/archive.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/util/archive.ts#L80

Added line #L80 was not covered by tests
await sleep(Math.floor(Math.random() * delay));
delay *= 2;
}
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk/lib/util/error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
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

Check warning on line 3 in packages/aws-cdk/lib/util/error.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/util/error.ts#L3

Added line #L3 was not covered by tests
.map((innerError: { message: any; toString: () => any }) => (innerError?.message || innerError?.toString()))
.join('\n');
return `AggregateError: ${innerMessages}`;

Check warning on line 6 in packages/aws-cdk/lib/util/error.ts

View check run for this annotation

Codecov / codecov/patch

packages/aws-cdk/lib/util/error.ts#L6

Added line #L6 was not covered by tests
}

// Fallback for regular Error or other types
return error?.message || error?.toString() || 'Unknown error';
}
3 changes: 2 additions & 1 deletion packages/aws-cdk/test/api/fake-sts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AssumeRoleCommand, GetCallerIdentityCommand, Tag } from '@aws-sdk/clien
import * as nock from 'nock';
import * as uuid from 'uuid';
import * as xmlJs from 'xml-js';
import { formatErrorMessage } from '../../lib/util/error';
import { mockSTSClient } from '../util/mock-sdk';

interface RegisteredIdentity {
Expand Down Expand Up @@ -81,7 +82,7 @@ export class FakeSts {
Error: {
Type: 'Sender',
Code: e.name ?? 'Error',
Message: e.message,
Message: formatErrorMessage(e),
},
RequestId: '1',
},
Expand Down
Loading