Skip to content

Commit bf81b3c

Browse files
authored
feat(cli): throw typed errors (#33005)
### Reason for this change In #32548 we enabled typed errors for some places in the CLI. However many places were missed and the eslint rule wasn't enabled to enforce it in future. ### Description of changes Enforce by enabling the respective eslint rule. Also adds and implements the eslint rule in the toolkit. This has little functional effect since all new errors are still `Error`s. The printed output of an error will slightly change. ### Describe any new or updated permissions being added n/a ### Description of how you validated changes existing tests ### 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*
1 parent 53dc0d8 commit bf81b3c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+218
-135
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,13 @@
11
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
22
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
3+
4+
// custom rules
5+
baseConfig.rules['@cdklabs/no-throw-default-error'] = ['error'];
6+
baseConfig.overrides.push({
7+
files: ["./test/**"],
8+
rules: {
9+
"@cdklabs/no-throw-default-error": "off",
10+
},
11+
});
12+
313
module.exports = baseConfig;

packages/@aws-cdk/toolkit/jest.config.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ module.exports = {
44
coverageThreshold: {
55
global: {
66
// this is very sad but we will get better
7-
branches: 27,
8-
statements: 53,
7+
branches: 42,
8+
statements: 69,
99
},
1010
},
1111
};

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
178178
const assembly = await this.assemblyFromSource(cx);
179179
ioHost;
180180
assembly;
181+
// temporary
182+
// eslint-disable-next-line @cdklabs/no-throw-default-error
181183
throw new Error('Not implemented yet');
182184
}
183185

@@ -189,6 +191,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
189191
const assembly = await this.assemblyFromSource(cx);
190192
const stacks = await assembly.selectStacksV2(options.stacks);
191193
await this.validateStacksMetadata(stacks, ioHost);
194+
// temporary
195+
// eslint-disable-next-line @cdklabs/no-throw-default-error
192196
throw new Error('Not implemented yet');
193197
}
194198

@@ -619,6 +623,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
619623
time: synthTime.asMs,
620624
}));
621625

626+
// temporary
627+
// eslint-disable-next-line @cdklabs/no-throw-default-error
622628
throw new Error('Not implemented yet');
623629
}
624630

packages/@aws-cdk/toolkit/test/_helpers/index.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as fs from 'node:fs';
22
import * as path from 'node:path';
3-
import { Toolkit } from '../../lib';
3+
import { Toolkit, ToolkitError } from '../../lib';
44
import { determineOutputDirectory } from '../../lib/api/cloud-assembly/private';
55

66
export * from './test-io-host';
@@ -12,7 +12,7 @@ function fixturePath(...parts: string[]): string {
1212
export async function appFixture(toolkit: Toolkit, name: string, context?: { [key: string]: any }) {
1313
const appPath = fixturePath(name, 'app.js');
1414
if (!fs.existsSync(appPath)) {
15-
throw new Error(`App Fixture ${name} does not exist in ${appPath}`);
15+
throw new ToolkitError(`App Fixture ${name} does not exist in ${appPath}`);
1616
}
1717
const app = `cat ${appPath} | node --input-type=module`;
1818
return toolkit.fromCdkApp(app, {
@@ -33,7 +33,7 @@ export function builderFixture(toolkit: Toolkit, name: string, context?: { [key:
3333
export function cdkOutFixture(toolkit: Toolkit, name: string) {
3434
const outdir = path.join(__dirname, '..', '_fixtures', name, 'cdk.out');
3535
if (!fs.existsSync(outdir)) {
36-
throw new Error(`Assembly Dir Fixture ${name} does not exist in ${outdir}`);
36+
throw new ToolkitError(`Assembly Dir Fixture ${name} does not exist in ${outdir}`);
3737
}
3838
return toolkit.fromAssemblyDirectory(outdir);
3939
}

packages/aws-cdk/.eslintrc.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
1-
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
2-
baseConfig.ignorePatterns.push('lib/init-templates/**/typescript/**/*.ts');
3-
baseConfig.ignorePatterns.push('test/integ/cli/sam_cdk_integ_app/**/*.ts');
4-
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
1+
const baseConfig = require("@aws-cdk/cdk-build-tools/config/eslintrc");
2+
baseConfig.ignorePatterns.push("lib/init-templates/**/typescript/**/*.ts");
3+
baseConfig.ignorePatterns.push("test/integ/cli/sam_cdk_integ_app/**/*.ts");
4+
baseConfig.parserOptions.project = __dirname + "/tsconfig.json";
5+
6+
// custom rules
7+
baseConfig.rules["@cdklabs/no-throw-default-error"] = ["error"];
8+
baseConfig.overrides.push({
9+
files: ["./test/**"],
10+
rules: {
11+
"@cdklabs/no-throw-default-error": "off",
12+
},
13+
});
14+
515
module.exports = baseConfig;

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import { AssetManifestBuilder } from '../util/asset-manifest-builder';
3333
import { determineAllowCrossAccountAssetPublishing } from './util/checks';
3434
import { publishAssets } from '../util/asset-publishing';
3535
import { StringWithoutPlaceholders } from './util/placeholders';
36+
import { ToolkitError } from '../toolkit/error';
3637
import { formatErrorMessage } from '../util/error';
3738

3839
export type DeployStackResult =
@@ -63,7 +64,7 @@ export interface ReplacementRequiresRollbackStackResult {
6364

6465
export function assertIsSuccessfulDeployStackResult(x: DeployStackResult): asserts x is SuccessfulDeployStackResult {
6566
if (x.type !== 'did-deploy-stack') {
66-
throw new Error(`Unexpected deployStack result. This should not happen: ${JSON.stringify(x)}. If you are seeing this error, please report it at https://github.com/aws/aws-cdk/issues/new/choose.`);
67+
throw new ToolkitError(`Unexpected deployStack result. This should not happen: ${JSON.stringify(x)}. If you are seeing this error, please report it at https://github.com/aws/aws-cdk/issues/new/choose.`);
6768
}
6869
}
6970

@@ -297,7 +298,7 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
297298
await cfn.deleteStack({ StackName: deployName });
298299
const deletedStack = await waitForStackDelete(cfn, deployName);
299300
if (deletedStack && deletedStack.stackStatus.name !== 'DELETE_COMPLETE') {
300-
throw new Error(
301+
throw new ToolkitError(
301302
`Failed deleting stack ${deployName} that had previously failed creation (current state: ${deletedStack.stackStatus})`,
302303
);
303304
}
@@ -455,7 +456,7 @@ class FullCloudFormationDeployment {
455456
};
456457

457458
if (deploymentMethod.method === 'direct' && this.options.resourcesToImport) {
458-
throw new Error('Importing resources requires a changeset deployment');
459+
throw new ToolkitError('Importing resources requires a changeset deployment');
459460
}
460461

461462
switch (deploymentMethod.method) {
@@ -669,11 +670,11 @@ class FullCloudFormationDeployment {
669670

670671
// This shouldn't really happen, but catch it anyway. You never know.
671672
if (!successStack) {
672-
throw new Error('Stack deploy failed (the stack disappeared while we were deploying it)');
673+
throw new ToolkitError('Stack deploy failed (the stack disappeared while we were deploying it)');
673674
}
674675
finalState = successStack;
675676
} catch (e: any) {
676-
throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
677+
throw new ToolkitError(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
677678
} finally {
678679
await monitor?.stop();
679680
}
@@ -748,10 +749,10 @@ export async function destroyStack(options: DestroyStackOptions) {
748749
await cfn.deleteStack({ StackName: deployName, RoleARN: options.roleArn });
749750
const destroyedStack = await waitForStackDelete(cfn, deployName);
750751
if (destroyedStack && destroyedStack.stackStatus.name !== 'DELETE_COMPLETE') {
751-
throw new Error(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
752+
throw new ToolkitError(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
752753
}
753754
} catch (e: any) {
754-
throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
755+
throw new ToolkitError(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
755756
} finally {
756757
if (monitor) {
757758
await monitor.stop();

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
Template,
2626
uploadStackTemplateAssets,
2727
} from './util/cloudformation';
28+
import { ToolkitError } from '../toolkit/error';
2829
import { AssetManifestBuilder } from '../util/asset-manifest-builder';
2930
import {
3031
buildAssets,
@@ -439,7 +440,7 @@ export class Deployments {
439440
let deploymentMethod = options.deploymentMethod;
440441
if (options.changeSetName || options.execute !== undefined) {
441442
if (deploymentMethod) {
442-
throw new Error(
443+
throw new ToolkitError(
443444
"You cannot supply both 'deploymentMethod' and 'changeSetName/execute'. Supply one or the other.",
444445
);
445446
}
@@ -492,7 +493,7 @@ export class Deployments {
492493
public async rollbackStack(options: RollbackStackOptions): Promise<RollbackStackResult> {
493494
let resourcesToSkip: string[] = options.orphanLogicalIds ?? [];
494495
if (options.force && resourcesToSkip.length > 0) {
495-
throw new Error('Cannot combine --force with --orphan');
496+
throw new ToolkitError('Cannot combine --force with --orphan');
496497
}
497498

498499
const env = await this.envs.accessStackForMutableStackOperations(options.stack);
@@ -564,7 +565,7 @@ export class Deployments {
564565
return { notInRollbackableState: true };
565566

566567
default:
567-
throw new Error(`Unexpected rollback choice: ${cloudFormationStack.stackStatus.rollbackChoice}`);
568+
throw new ToolkitError(`Unexpected rollback choice: ${cloudFormationStack.stackStatus.rollbackChoice}`);
568569
}
569570

570571
const monitor = options.quiet
@@ -580,7 +581,7 @@ export class Deployments {
580581

581582
// This shouldn't really happen, but catch it anyway. You never know.
582583
if (!successStack) {
583-
throw new Error('Stack deploy failed (the stack disappeared while we were rolling it back)');
584+
throw new ToolkitError('Stack deploy failed (the stack disappeared while we were rolling it back)');
584585
}
585586
finalStackState = successStack;
586587

@@ -604,11 +605,11 @@ export class Deployments {
604605
continue;
605606
}
606607

607-
throw new Error(
608+
throw new ToolkitError(
608609
`${stackErrorMessage} (fix problem and retry, or orphan these resources using --orphan or --force)`,
609610
);
610611
}
611-
throw new Error(
612+
throw new ToolkitError(
612613
"Rollback did not finish after a large number of iterations; stopping because it looks like we're not making progress anymore. You can retry if rollback was progressing as expected.",
613614
);
614615
}
@@ -698,7 +699,7 @@ export class Deployments {
698699
const publisher = this.cachedPublisher(assetManifest, resolvedEnvironment, options.stackName);
699700
await publisher.buildEntry(asset);
700701
if (publisher.hasFailures) {
701-
throw new Error(`Failed to build asset ${asset.id}`);
702+
throw new ToolkitError(`Failed to build asset ${asset.id}`);
702703
}
703704
}
704705

@@ -717,7 +718,7 @@ export class Deployments {
717718
const publisher = this.cachedPublisher(assetManifest, stackEnv, options.stackName);
718719
await publisher.publishEntry(asset, { allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(options.stack) });
719720
if (publisher.hasFailures) {
720-
throw new Error(`Failed to publish asset ${asset.id}`);
721+
throw new ToolkitError(`Failed to publish asset ${asset.id}`);
721722
}
722723
}
723724

@@ -756,7 +757,7 @@ export class Deployments {
756757
try {
757758
await envResources.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
758759
} catch (e: any) {
759-
throw new Error(`${stackName}: ${formatErrorMessage(e)}`);
760+
throw new ToolkitError(`${stackName}: ${formatErrorMessage(e)}`);
760761
}
761762
}
762763

packages/aws-cdk/lib/api/environment-access.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { CredentialsOptions, SdkForEnvironment, SdkProvider } from './aws-auth/s
55
import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
66
import { Mode } from './plugin/mode';
77
import { replaceEnvPlaceholders, StringWithoutPlaceholders } from './util/placeholders';
8+
import { ToolkitError } from '../toolkit/error';
89
import { formatErrorMessage } from '../util/error';
910

1011
/**
@@ -87,7 +88,7 @@ export class EnvironmentAccess {
8788
*/
8889
public async accessStackForLookup(stack: cxapi.CloudFormationStackArtifact): Promise<TargetEnvironment> {
8990
if (!stack.environment) {
90-
throw new Error(`The stack ${stack.displayName} does not have an environment`);
91+
throw new ToolkitError(`The stack ${stack.displayName} does not have an environment`);
9192
}
9293

9394
const lookupEnv = await this.prepareSdk({
@@ -102,7 +103,7 @@ export class EnvironmentAccess {
102103
if (lookupEnv.didAssumeRole && stack.lookupRole?.bootstrapStackVersionSsmParameter && stack.lookupRole.requiresBootstrapStackVersion) {
103104
const version = await lookupEnv.resources.versionFromSsmParameter(stack.lookupRole.bootstrapStackVersionSsmParameter);
104105
if (version < stack.lookupRole.requiresBootstrapStackVersion) {
105-
throw new Error(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'. To get rid of this error, please upgrade to bootstrap version >= ${stack.lookupRole.requiresBootstrapStackVersion}`);
106+
throw new ToolkitError(`Bootstrap stack version '${stack.lookupRole.requiresBootstrapStackVersion}' is required, found version '${version}'. To get rid of this error, please upgrade to bootstrap version >= ${stack.lookupRole.requiresBootstrapStackVersion}`);
106107
}
107108
}
108109
if (lookupEnv.isFallbackCredentials) {
@@ -125,7 +126,7 @@ export class EnvironmentAccess {
125126
*/
126127
public async accessStackForLookupBestEffort(stack: cxapi.CloudFormationStackArtifact): Promise<TargetEnvironment> {
127128
if (!stack.environment) {
128-
throw new Error(`The stack ${stack.displayName} does not have an environment`);
129+
throw new ToolkitError(`The stack ${stack.displayName} does not have an environment`);
129130
}
130131

131132
try {
@@ -147,7 +148,7 @@ export class EnvironmentAccess {
147148
*/
148149
private async accessStackForStackOperations(stack: cxapi.CloudFormationStackArtifact, mode: Mode): Promise<TargetEnvironment> {
149150
if (!stack.environment) {
150-
throw new Error(`The stack ${stack.displayName} does not have an environment`);
151+
throw new ToolkitError(`The stack ${stack.displayName} does not have an environment`);
151152
}
152153

153154
return this.prepareSdk({

packages/aws-cdk/lib/api/environment-resources.ts

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type { SDK } from './aws-auth';
33
import { type EcrRepositoryInfo, ToolkitInfo } from './toolkit-info';
44
import { debug, warning } from '../logging';
55
import { Notices } from '../notices';
6+
import { ToolkitError } from '../toolkit/error';
67
import { formatErrorMessage } from '../util/error';
78

89
/**
@@ -101,7 +102,7 @@ export class EnvironmentResources {
101102
return;
102103
}
103104

104-
throw new Error(
105+
throw new ToolkitError(
105106
`This CDK deployment requires bootstrap stack version '${expectedVersion}', but during the confirmation via SSM parameter ${ssmParameterName} the following error occurred: ${e}`,
106107
);
107108
}
@@ -119,7 +120,7 @@ export class EnvironmentResources {
119120
notices.addBootstrappedEnvironment({ bootstrapStackVersion: version, environment });
120121
}
121122
if (defExpectedVersion > version) {
122-
throw new Error(
123+
throw new ToolkitError(
123124
`This CDK deployment requires bootstrap stack version '${expectedVersion}', found '${version}'. Please run 'cdk bootstrap'.`,
124125
);
125126
}
@@ -142,14 +143,14 @@ export class EnvironmentResources {
142143

143144
const asNumber = parseInt(`${result.Parameter?.Value}`, 10);
144145
if (isNaN(asNumber)) {
145-
throw new Error(`SSM parameter ${parameterName} not a number: ${result.Parameter?.Value}`);
146+
throw new ToolkitError(`SSM parameter ${parameterName} not a number: ${result.Parameter?.Value}`);
146147
}
147148

148149
this.cache.ssmParameters.set(parameterName, asNumber);
149150
return asNumber;
150151
} catch (e: any) {
151152
if (e.name === 'ParameterNotFound') {
152-
throw new Error(
153+
throw new ToolkitError(
153154
`SSM parameter ${parameterName} not found. Has the environment been bootstrapped? Please run \'cdk bootstrap\' (see https://docs.aws.amazon.com/cdk/latest/guide/bootstrapping.html)`,
154155
);
155156
}
@@ -159,7 +160,7 @@ export class EnvironmentResources {
159160

160161
public async prepareEcrRepository(repositoryName: string): Promise<EcrRepositoryInfo> {
161162
if (!this.sdk) {
162-
throw new Error('ToolkitInfo needs to have been initialized with an sdk to call prepareEcrRepository');
163+
throw new ToolkitError('ToolkitInfo needs to have been initialized with an sdk to call prepareEcrRepository');
163164
}
164165
const ecr = this.sdk.ecr();
165166

@@ -188,7 +189,7 @@ export class EnvironmentResources {
188189
});
189190
const repositoryUri = response.repository?.repositoryUri;
190191
if (!repositoryUri) {
191-
throw new Error(`CreateRepository did not return a repository URI for ${repositoryUri}`);
192+
throw new ToolkitError(`CreateRepository did not return a repository URI for ${repositoryUri}`);
192193
}
193194

194195
// configure image scanning on push (helps in identifying software vulnerabilities, no additional charge)
@@ -211,7 +212,7 @@ export class NoBootstrapStackEnvironmentResources extends EnvironmentResources {
211212
* Look up the toolkit for a given environment, using a given SDK
212213
*/
213214
public async lookupToolkit(): Promise<ToolkitInfo> {
214-
throw new Error(
215+
throw new ToolkitError(
215216
'Trying to perform an operation that requires a bootstrap stack; you should not see this error, this is a bug in the CDK CLI.',
216217
);
217218
}

packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type { Export, ListExportsCommandOutput, StackResourceSummary } from '@aws-sdk/client-cloudformation';
22
import type { SDK } from './aws-auth';
33
import type { NestedStackTemplates } from './nested-stack-helpers';
4+
import { ToolkitError } from '../toolkit/error';
45

56
export interface ListStackResources {
67
listStackResources(): Promise<StackResourceSummary[]>;
@@ -556,7 +557,7 @@ interface Intrinsic {
556557

557558
async function asyncGlobalReplace(str: string, regex: RegExp, cb: (x: string) => Promise<string>): Promise<string> {
558559
if (!regex.global) {
559-
throw new Error('Regex must be created with /g flag');
560+
throw new ToolkitError('Regex must be created with /g flag');
560561
}
561562

562563
const ret = new Array<string>();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { isHotswappableStateMachineChange } from './hotswap/stepfunctions-state-
2929
import { NestedStackTemplates, loadCurrentTemplateWithNestedStacks } from './nested-stack-helpers';
3030
import { Mode } from './plugin/mode';
3131
import { CloudFormationStack } from './util/cloudformation';
32+
import { ToolkitError } from '../toolkit/error';
3233
import { formatErrorMessage } from '../util/error';
3334

3435
// Must use a require() otherwise esbuild complains about calling a namespace
@@ -425,7 +426,7 @@ async function applyHotswappableChange(sdk: SDK, hotswapOperation: HotswappableC
425426
} catch (e: any) {
426427
if (e.name === 'TimeoutError' || e.name === 'AbortError') {
427428
const result: WaiterResult = JSON.parse(formatErrorMessage(e));
428-
const error = new Error([
429+
const error = new ToolkitError([
429430
`Resource is not in the expected state due to waiter status: ${result.state}`,
430431
result.reason ? `${result.reason}.` : '',
431432
].join('. '));

0 commit comments

Comments
 (0)