Skip to content

Commit

Permalink
feat(cx-api): throw CloudAssemblyError instead of untyped Errors (#…
Browse files Browse the repository at this point in the history
…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*
  • Loading branch information
mrgrain authored Feb 12, 2025
1 parent d3f3309 commit ae95d95
Show file tree
Hide file tree
Showing 11 changed files with 76 additions and 18 deletions.
1 change: 1 addition & 0 deletions packages/aws-cdk-lib/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ const enableNoThrowDefaultErrorIn = [
'aws-ssmincidents',
'aws-ssmquicksetup',
'aws-synthetics',
'cx-api',
'aws-s3',
'aws-s3-assets',
'aws-s3-deployment',
Expand Down
11 changes: 11 additions & 0 deletions packages/aws-cdk-lib/core/lib/errors.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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');

Expand Down Expand Up @@ -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;
}
Expand Down
5 changes: 3 additions & 2 deletions packages/aws-cdk-lib/cx-api/lib/cloud-artifact.ts
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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');
}

/**
Expand Down Expand Up @@ -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;
});
Expand Down
19 changes: 10 additions & 9 deletions packages/aws-cdk-lib/cx-api/lib/cloud-assembly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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 });
Expand Down
6 changes: 4 additions & 2 deletions packages/aws-cdk-lib/cx-api/lib/environment.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { CloudAssemblyError } from './private/error';

/**
* Parser for the artifact environment field.
*
Expand Down Expand Up @@ -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 };
Expand Down
2 changes: 2 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/features.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
35 changes: 35 additions & 0 deletions packages/aws-cdk-lib/cx-api/lib/private/error.ts
Original file line number Diff line number Diff line change
@@ -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();
}
}
4 changes: 3 additions & 1 deletion packages/aws-cdk-lib/cx-api/lib/toposort.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { CloudAssemblyError } from './private/error';

export type KeyFunc<T> = (x: T) => string;
export type DepFunc<T> = (x: T) => string[];

Expand Down Expand Up @@ -30,7 +32,7 @@ export function topologicalSort<T>(xs: Iterable<T>, keyFn: KeyFunc<T>, 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(', ')}`);
}
}

Expand Down

0 comments on commit ae95d95

Please sign in to comment.