From ae95d9571c0fb4469e5c91ffc443ff04e965d1db Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 12 Feb 2025 12:00:33 +0000 Subject: [PATCH] feat(cx-api): throw `CloudAssemblyError` instead of untyped Errors (#33390) ### Issue Relates to #32569 ### Description of changes `ValidationErrors` everywhere ### Describe any new or updated permissions being added n/a ### Description of how you validated changes Existing tests. Exemptions granted as this is a refactor of existing code. ### 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* --- packages/aws-cdk-lib/.eslintrc.js | 1 + packages/aws-cdk-lib/core/lib/errors.ts | 11 ++++++ .../lib/artifacts/asset-manifest-artifact.ts | 3 +- .../lib/artifacts/cloudformation-artifact.ts | 5 +-- .../lib/artifacts/tree-cloud-artifact.ts | 3 +- .../aws-cdk-lib/cx-api/lib/cloud-artifact.ts | 5 +-- .../aws-cdk-lib/cx-api/lib/cloud-assembly.ts | 19 +++++----- .../aws-cdk-lib/cx-api/lib/environment.ts | 6 ++-- packages/aws-cdk-lib/cx-api/lib/features.ts | 2 ++ .../aws-cdk-lib/cx-api/lib/private/error.ts | 35 +++++++++++++++++++ packages/aws-cdk-lib/cx-api/lib/toposort.ts | 4 ++- 11 files changed, 76 insertions(+), 18 deletions(-) create mode 100644 packages/aws-cdk-lib/cx-api/lib/private/error.ts diff --git a/packages/aws-cdk-lib/.eslintrc.js b/packages/aws-cdk-lib/.eslintrc.js index 46d893f5a54b2..39f5f2e036779 100644 --- a/packages/aws-cdk-lib/.eslintrc.js +++ b/packages/aws-cdk-lib/.eslintrc.js @@ -60,6 +60,7 @@ const enableNoThrowDefaultErrorIn = [ 'aws-ssmincidents', 'aws-ssmquicksetup', 'aws-synthetics', + 'cx-api', 'aws-s3', 'aws-s3-assets', 'aws-s3-deployment', diff --git a/packages/aws-cdk-lib/core/lib/errors.ts b/packages/aws-cdk-lib/core/lib/errors.ts index 2764c7adc89bb..f7d7c1cd14687 100644 --- a/packages/aws-cdk-lib/core/lib/errors.ts +++ b/packages/aws-cdk-lib/core/lib/errors.ts @@ -1,8 +1,10 @@ import { IConstruct } from 'constructs'; import { constructInfoFromConstruct } from './helpers-internal'; +import type { CloudAssemblyError } from '../../cx-api/lib/private/error'; const CONSTRUCT_ERROR_SYMBOL = Symbol.for('@aws-cdk/core.SynthesisError'); const VALIDATION_ERROR_SYMBOL = Symbol.for('@aws-cdk/core.ValidationError'); +const ASSEMBLY_ERROR_SYMBOL = Symbol.for('@aws-cdk/cx-api.CloudAssemblyError'); /** * Helper to check if an error is of a certain type. @@ -27,6 +29,15 @@ export class Errors { public static isValidationError(x: any): x is ValidationError { return Errors.isConstructError(x) && VALIDATION_ERROR_SYMBOL in x; } + + /** + * Test whether the given error is a CloudAssemblyError. + * + * A CloudAssemblyError is thrown for unexpected problems with the synthesized assembly. + */ + public static isCloudAssemblyError(x: any): x is CloudAssemblyError { + return x !== null && typeof(x) === 'object' && ASSEMBLY_ERROR_SYMBOL in x; + } } interface ConstructInfo { diff --git a/packages/aws-cdk-lib/cx-api/lib/artifacts/asset-manifest-artifact.ts b/packages/aws-cdk-lib/cx-api/lib/artifacts/asset-manifest-artifact.ts index b6f5e9c3eeb1e..f5f80fb599e87 100644 --- a/packages/aws-cdk-lib/cx-api/lib/artifacts/asset-manifest-artifact.ts +++ b/packages/aws-cdk-lib/cx-api/lib/artifacts/asset-manifest-artifact.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import * as cxschema from '../../../cloud-assembly-schema'; import { CloudArtifact } from '../cloud-artifact'; import { CloudAssembly } from '../cloud-assembly'; +import { CloudAssemblyError } from '../private/error'; const ASSET_MANIFEST_ARTIFACT_SYM = Symbol.for('@aws-cdk/cx-api.AssetManifestArtifact'); @@ -55,7 +56,7 @@ export class AssetManifestArtifact extends CloudArtifact { const properties = (this.manifest.properties || {}) as cxschema.AssetManifestProperties; if (!properties.file) { - throw new Error('Invalid AssetManifestArtifact. Missing "file" property'); + throw new CloudAssemblyError('Invalid AssetManifestArtifact. Missing "file" property'); } this.file = path.resolve(this.assembly.directory, properties.file); this.requiresBootstrapStackVersion = properties.requiresBootstrapStackVersion; diff --git a/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts b/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts index 3009129056e93..0939f784ba4a7 100644 --- a/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts +++ b/packages/aws-cdk-lib/cx-api/lib/artifacts/cloudformation-artifact.ts @@ -4,6 +4,7 @@ import * as cxschema from '../../../cloud-assembly-schema'; import { CloudArtifact } from '../cloud-artifact'; import type { CloudAssembly } from '../cloud-assembly'; import { Environment, EnvironmentUtils } from '../environment'; +import { CloudAssemblyError } from '../private/error'; const CLOUDFORMATION_STACK_ARTIFACT_SYM = Symbol.for('@aws-cdk/cx-api.CloudFormationStackArtifact'); export class CloudFormationStackArtifact extends CloudArtifact { @@ -162,10 +163,10 @@ export class CloudFormationStackArtifact extends CloudArtifact { const properties = (this.manifest.properties || {}) as cxschema.AwsCloudFormationStackProperties; if (!properties.templateFile) { - throw new Error('Invalid CloudFormation stack artifact. Missing "templateFile" property in cloud assembly manifest'); + throw new CloudAssemblyError('Invalid CloudFormation stack artifact. Missing "templateFile" property in cloud assembly manifest'); } if (!artifact.environment) { - throw new Error('Invalid CloudFormation stack artifact. Missing environment'); + throw new CloudAssemblyError('Invalid CloudFormation stack artifact. Missing environment'); } this.environment = EnvironmentUtils.parse(artifact.environment); this.templateFile = properties.templateFile; diff --git a/packages/aws-cdk-lib/cx-api/lib/artifacts/tree-cloud-artifact.ts b/packages/aws-cdk-lib/cx-api/lib/artifacts/tree-cloud-artifact.ts index e22b6640c6215..c1ce5d7bb1028 100644 --- a/packages/aws-cdk-lib/cx-api/lib/artifacts/tree-cloud-artifact.ts +++ b/packages/aws-cdk-lib/cx-api/lib/artifacts/tree-cloud-artifact.ts @@ -1,6 +1,7 @@ import * as cxschema from '../../../cloud-assembly-schema'; import { CloudArtifact } from '../cloud-artifact'; import { CloudAssembly } from '../cloud-assembly'; +import { CloudAssemblyError } from '../private/error'; const TREE_CLOUD_ARTIFACT_SYM = Symbol.for('@aws-cdk/cx-api.TreeCloudArtifact'); @@ -33,7 +34,7 @@ export class TreeCloudArtifact extends CloudArtifact { const properties = (this.manifest.properties || {}) as cxschema.TreeArtifactProperties; if (!properties.file) { - throw new Error('Invalid TreeCloudArtifact. Missing "file" property'); + throw new CloudAssemblyError('Invalid TreeCloudArtifact. Missing "file" property'); } this.file = properties.file; } diff --git a/packages/aws-cdk-lib/cx-api/lib/cloud-artifact.ts b/packages/aws-cdk-lib/cx-api/lib/cloud-artifact.ts index 22edeac8b871c..415c2d968787b 100644 --- a/packages/aws-cdk-lib/cx-api/lib/cloud-artifact.ts +++ b/packages/aws-cdk-lib/cx-api/lib/cloud-artifact.ts @@ -1,6 +1,7 @@ import type { CloudAssembly } from './cloud-assembly'; import { MetadataEntryResult, SynthesisMessage, SynthesisMessageLevel } from './metadata'; import * as cxschema from '../../cloud-assembly-schema'; +import { CloudAssemblyError } from './private/error'; /** * Artifact properties for CloudFormation stacks. @@ -45,7 +46,7 @@ export class CloudArtifact { public static fromManifest(assembly: CloudAssembly, id: string, artifact: cxschema.ArtifactManifest): CloudArtifact | undefined { // Implementation is defined in a separate file to break cyclic dependencies void(assembly), void(id), void(artifact); - throw new Error('Implementation not overridden yet'); + throw new CloudAssemblyError('Implementation not overridden yet'); } /** @@ -84,7 +85,7 @@ export class CloudArtifact { this._deps = this._dependencyIDs.map(id => { const dep = this.assembly.tryGetArtifact(id); if (!dep) { - throw new Error(`Artifact ${this.id} depends on non-existing artifact ${id}`); + throw new CloudAssemblyError(`Artifact ${this.id} depends on non-existing artifact ${id}`); } return dep; }); diff --git a/packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts b/packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts index 71600a506bbf9..7989403690f44 100644 --- a/packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts +++ b/packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts @@ -11,6 +11,7 @@ import { TreeCloudArtifact } from './artifacts/tree-cloud-artifact'; import { CloudArtifact } from './cloud-artifact'; import { topologicalSort } from './toposort'; import * as cxschema from '../../cloud-assembly-schema'; +import { CloudAssemblyError } from './private/error'; const CLOUD_ASSEMBLY_SYMBOL = Symbol.for('@aws-cdk/cx-api.CloudAssembly'); @@ -98,12 +99,12 @@ export class CloudAssembly implements ICloudAssembly { public getStackByName(stackName: string): CloudFormationStackArtifact { const artifacts = this.artifacts.filter(a => a instanceof CloudFormationStackArtifact && a.stackName === stackName); if (!artifacts || artifacts.length === 0) { - throw new Error(`Unable to find stack with stack name "${stackName}"`); + throw new CloudAssemblyError(`Unable to find stack with stack name "${stackName}"`); } if (artifacts.length > 1) { // eslint-disable-next-line max-len - throw new Error(`There are multiple stacks with the stack name "${stackName}" (${artifacts.map(a => a.id).join(',')}). Use "getStackArtifact(id)" instead`); + throw new CloudAssemblyError(`There are multiple stacks with the stack name "${stackName}" (${artifacts.map(a => a.id).join(',')}). Use "getStackArtifact(id)" instead`); } return artifacts[0] as CloudFormationStackArtifact; @@ -128,11 +129,11 @@ export class CloudAssembly implements ICloudAssembly { const artifact = this.tryGetArtifactRecursively(artifactId); if (!artifact) { - throw new Error(`Unable to find artifact with id "${artifactId}"`); + throw new CloudAssemblyError(`Unable to find artifact with id "${artifactId}"`); } if (!(artifact instanceof CloudFormationStackArtifact)) { - throw new Error(`Artifact ${artifactId} is not a CloudFormation stack`); + throw new CloudAssemblyError(`Artifact ${artifactId} is not a CloudFormation stack`); } return artifact; @@ -167,11 +168,11 @@ export class CloudAssembly implements ICloudAssembly { public getNestedAssemblyArtifact(artifactId: string): NestedCloudAssemblyArtifact { const artifact = this.tryGetArtifact(artifactId); if (!artifact) { - throw new Error(`Unable to find artifact with id "${artifactId}"`); + throw new CloudAssemblyError(`Unable to find artifact with id "${artifactId}"`); } if (!(artifact instanceof NestedCloudAssemblyArtifact)) { - throw new Error(`Found artifact '${artifactId}' but it's not a nested cloud assembly`); + throw new CloudAssemblyError(`Found artifact '${artifactId}' but it's not a nested cloud assembly`); } return artifact; @@ -196,12 +197,12 @@ export class CloudAssembly implements ICloudAssembly { if (trees.length === 0) { return undefined; } else if (trees.length > 1) { - throw new Error(`Multiple artifacts of type ${cxschema.ArtifactType.CDK_TREE} found in manifest`); + throw new CloudAssemblyError(`Multiple artifacts of type ${cxschema.ArtifactType.CDK_TREE} found in manifest`); } const tree = trees[0]; if (!(tree instanceof TreeCloudArtifact)) { - throw new Error('"Tree" artifact is not of expected type'); + throw new CloudAssemblyError('"Tree" artifact is not of expected type'); } return tree; @@ -481,7 +482,7 @@ function determineOutputDirectory(outdir?: string) { function ensureDirSync(dir: string) { if (fs.existsSync(dir)) { if (!fs.statSync(dir).isDirectory()) { - throw new Error(`${dir} must be a directory`); + throw new CloudAssemblyError(`${dir} must be a directory`); } } else { fs.mkdirSync(dir, { recursive: true }); diff --git a/packages/aws-cdk-lib/cx-api/lib/environment.ts b/packages/aws-cdk-lib/cx-api/lib/environment.ts index 3ba1369e76850..941cc2675a01a 100644 --- a/packages/aws-cdk-lib/cx-api/lib/environment.ts +++ b/packages/aws-cdk-lib/cx-api/lib/environment.ts @@ -1,3 +1,5 @@ +import { CloudAssemblyError } from './private/error'; + /** * Parser for the artifact environment field. * @@ -26,14 +28,14 @@ export class EnvironmentUtils { public static parse(environment: string): Environment { const env = AWS_ENV_REGEX.exec(environment); if (!env) { - throw new Error( + throw new CloudAssemblyError( `Unable to parse environment specification "${environment}". ` + 'Expected format: aws://account/region'); } const [, account, region] = env; if (!account || !region) { - throw new Error(`Invalid environment specification: ${environment}`); + throw new CloudAssemblyError(`Invalid environment specification: ${environment}`); } return { account, region, name: environment }; diff --git a/packages/aws-cdk-lib/cx-api/lib/features.ts b/packages/aws-cdk-lib/cx-api/lib/features.ts index 391ff8139fa5b..5aa79973fb712 100644 --- a/packages/aws-cdk-lib/cx-api/lib/features.ts +++ b/packages/aws-cdk-lib/cx-api/lib/features.ts @@ -1432,6 +1432,8 @@ export const CURRENT_VERSION_FLAG_DEFAULTS = Object.fromEntries(Object.entries(F export function futureFlagDefault(flag: string): boolean { const value = CURRENT_VERSION_FLAG_DEFAULTS[flag] ?? false; if (typeof value !== 'boolean') { + // This should never happen, if this error is thrown it's a bug + // eslint-disable-next-line @cdklabs/no-throw-default-error throw new Error(`futureFlagDefault: default type of flag '${flag}' should be boolean, got '${typeof value}'`); } return value; diff --git a/packages/aws-cdk-lib/cx-api/lib/private/error.ts b/packages/aws-cdk-lib/cx-api/lib/private/error.ts new file mode 100644 index 0000000000000..3067ad4be99a2 --- /dev/null +++ b/packages/aws-cdk-lib/cx-api/lib/private/error.ts @@ -0,0 +1,35 @@ +const ASSEMBLY_ERROR_SYMBOL = Symbol.for('@aws-cdk/cx-api.CloudAssemblyError'); + +/** + * A CloudAssemblyError is thrown for issues with the synthesized CloudAssembly. + * + * These are typically exceptions that are unexpected for end-users, + * and should only occur during abnormal operation, e.g. when the synthesis + * didn't fully complete. + * + * @internal + */ +export class CloudAssemblyError extends Error { + #time: string; + + /** + * The time the error was thrown. + */ + public get time(): string { + return this.#time; + } + + public get type(): 'assembly' { + return 'assembly'; + } + + constructor(msg: string) { + super(msg); + + Object.setPrototypeOf(this, CloudAssemblyError.prototype); + Object.defineProperty(this, ASSEMBLY_ERROR_SYMBOL, { value: true }); + + this.name = new.target.name; + this.#time = new Date().toISOString(); + } +} diff --git a/packages/aws-cdk-lib/cx-api/lib/toposort.ts b/packages/aws-cdk-lib/cx-api/lib/toposort.ts index ece77f1307876..5141c2a91024c 100644 --- a/packages/aws-cdk-lib/cx-api/lib/toposort.ts +++ b/packages/aws-cdk-lib/cx-api/lib/toposort.ts @@ -1,3 +1,5 @@ +import { CloudAssemblyError } from './private/error'; + export type KeyFunc = (x: T) => string; export type DepFunc = (x: T) => string[]; @@ -30,7 +32,7 @@ export function topologicalSort(xs: Iterable, keyFn: KeyFunc, depFn: De // If we didn't make any progress, we got stuck if (selectable.length === 0) { - throw new Error(`Could not determine ordering between: ${Array.from(remaining.keys()).join(', ')}`); + throw new CloudAssemblyError(`Could not determine ordering between: ${Array.from(remaining.keys()).join(', ')}`); } }