Skip to content

Commit

Permalink
feat(cli): throw typed errors
Browse files Browse the repository at this point in the history
Enforce by enabling the respective eslint rule.
Also adds this rule to the toolkit.
  • Loading branch information
mrgrain committed Jan 18, 2025
1 parent 72e089b commit 6f6aba5
Show file tree
Hide file tree
Showing 42 changed files with 213 additions and 130 deletions.
10 changes: 10 additions & 0 deletions packages/@aws-cdk/toolkit/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,13 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';

// custom rules
baseConfig.rules['@cdklabs/no-throw-default-error'] = ['error'];
baseConfig.overrides.push({
files: ["./test/**"],
rules: {
"@cdklabs/no-throw-default-error": "off",
},
});

module.exports = baseConfig;
4 changes: 2 additions & 2 deletions packages/@aws-cdk/toolkit/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ module.exports = {
coverageThreshold: {
global: {
// this is very sad but we will get better
branches: 27,
statements: 53,
branches: 42,
statements: 69,
},
},
};
6 changes: 6 additions & 0 deletions packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
const assembly = await this.assemblyFromSource(cx);
ioHost;
assembly;
// temporary
// eslint-disable-next-line @cdklabs/no-throw-default-error
throw new Error('Not implemented yet');
}

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

Expand Down Expand Up @@ -619,6 +623,8 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
time: synthTime.asMs,
}));

// temporary
// eslint-disable-next-line @cdklabs/no-throw-default-error
throw new Error('Not implemented yet');
}

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/toolkit/test/_helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import { Toolkit } from '../../lib';
import { Toolkit, ToolkitError } from '../../lib';
import { determineOutputDirectory } from '../../lib/api/cloud-assembly/private';

export * from './test-io-host';
Expand All @@ -12,7 +12,7 @@ function fixturePath(...parts: string[]): string {
export async function appFixture(toolkit: Toolkit, name: string, context?: { [key: string]: any }) {
const appPath = fixturePath(name, 'app.js');
if (!fs.existsSync(appPath)) {
throw new Error(`App Fixture ${name} does not exist in ${appPath}`);
throw new ToolkitError(`App Fixture ${name} does not exist in ${appPath}`);
}
const app = `cat ${appPath} | node --input-type=module`;
return toolkit.fromCdkApp(app, {
Expand All @@ -33,7 +33,7 @@ export function builderFixture(toolkit: Toolkit, name: string, context?: { [key:
export function cdkOutFixture(toolkit: Toolkit, name: string) {
const outdir = path.join(__dirname, '..', '_fixtures', name, 'cdk.out');
if (!fs.existsSync(outdir)) {
throw new Error(`Assembly Dir Fixture ${name} does not exist in ${outdir}`);
throw new ToolkitError(`Assembly Dir Fixture ${name} does not exist in ${outdir}`);
}
return toolkit.fromAssemblyDirectory(outdir);
}
18 changes: 14 additions & 4 deletions packages/aws-cdk/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
const baseConfig = require('@aws-cdk/cdk-build-tools/config/eslintrc');
baseConfig.ignorePatterns.push('lib/init-templates/**/typescript/**/*.ts');
baseConfig.ignorePatterns.push('test/integ/cli/sam_cdk_integ_app/**/*.ts');
baseConfig.parserOptions.project = __dirname + '/tsconfig.json';
const baseConfig = require("@aws-cdk/cdk-build-tools/config/eslintrc");
baseConfig.ignorePatterns.push("lib/init-templates/**/typescript/**/*.ts");
baseConfig.ignorePatterns.push("test/integ/cli/sam_cdk_integ_app/**/*.ts");
baseConfig.parserOptions.project = __dirname + "/tsconfig.json";

// custom rules
baseConfig.rules["@cdklabs/no-throw-default-error"] = ["error"];
baseConfig.overrides.push({
files: ["./test/**"],
rules: {
"@cdklabs/no-throw-default-error": "off",
},
});

module.exports = baseConfig;
15 changes: 8 additions & 7 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 { ToolkitError } from '../toolkit/error';
import { formatErrorMessage } from '../util/error';

export type DeployStackResult =
Expand Down Expand Up @@ -63,7 +64,7 @@ export interface ReplacementRequiresRollbackStackResult {

export function assertIsSuccessfulDeployStackResult(x: DeployStackResult): asserts x is SuccessfulDeployStackResult {
if (x.type !== 'did-deploy-stack') {
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.`);
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.`);
}
}

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

if (deploymentMethod.method === 'direct' && this.options.resourcesToImport) {
throw new Error('Importing resources requires a changeset deployment');
throw new ToolkitError('Importing resources requires a changeset deployment');
}

switch (deploymentMethod.method) {
Expand Down Expand Up @@ -669,11 +670,11 @@ class FullCloudFormationDeployment {

// This shouldn't really happen, but catch it anyway. You never know.
if (!successStack) {
throw new Error('Stack deploy failed (the stack disappeared while we were deploying it)');
throw new ToolkitError('Stack deploy failed (the stack disappeared while we were deploying it)');
}
finalState = successStack;
} catch (e: any) {
throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
throw new ToolkitError(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
} finally {
await monitor?.stop();
}
Expand Down Expand Up @@ -748,10 +749,10 @@ export async function destroyStack(options: DestroyStackOptions) {
await cfn.deleteStack({ StackName: deployName, RoleARN: options.roleArn });
const destroyedStack = await waitForStackDelete(cfn, deployName);
if (destroyedStack && destroyedStack.stackStatus.name !== 'DELETE_COMPLETE') {
throw new Error(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
throw new ToolkitError(`Failed to destroy ${deployName}: ${destroyedStack.stackStatus}`);
}
} catch (e: any) {
throw new Error(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
throw new ToolkitError(suffixWithErrors(formatErrorMessage(e), monitor?.errors));
} finally {
if (monitor) {
await monitor.stop();
Expand Down
19 changes: 10 additions & 9 deletions packages/aws-cdk/lib/api/deployments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
Template,
uploadStackTemplateAssets,
} from './util/cloudformation';
import { ToolkitError } from '../toolkit/error';
import { AssetManifestBuilder } from '../util/asset-manifest-builder';
import {
buildAssets,
Expand Down Expand Up @@ -439,7 +440,7 @@ export class Deployments {
let deploymentMethod = options.deploymentMethod;
if (options.changeSetName || options.execute !== undefined) {
if (deploymentMethod) {
throw new Error(
throw new ToolkitError(
"You cannot supply both 'deploymentMethod' and 'changeSetName/execute'. Supply one or the other.",
);
}
Expand Down Expand Up @@ -492,7 +493,7 @@ export class Deployments {
public async rollbackStack(options: RollbackStackOptions): Promise<RollbackStackResult> {
let resourcesToSkip: string[] = options.orphanLogicalIds ?? [];
if (options.force && resourcesToSkip.length > 0) {
throw new Error('Cannot combine --force with --orphan');
throw new ToolkitError('Cannot combine --force with --orphan');
}

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

default:
throw new Error(`Unexpected rollback choice: ${cloudFormationStack.stackStatus.rollbackChoice}`);
throw new ToolkitError(`Unexpected rollback choice: ${cloudFormationStack.stackStatus.rollbackChoice}`);
}

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

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

Expand All @@ -604,11 +605,11 @@ export class Deployments {
continue;
}

throw new Error(
throw new ToolkitError(
`${stackErrorMessage} (fix problem and retry, or orphan these resources using --orphan or --force)`,
);
}
throw new Error(
throw new ToolkitError(
"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.",
);
}
Expand Down Expand Up @@ -698,7 +699,7 @@ export class Deployments {
const publisher = this.cachedPublisher(assetManifest, resolvedEnvironment, options.stackName);
await publisher.buildEntry(asset);
if (publisher.hasFailures) {
throw new Error(`Failed to build asset ${asset.id}`);
throw new ToolkitError(`Failed to build asset ${asset.id}`);
}
}

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

Expand Down Expand Up @@ -756,7 +757,7 @@ export class Deployments {
try {
await envResources.validateVersion(requiresBootstrapStackVersion, bootstrapStackVersionSsmParameter);
} catch (e: any) {
throw new Error(`${stackName}: ${formatErrorMessage(e)}`);
throw new ToolkitError(`${stackName}: ${formatErrorMessage(e)}`);
}
}

Expand Down
9 changes: 5 additions & 4 deletions 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 { ToolkitError } from '../toolkit/error';
import { formatErrorMessage } from '../util/error';

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

const lookupEnv = await this.prepareSdk({
Expand All @@ -102,7 +103,7 @@ export class EnvironmentAccess {
if (lookupEnv.didAssumeRole && stack.lookupRole?.bootstrapStackVersionSsmParameter && stack.lookupRole.requiresBootstrapStackVersion) {
const version = await lookupEnv.resources.versionFromSsmParameter(stack.lookupRole.bootstrapStackVersionSsmParameter);
if (version < stack.lookupRole.requiresBootstrapStackVersion) {
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}`);
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}`);
}
}
if (lookupEnv.isFallbackCredentials) {
Expand All @@ -125,7 +126,7 @@ export class EnvironmentAccess {
*/
public async accessStackForLookupBestEffort(stack: cxapi.CloudFormationStackArtifact): Promise<TargetEnvironment> {
if (!stack.environment) {
throw new Error(`The stack ${stack.displayName} does not have an environment`);
throw new ToolkitError(`The stack ${stack.displayName} does not have an environment`);
}

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

return this.prepareSdk({
Expand Down
15 changes: 8 additions & 7 deletions 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 { ToolkitError } from '../toolkit/error';
import { formatErrorMessage } from '../util/error';

/**
Expand Down Expand Up @@ -101,7 +102,7 @@ export class EnvironmentResources {
return;
}

throw new Error(
throw new ToolkitError(
`This CDK deployment requires bootstrap stack version '${expectedVersion}', but during the confirmation via SSM parameter ${ssmParameterName} the following error occurred: ${e}`,
);
}
Expand All @@ -119,7 +120,7 @@ export class EnvironmentResources {
notices.addBootstrappedEnvironment({ bootstrapStackVersion: version, environment });
}
if (defExpectedVersion > version) {
throw new Error(
throw new ToolkitError(
`This CDK deployment requires bootstrap stack version '${expectedVersion}', found '${version}'. Please run 'cdk bootstrap'.`,
);
}
Expand All @@ -142,14 +143,14 @@ export class EnvironmentResources {

const asNumber = parseInt(`${result.Parameter?.Value}`, 10);
if (isNaN(asNumber)) {
throw new Error(`SSM parameter ${parameterName} not a number: ${result.Parameter?.Value}`);
throw new ToolkitError(`SSM parameter ${parameterName} not a number: ${result.Parameter?.Value}`);
}

this.cache.ssmParameters.set(parameterName, asNumber);
return asNumber;
} catch (e: any) {
if (e.name === 'ParameterNotFound') {
throw new Error(
throw new ToolkitError(
`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)`,
);
}
Expand All @@ -159,7 +160,7 @@ export class EnvironmentResources {

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

Expand Down Expand Up @@ -188,7 +189,7 @@ export class EnvironmentResources {
});
const repositoryUri = response.repository?.repositoryUri;
if (!repositoryUri) {
throw new Error(`CreateRepository did not return a repository URI for ${repositoryUri}`);
throw new ToolkitError(`CreateRepository did not return a repository URI for ${repositoryUri}`);
}

// configure image scanning on push (helps in identifying software vulnerabilities, no additional charge)
Expand All @@ -211,7 +212,7 @@ export class NoBootstrapStackEnvironmentResources extends EnvironmentResources {
* Look up the toolkit for a given environment, using a given SDK
*/
public async lookupToolkit(): Promise<ToolkitInfo> {
throw new Error(
throw new ToolkitError(
'Trying to perform an operation that requires a bootstrap stack; you should not see this error, this is a bug in the CDK CLI.',
);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk/lib/api/evaluate-cloudformation-template.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { Export, ListExportsCommandOutput, StackResourceSummary } from '@aws-sdk/client-cloudformation';
import type { SDK } from './aws-auth';
import type { NestedStackTemplates } from './nested-stack-helpers';
import { ToolkitError } from '../toolkit/error';

export interface ListStackResources {
listStackResources(): Promise<StackResourceSummary[]>;
Expand Down Expand Up @@ -556,7 +557,7 @@ interface Intrinsic {

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

const ret = new Array<string>();
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 { ToolkitError } from '../toolkit/error';
import { formatErrorMessage } from '../util/error';

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

0 comments on commit 6f6aba5

Please sign in to comment.