From 2607eb3a905f735b96713dda4f32d28d10d686fd Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Fri, 6 Dec 2024 18:16:14 -0800 Subject: [PATCH] fix(lambda): improve validation errors for lambda functions (#32323) ### Issue # (if applicable) Relates to #32324 ### Reason for this change Currently all errors are untyped. This makes it difficult users to programmatically distinguish between different classes of errors, e.g. what is a validation error vs what is a syntax error? With this change, users can catch errors and check their type before proceeding accordingly. ### Description of changes Addition of a new Error type `ValidationError`. For now this error is used only in a single file. The intention is to extend this to all error cases. `ValidationError` extends an abstract `ConstructError` which also handles any improvements to error display. `ConstructError` manipulates the stack trace to improve display. It's changing two things, both of which are based on a construct that is passed in on error creation. If not construct is passed, the error behaves as before. 1. Construct information is inserted as the first line of the stack trace. 2. The strack trace is captured from the point of _creation of the construct_. That is the class constructor call. This is achieved by passing the error's constructs into [Error.captureStackTrace](https://nodejs.org/docs/latest-v22.x/api/errors.html#errorcapturestacktracetargetobject-constructoropt). As a side effect, in many cases the "line of error" is not minified code anymore and thus doesn't ruin the error experience for users. See comments for current vs future errors. ### Description of how you validated changes Existing test. Manual testing of error cases. ### 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* --- .../aws-cdk-lib/aws-lambda/lib/function.ts | 87 +++++------ packages/aws-cdk-lib/core/lib/errors.ts | 145 ++++++++++++++++++ packages/aws-cdk-lib/core/lib/index.ts | 1 + packages/aws-cdk-lib/core/test/errors.test.ts | 34 ++++ 4 files changed, 224 insertions(+), 43 deletions(-) create mode 100644 packages/aws-cdk-lib/core/lib/errors.ts create mode 100644 packages/aws-cdk-lib/core/test/errors.test.ts diff --git a/packages/aws-cdk-lib/aws-lambda/lib/function.ts b/packages/aws-cdk-lib/aws-lambda/lib/function.ts index 38481f883b92e..ad239483b50ef 100644 --- a/packages/aws-cdk-lib/aws-lambda/lib/function.ts +++ b/packages/aws-cdk-lib/aws-lambda/lib/function.ts @@ -30,6 +30,7 @@ import * as logs from '../../aws-logs'; import * as sns from '../../aws-sns'; import * as sqs from '../../aws-sqs'; import { Annotations, ArnFormat, CfnResource, Duration, FeatureFlags, Fn, IAspect, Lazy, Names, Size, Stack, Token } from '../../core'; +import { ValidationError } from '../../core/lib/errors'; import { LAMBDA_RECOGNIZE_LAYER_VERSION } from '../../cx-api'; /** @@ -917,16 +918,16 @@ export class Function extends FunctionBase { if (props.functionName && !Token.isUnresolved(props.functionName)) { if (props.functionName.length > 64) { - throw new Error(`Function name can not be longer than 64 characters but has ${props.functionName.length} characters.`); + throw new ValidationError(`Function name can not be longer than 64 characters but has ${props.functionName.length} characters.`, this); } if (!/^[a-zA-Z0-9-_]+$/.test(props.functionName)) { - throw new Error(`Function name ${props.functionName} can contain only letters, numbers, hyphens, or underscores with no spaces.`); + throw new ValidationError(`Function name ${props.functionName} can contain only letters, numbers, hyphens, or underscores with no spaces.`, this); } } if (props.description && !Token.isUnresolved(props.description)) { if (props.description.length > 256) { - throw new Error(`Function description can not be longer than 256 characters but has ${props.description.length} characters.`); + throw new ValidationError(`Function description can not be longer than 256 characters but has ${props.description.length} characters.`, this); } } @@ -951,10 +952,10 @@ export class Function extends FunctionBase { const config = props.filesystem.config; if (!Token.isUnresolved(config.localMountPath)) { if (!/^\/mnt\/[a-zA-Z0-9-_.]+$/.test(config.localMountPath)) { - throw new Error(`Local mount path should match with ^/mnt/[a-zA-Z0-9-_.]+$ but given ${config.localMountPath}.`); + throw new ValidationError(`Local mount path should match with ^/mnt/[a-zA-Z0-9-_.]+$ but given ${config.localMountPath}.`, this); } if (config.localMountPath.length > 160) { - throw new Error(`Local mount path can not be longer than 160 characters but has ${config.localMountPath.length} characters.`); + throw new ValidationError(`Local mount path can not be longer than 160 characters but has ${config.localMountPath.length} characters.`, this); } } if (config.policies) { @@ -1019,16 +1020,16 @@ export class Function extends FunctionBase { } if (props.architecture && props.architectures !== undefined) { - throw new Error('Either architecture or architectures must be specified but not both.'); + throw new ValidationError('Either architecture or architectures must be specified but not both.', this); } if (props.architectures && props.architectures.length > 1) { - throw new Error('Only one architecture must be specified.'); + throw new ValidationError('Only one architecture must be specified.', this); } this._architecture = props.architecture ?? (props.architectures && props.architectures[0]); if (props.ephemeralStorageSize && !props.ephemeralStorageSize.isUnresolved() && (props.ephemeralStorageSize.toMebibytes() < 512 || props.ephemeralStorageSize.toMebibytes() > 10240)) { - throw new Error(`Ephemeral storage size must be between 512 and 10240 MB, received ${props.ephemeralStorageSize}.`); + throw new ValidationError(`Ephemeral storage size must be between 512 and 10240 MB, received ${props.ephemeralStorageSize}.`, this); } const resource: CfnFunction = new CfnFunction(this, 'Resource', { @@ -1096,7 +1097,7 @@ export class Function extends FunctionBase { if (props.layers) { if (props.runtime === Runtime.FROM_IMAGE) { - throw new Error('Layers are not supported for container image functions'); + throw new ValidationError('Layers are not supported for container image functions', this); } this.addLayers(...props.layers); @@ -1109,7 +1110,7 @@ export class Function extends FunctionBase { // Log retention if (props.logRetention) { if (props.logGroup) { - throw new Error('CDK does not support setting logRetention and logGroup'); + throw new ValidationError('CDK does not support setting logRetention and logGroup', this); } const logRetention = new logs.LogRetention(this, 'LogRetention', { logGroupName: `/aws/lambda/${this.functionName}`, @@ -1137,7 +1138,7 @@ export class Function extends FunctionBase { if (props.filesystem) { if (!props.vpc) { - throw new Error('Cannot configure \'filesystem\' without configuring a VPC.'); + throw new ValidationError('Cannot configure \'filesystem\' without configuring a VPC.', this); } const config = props.filesystem.config; if (config.dependency) { @@ -1201,7 +1202,7 @@ export class Function extends FunctionBase { 'LAMBDA_RUNTIME_DIR', ]; if (reservedEnvironmentVariables.includes(key)) { - throw new Error(`${key} environment variable is reserved by the lambda runtime and can not be set manually. See https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html`); + throw new ValidationError(`${key} environment variable is reserved by the lambda runtime and can not be set manually. See https://docs.aws.amazon.com/lambda/latest/dg/configuration-envvars.html`, this); } this.environment[key] = { value, ...options }; return this; @@ -1214,24 +1215,24 @@ export class Function extends FunctionBase { */ private getLoggingConfig(props: FunctionProps): CfnFunction.LoggingConfigProperty | undefined { if (props.logFormat && props.loggingFormat) { - throw new Error('Only define LogFormat or LoggingFormat, not both.'); + throw new ValidationError('Only define LogFormat or LoggingFormat, not both.', this); } if (props.applicationLogLevel && props.applicationLogLevelV2) { - throw new Error('Only define applicationLogLevel or applicationLogLevelV2, not both.'); + throw new ValidationError('Only define applicationLogLevel or applicationLogLevelV2, not both.', this); } if (props.systemLogLevel && props.systemLogLevelV2) { - throw new Error('Only define systemLogLevel or systemLogLevelV2, not both.'); + throw new ValidationError('Only define systemLogLevel or systemLogLevelV2, not both.', this); } if (props.applicationLogLevel || props.applicationLogLevelV2 || props.systemLogLevel || props.systemLogLevelV2) { if (props.logFormat !== LogFormat.JSON && props.loggingFormat === undefined) { - throw new Error(`To use ApplicationLogLevel and/or SystemLogLevel you must set LogFormat to '${LogFormat.JSON}', got '${props.logFormat}'.`); + throw new ValidationError(`To use ApplicationLogLevel and/or SystemLogLevel you must set LogFormat to '${LogFormat.JSON}', got '${props.logFormat}'.`, this); } if (props.loggingFormat !== LoggingFormat.JSON && props.logFormat === undefined) { - throw new Error(`To use ApplicationLogLevel and/or SystemLogLevel you must set LoggingFormat to '${LoggingFormat.JSON}', got '${props.loggingFormat}'.`); + throw new ValidationError(`To use ApplicationLogLevel and/or SystemLogLevel you must set LoggingFormat to '${LoggingFormat.JSON}', got '${props.loggingFormat}'.`, this); } } @@ -1268,7 +1269,7 @@ export class Function extends FunctionBase { */ public invalidateVersionBasedOn(x: string) { if (Token.isUnresolved(x)) { - throw new Error('invalidateVersionOn: input may not contain unresolved tokens'); + throw new ValidationError('invalidateVersionOn: input may not contain unresolved tokens', this); } this.hashMixins.push(x); } @@ -1283,11 +1284,11 @@ export class Function extends FunctionBase { public addLayers(...layers: ILayerVersion[]): void { for (const layer of layers) { if (this._layers.length === 5) { - throw new Error('Unable to add layer: this lambda function already uses 5 layers.'); + throw new ValidationError('Unable to add layer: this lambda function already uses 5 layers.', this); } if (layer.compatibleRuntimes && !layer.compatibleRuntimes.find(runtime => runtime.runtimeEquals(this.runtime))) { const runtimes = layer.compatibleRuntimes.map(runtime => runtime.name).join(', '); - throw new Error(`This lambda function uses a runtime that is incompatible with this layer (${this.runtime.name} is not in [${runtimes}])`); + throw new ValidationError(`This lambda function uses a runtime that is incompatible with this layer (${this.runtime.name} is not in [${runtimes}])`, this); } // Currently no validations for compatible architectures since Lambda service @@ -1398,8 +1399,8 @@ export class Function extends FunctionBase { } const envKeys = Object.keys(this.environment); if (envKeys.length !== 0) { - throw new Error(`The function ${this.node.path} contains environment variables [${envKeys}] and is not compatible with Lambda@Edge. \ -Environment variables can be marked for removal when used in Lambda@Edge by setting the \'removeInEdge\' property in the \'addEnvironment()\' API.`); + throw new ValidationError(`The function ${this.node.path} contains environment variables [${envKeys}] and is not compatible with Lambda@Edge. \ +Environment variables can be marked for removal when used in Lambda@Edge by setting the \'removeInEdge\' property in the \'addEnvironment()\' API.`, this); } return; @@ -1435,19 +1436,19 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett } if (props.runtime === Runtime.FROM_IMAGE) { - throw new Error("ADOT Lambda layer can't be configured with container image package type"); + throw new ValidationError("ADOT Lambda layer can't be configured with container image package type", this); } // This is not the complete list of incompatible runtimes and layer types. We are only // checking for common mistakes on a best-effort basis. if (this.runtime === Runtime.GO_1_X) { - throw new Error('Runtime go1.x is not supported by the ADOT Lambda Go SDK'); + throw new ValidationError('Runtime go1.x is not supported by the ADOT Lambda Go SDK', this); } // The Runtime is Python and Adot is set it requires a different EXEC_WRAPPER than the other code bases. if (this.runtime.family === RuntimeFamily.PYTHON && props.adotInstrumentation.execWrapper.valueOf() !== AdotLambdaExecWrapper.INSTRUMENT_HANDLER) { - throw new Error('Python Adot Lambda layer requires AdotLambdaExecWrapper.INSTRUMENT_HANDLER'); + throw new ValidationError('Python Adot Lambda layer requires AdotLambdaExecWrapper.INSTRUMENT_HANDLER', this); } this.addLayers(LayerVersion.fromLayerVersionArn(this, 'AdotLayer', props.adotInstrumentation.layerVersion._bind(this).arn)); @@ -1510,47 +1511,47 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett */ private configureVpc(props: FunctionProps): CfnFunction.VpcConfigProperty | undefined { if (props.securityGroup && props.securityGroups) { - throw new Error('Only one of the function props, securityGroup or securityGroups, is allowed'); + throw new ValidationError('Only one of the function props, securityGroup or securityGroups, is allowed', this); } const hasSecurityGroups = props.securityGroups && props.securityGroups.length > 0; if (!props.vpc) { if (props.allowAllOutbound !== undefined) { - throw new Error('Cannot configure \'allowAllOutbound\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'allowAllOutbound\' without configuring a VPC', this); } if (props.securityGroup) { - throw new Error('Cannot configure \'securityGroup\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'securityGroup\' without configuring a VPC', this); } if (hasSecurityGroups) { - throw new Error('Cannot configure \'securityGroups\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'securityGroups\' without configuring a VPC', this); } if (props.vpcSubnets) { - throw new Error('Cannot configure \'vpcSubnets\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'vpcSubnets\' without configuring a VPC', this); } if (props.ipv6AllowedForDualStack) { - throw new Error('Cannot configure \'ipv6AllowedForDualStack\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'ipv6AllowedForDualStack\' without configuring a VPC', this); } if (props.allowAllIpv6Outbound !== undefined) { - throw new Error('Cannot configure \'allowAllIpv6Outbound\' without configuring a VPC'); + throw new ValidationError('Cannot configure \'allowAllIpv6Outbound\' without configuring a VPC', this); } return undefined; } if (props.allowAllOutbound !== undefined) { if (props.securityGroup) { - throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroup.'); + throw new ValidationError('Configure \'allowAllOutbound\' directly on the supplied SecurityGroup.', this); } if (hasSecurityGroups) { - throw new Error('Configure \'allowAllOutbound\' directly on the supplied SecurityGroups.'); + throw new ValidationError('Configure \'allowAllOutbound\' directly on the supplied SecurityGroups.', this); } } if (props.allowAllIpv6Outbound !== undefined) { if (props.securityGroup) { - throw new Error('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroup.'); + throw new ValidationError('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroup.', this); } if (hasSecurityGroups) { - throw new Error('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroups.'); + throw new ValidationError('Configure \'allowAllIpv6Outbound\' directly on the supplied SecurityGroups.', this); } } @@ -1585,8 +1586,8 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett const publicSubnetIds = new Set(props.vpc.publicSubnets.map(s => s.subnetId)); for (const subnetId of selectedSubnets.subnetIds) { if (publicSubnetIds.has(subnetId) && !allowPublicSubnet) { - throw new Error('Lambda Functions in a public subnet can NOT access the internet. ' + - 'If you are aware of this limitation and would still like to place the function in a public subnet, set `allowPublicSubnet` to true'); + throw new ValidationError('Lambda Functions in a public subnet can NOT access the internet. ' + + 'If you are aware of this limitation and would still like to place the function in a public subnet, set `allowPublicSubnet` to true', this); } } this.node.addDependency(selectedSubnets.internetConnectivityEstablished); @@ -1622,15 +1623,15 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett Annotations.of(this).addWarningV2('@aws-cdk/aws-lambda:snapStartRequirePublish', 'SnapStart only support published Lambda versions. Ignore if function already have published versions'); if (!props.runtime.supportsSnapStart) { - throw new Error(`SnapStart currently not supported by runtime ${props.runtime.name}`); + throw new ValidationError(`SnapStart currently not supported by runtime ${props.runtime.name}`, this); } if (props.filesystem) { - throw new Error('SnapStart is currently not supported using EFS'); + throw new ValidationError('SnapStart is currently not supported using EFS', this); } if (props.ephemeralStorageSize && props.ephemeralStorageSize?.toMebibytes() > 512) { - throw new Error('SnapStart is currently not supported using more than 512 MiB Ephemeral Storage'); + throw new ValidationError('SnapStart is currently not supported using more than 512 MiB Ephemeral Storage', this); } return props.snapStart._render(); @@ -1648,7 +1649,7 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett throw Error('deadLetterQueue defined but deadLetterQueueEnabled explicitly set to false'); } if (props.deadLetterTopic && (props.deadLetterQueue || props.deadLetterQueueEnabled !== undefined)) { - throw new Error('deadLetterQueue and deadLetterTopic cannot be specified together at the same time'); + throw new ValidationError('deadLetterQueue and deadLetterTopic cannot be specified together at the same time', this); } let deadLetterQueue: sqs.IQueue | sns.ITopic; @@ -1698,7 +1699,7 @@ Environment variables can be marked for removal when used in Lambda@Edge by sett private validateProfiling(props: FunctionProps) { if (!props.runtime.supportsCodeGuruProfiling) { - throw new Error(`CodeGuru profiling is not supported by runtime ${props.runtime.name}`); + throw new ValidationError(`CodeGuru profiling is not supported by runtime ${props.runtime.name}`, this); } if (props.environment && (props.environment.AWS_CODEGURU_PROFILER_GROUP_NAME || props.environment.AWS_CODEGURU_PROFILER_GROUP_ARN diff --git a/packages/aws-cdk-lib/core/lib/errors.ts b/packages/aws-cdk-lib/core/lib/errors.ts new file mode 100644 index 0000000000000..727cf0228c396 --- /dev/null +++ b/packages/aws-cdk-lib/core/lib/errors.ts @@ -0,0 +1,145 @@ +import { IConstruct } from 'constructs'; +import { constructInfoFromConstruct } from './helpers-internal'; + +const CONSTRUCT_ERROR_SYMBOL = Symbol.for('@aws-cdk/core.SynthesisError'); +const VALIDATION_ERROR_SYMBOL = Symbol.for('@aws-cdk/core.ValidationError'); + +/** + * Helper to check if an error is a SynthesisErrors + */ +export class Errors { + /** + * Test whether the given errors is a ConstructionError + */ + public static isConstructError(x: any): x is ConstructError { + return x !== null && typeof(x) === 'object' && CONSTRUCT_ERROR_SYMBOL in x; + } + + /** + * Test whether the given error is a ValidationError + */ + public static isValidationError(x: any): x is ValidationError { + return Errors.isConstructError(x) && VALIDATION_ERROR_SYMBOL in x; + } +} + +interface ConstructInfo { + readonly fqn: string; + readonly version: string; +} + +/** + * Generic, abstract error that is thrown from the users app during app construction or synth. + */ +abstract class ConstructError extends Error { + #time: string; + #constructPath?: string; + #constructInfo?: ConstructInfo; + + /** + * The time the error was thrown. + */ + public get time(): string { + return this.#time; + } + + /** + * The level. Always `'error'`. + */ + public get level(): 'error' { + return 'error'; + } + + /** + * The type of the error. + */ + public abstract get type(): string; + + /** + * The path of the construct this error is thrown from, if available. + */ + public get constructPath(): string | undefined { + return this.#constructPath; + } + + /** + * Information on the construct this error is thrown from, if available. + */ + public get constructInfo(): ConstructInfo | undefined { + return this.#constructInfo; + } + + constructor(msg: string, scope?: IConstruct) { + super(msg); + Object.setPrototypeOf(this, ConstructError.prototype); + Object.defineProperty(this, CONSTRUCT_ERROR_SYMBOL, { value: true }); + + this.name = new.target.name; + this.#time = new Date().toISOString(); + this.#constructPath = scope?.node.path; + + if (scope) { + Error.captureStackTrace(this, scope.constructor); + try { + this.#constructInfo = scope ? constructInfoFromConstruct(scope) : undefined; + } catch (_) { + // we don't want to fail if construct info is not available + } + } + + const stack = [ + `${this.name}: ${this.message}`, + ]; + + if (this.constructInfo) { + stack.push(`in ${this.constructInfo?.fqn} at [${this.constructPath}]`); + } else { + stack.push(`in [${this.constructPath}]`); + } + + if (this.stack) { + stack.push(this.stack); + } + + this.stack = this.constructStack(this.stack); + } + + /** + * Helper message to clean-up the stack and amend with construct information. + */ + private constructStack(prev?: string) { + const indent = ' '.repeat(4); + + const stack = [ + `${this.name}: ${this.message}`, + ]; + + if (this.constructInfo) { + stack.push(`${indent}at path [${this.constructPath}] in ${this.constructInfo?.fqn}`); + } else { + stack.push(`${indent}at path [${this.constructPath}]`); + } + + if (prev) { + stack.push(''); + stack.push(...prev.split('\n').slice(1)); + } + + return stack.join('\n'); + } +} + +/** + * An Error that is thrown when a construct has validation errors. + */ +export class ValidationError extends ConstructError { + public get type(): 'validation' { + return 'validation'; + } + + constructor(msg: string, scope: IConstruct) { + super(msg, scope); + Object.setPrototypeOf(this, ValidationError.prototype); + Object.defineProperty(this, VALIDATION_ERROR_SYMBOL, { value: true }); + } +} diff --git a/packages/aws-cdk-lib/core/lib/index.ts b/packages/aws-cdk-lib/core/lib/index.ts index b35e89c0e59e4..9fb4041a136a0 100644 --- a/packages/aws-cdk-lib/core/lib/index.ts +++ b/packages/aws-cdk-lib/core/lib/index.ts @@ -35,6 +35,7 @@ export * from './expiration'; export * from './size'; export * from './stack-trace'; export { Element } from './deps'; +export { Errors } from './errors'; export * from './app'; export * from './context-provider'; diff --git a/packages/aws-cdk-lib/core/test/errors.test.ts b/packages/aws-cdk-lib/core/test/errors.test.ts new file mode 100644 index 0000000000000..a1a7f34de051e --- /dev/null +++ b/packages/aws-cdk-lib/core/test/errors.test.ts @@ -0,0 +1,34 @@ +import { App, Stack } from '../lib'; +import { Errors, ValidationError } from '../lib/errors'; + +jest + .useFakeTimers() + .setSystemTime(new Date('2020-01-01')); + +describe('ValidationError', () => { + test('ValidationError is ValidationError and ConstructError', () => { + const error = new ValidationError('this is an error', new App()); + + expect(Errors.isConstructError(error)).toBe(true); + expect(Errors.isValidationError(error)).toBe(true); + }); + + test('ValidationError data', () => { + const app = new App(); + const stack = new Stack(app, 'MyStack'); + const error = new ValidationError('this is an error', stack); + + expect(error.time).toBe('2020-01-01T00:00:00.000Z'); + expect(error.type).toBe('validation'); + expect(error.level).toBe('error'); + expect(error.constructPath).toBe('MyStack'); + expect(error.constructInfo).toMatchObject({ + // fqns are different when run from compiled JS (first) and dynamically from TS (second) + fqn: expect.stringMatching(/aws-cdk-lib\.Stack|constructs\.Construct/), + version: expect.stringMatching(/^\d+\.\d+\.\d+$/), + }); + expect(error.message).toBe('this is an error'); + expect(error.stack).toContain('ValidationError: this is an error'); + expect(error.stack).toContain('at path [MyStack] in'); + }); +});