Skip to content

Commit

Permalink
feat(sns): throw ValidationError instead of untyped errors (#33045)
Browse files Browse the repository at this point in the history 
### Issue 

`aws-sns` for #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 basically 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
@kaizencc
kaizencc authored Jan 22, 2025
1 parent 0b2db62 commit 7452462
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 35 deletions.
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/.eslintrc.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [


// no-throw-default-error
const modules = ['aws-s3', 'aws-lambda', 'aws-rds'];
const modules = ['aws-s3', 'aws-lambda', 'aws-rds', 'aws-sns'];
baseConfig.overrides.push({
files: modules.map(m => `./${m}/lib/**`),
rules: { "@cdklabs/no-throw-default-error": ['error'] },
Expand Down
8 changes: 5 additions & 3 deletions packages/aws-cdk-lib/aws-sns/lib/subscription-filter.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { UnscopedValidationError } from '../../core/lib/errors';

/**
* Conditions that can be applied to string attributes.
*/
Expand Down Expand Up @@ -131,10 +133,10 @@ export class SubscriptionFilter {
const conditions = new Array<any>();

if (stringConditions.whitelist && stringConditions.allowlist) {
throw new Error('`whitelist` is deprecated; please use `allowlist` instead');
throw new UnscopedValidationError('`whitelist` is deprecated; please use `allowlist` instead');
}
if (stringConditions.blacklist && stringConditions.denylist) {
throw new Error('`blacklist` is deprecated; please use `denylist` instead');
throw new UnscopedValidationError('`blacklist` is deprecated; please use `denylist` instead');
}
const allowlist = stringConditions.allowlist ?? stringConditions.whitelist;
const denylist = stringConditions.denylist ?? stringConditions.blacklist;
Expand Down Expand Up @@ -165,7 +167,7 @@ export class SubscriptionFilter {
const conditions = new Array<any>();

if (numericConditions.whitelist && numericConditions.allowlist) {
throw new Error('`whitelist` is deprecated; please use `allowlist` instead');
throw new UnscopedValidationError('`whitelist` is deprecated; please use `allowlist` instead');
}
const allowlist = numericConditions.allowlist ?? numericConditions.whitelist;

Expand Down
40 changes: 21 additions & 19 deletions packages/aws-cdk-lib/aws-sns/lib/subscription.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { ITopic } from './topic-base';
import { PolicyStatement, ServicePrincipal } from '../../aws-iam';
import { IQueue } from '../../aws-sqs';
import { Resource } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Options for creating a new subscription
Expand Down Expand Up @@ -114,12 +115,12 @@ export class Subscription extends Resource {
SubscriptionProtocol.FIREHOSE,
]
.indexOf(props.protocol) < 0) {
throw new Error('Raw message delivery can only be enabled for HTTP, HTTPS, SQS, and Firehose subscriptions.');
throw new ValidationError('Raw message delivery can only be enabled for HTTP, HTTPS, SQS, and Firehose subscriptions.', this);
}

if (props.filterPolicy) {
if (Object.keys(props.filterPolicy).length > 5) {
throw new Error('A filter policy can have a maximum of 5 attribute names.');
throw new ValidationError('A filter policy can have a maximum of 5 attribute names.', this);
}

this.filterPolicy = Object.entries(props.filterPolicy)
Expand All @@ -131,22 +132,22 @@ export class Subscription extends Resource {
let total = 1;
Object.values(this.filterPolicy).forEach(filter => { total *= filter.length; });
if (total > 150) {
throw new Error(`The total combination of values (${total}) must not exceed 150.`);
throw new ValidationError(`The total combination of values (${total}) must not exceed 150.`, this);
}
} else if (props.filterPolicyWithMessageBody) {
if (Object.keys(props.filterPolicyWithMessageBody).length > 5) {
throw new Error('A filter policy can have a maximum of 5 attribute names.');
throw new ValidationError('A filter policy can have a maximum of 5 attribute names.', this);
}
this.filterPolicyWithMessageBody = props.filterPolicyWithMessageBody;
}

if (props.protocol === SubscriptionProtocol.FIREHOSE && !props.subscriptionRoleArn) {
throw new Error('Subscription role arn is required field for subscriptions with a firehose protocol.');
throw new ValidationError('Subscription role arn is required field for subscriptions with a firehose protocol.', this);
}

// Format filter policy
const filterPolicy = this.filterPolicyWithMessageBody
? buildFilterPolicyWithMessageBody(this.filterPolicyWithMessageBody)
? buildFilterPolicyWithMessageBody(this, this.filterPolicyWithMessageBody)
: this.filterPolicy;

this.deadLetterQueue = this.buildDeadLetterQueue(props);
Expand All @@ -167,7 +168,7 @@ export class Subscription extends Resource {

private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy, protocol: SubscriptionProtocol): any {
if (![SubscriptionProtocol.HTTP, SubscriptionProtocol.HTTPS].includes(protocol)) {
throw new Error(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`);
throw new ValidationError(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`, this);
}
const { healthyRetryPolicy, throttlePolicy } = deliveryPolicy;
if (healthyRetryPolicy) {
Expand All @@ -176,45 +177,45 @@ export class Subscription extends Resource {
const maxDelayTarget = healthyRetryPolicy.maxDelayTarget;
if (minDelayTarget !== undefined) {
if (minDelayTarget.toMilliseconds() % 1000 !== 0) {
throw new Error(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`);
throw new ValidationError(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`, this);
}
const minDelayTargetSecs = minDelayTarget.toSeconds();
if (minDelayTargetSecs < 1 || minDelayTargetSecs > delayTargetLimitSecs) {
throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${minDelayTargetSecs}s`);
throw new ValidationError(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${minDelayTargetSecs}s`, this);
}
}
if (maxDelayTarget !== undefined) {
if (maxDelayTarget.toMilliseconds() % 1000 !== 0) {
throw new Error(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`);
throw new ValidationError(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`, this);
}
const maxDelayTargetSecs = maxDelayTarget.toSeconds();
if (maxDelayTargetSecs < 1 || maxDelayTargetSecs > delayTargetLimitSecs) {
throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${maxDelayTargetSecs}s`);
throw new ValidationError(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${maxDelayTargetSecs}s`, this);
}
if ((minDelayTarget !== undefined) && minDelayTarget.toSeconds() > maxDelayTargetSecs) {
throw new Error('minDelayTarget must not exceed maxDelayTarget');
throw new ValidationError('minDelayTarget must not exceed maxDelayTarget', this);
}
}

const numRetriesLimit = 100;
if (healthyRetryPolicy.numRetries && (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit)) {
throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive, got: ${healthyRetryPolicy.numRetries}`);
throw new ValidationError(`numRetries must be between 0 and ${numRetriesLimit} inclusive, got: ${healthyRetryPolicy.numRetries}`, this);
}
const { numNoDelayRetries, numMinDelayRetries, numMaxDelayRetries } = healthyRetryPolicy;
if (numNoDelayRetries && (numNoDelayRetries < 0 || !Number.isInteger(numNoDelayRetries))) {
throw new Error(`numNoDelayRetries must be an integer zero or greater, got: ${numNoDelayRetries}`);
throw new ValidationError(`numNoDelayRetries must be an integer zero or greater, got: ${numNoDelayRetries}`, this);
}
if (numMinDelayRetries && (numMinDelayRetries < 0 || !Number.isInteger(numMinDelayRetries))) {
throw new Error(`numMinDelayRetries must be an integer zero or greater, got: ${numMinDelayRetries}`);
throw new ValidationError(`numMinDelayRetries must be an integer zero or greater, got: ${numMinDelayRetries}`, this);
}
if (numMaxDelayRetries && (numMaxDelayRetries < 0 || !Number.isInteger(numMaxDelayRetries))) {
throw new Error(`numMaxDelayRetries must be an integer zero or greater, got: ${numMaxDelayRetries}`);
throw new ValidationError(`numMaxDelayRetries must be an integer zero or greater, got: ${numMaxDelayRetries}`, this);
}
}
if (throttlePolicy) {
const maxReceivesPerSecond = throttlePolicy.maxReceivesPerSecond;
if (maxReceivesPerSecond !== undefined && (maxReceivesPerSecond < 1 || !Number.isInteger(maxReceivesPerSecond))) {
throw new Error(`maxReceivesPerSecond must be an integer greater than zero, got: ${maxReceivesPerSecond}`);
throw new ValidationError(`maxReceivesPerSecond must be an integer greater than zero, got: ${maxReceivesPerSecond}`, this);
}
}
return {
Expand Down Expand Up @@ -320,6 +321,7 @@ export enum SubscriptionProtocol {
}

function buildFilterPolicyWithMessageBody(
scope: Construct,
inputObject: { [key: string]: FilterOrPolicy },
depth = 1,
totalCombinationValues = [1],
Expand All @@ -328,7 +330,7 @@ function buildFilterPolicyWithMessageBody(

for (const [key, filterOrPolicy] of Object.entries(inputObject)) {
if (filterOrPolicy.isPolicy()) {
result[key] = buildFilterPolicyWithMessageBody(filterOrPolicy.policyDoc, depth + 1, totalCombinationValues);
result[key] = buildFilterPolicyWithMessageBody(scope, filterOrPolicy.policyDoc, depth + 1, totalCombinationValues);
} else if (filterOrPolicy.isFilter()) {
const filter = filterOrPolicy.filterDoc.conditions;
result[key] = filter;
Expand All @@ -338,7 +340,7 @@ function buildFilterPolicyWithMessageBody(

// https://docs.aws.amazon.com/sns/latest/dg/subscription-filter-policy-constraints.html
if (totalCombinationValues[0] > 150) {
throw new Error(`The total combination of values (${totalCombinationValues}) must not exceed 150.`);
throw new ValidationError(`The total combination of values (${totalCombinationValues}) must not exceed 150.`, scope);
}

return result;
Expand Down
3 changes: 2 additions & 1 deletion packages/aws-cdk-lib/aws-sns/lib/topic-base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { Subscription } from './subscription';
import * as notifications from '../../aws-codestarnotifications';
import * as iam from '../../aws-iam';
import { IResource, Resource, ResourceProps, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Represents an SNS topic
Expand Down Expand Up @@ -111,7 +112,7 @@ export abstract class TopicBase extends Resource implements ITopic {
// We use the subscriber's id as the construct id. There's no meaning
// to subscribing the same subscriber twice on the same topic.
if (scope.node.tryFindChild(id)) {
throw new Error(`A subscription with id "${id}" already exists under the scope ${scope.node.path}`);
throw new ValidationError(`A subscription with id "${id}" already exists under the scope ${scope.node.path}`, scope);
}

const subscription = new Subscription(scope, id, {
Expand Down
23 changes: 12 additions & 11 deletions packages/aws-cdk-lib/aws-sns/lib/topic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { ITopic, TopicBase } from './topic-base';
import { IRole } from '../../aws-iam';
import { IKey } from '../../aws-kms';
import { ArnFormat, Lazy, Names, Stack, Token } from '../../core';
import { ValidationError } from '../../core/lib/errors';

/**
* Properties for a new SNS topic
Expand Down Expand Up @@ -226,7 +227,7 @@ export class Topic extends TopicBase {
const fifo = topicName.endsWith('.fifo');

if (attrs.contentBasedDeduplication && !fifo) {
throw new Error('Cannot import topic; contentBasedDeduplication is only available for FIFO SNS topics.');
throw new ValidationError('Cannot import topic; contentBasedDeduplication is only available for FIFO SNS topics.', scope);
}

class Import extends TopicBase {
Expand Down Expand Up @@ -259,17 +260,17 @@ export class Topic extends TopicBase {
this.enforceSSL = props.enforceSSL;

if (props.contentBasedDeduplication && !props.fifo) {
throw new Error('Content based deduplication can only be enabled for FIFO SNS topics.');
throw new ValidationError('Content based deduplication can only be enabled for FIFO SNS topics.', this);
}
if (props.messageRetentionPeriodInDays && !props.fifo) {
throw new Error('`messageRetentionPeriodInDays` is only valid for FIFO SNS topics.');
throw new ValidationError('`messageRetentionPeriodInDays` is only valid for FIFO SNS topics.', this);
}
if (
props.messageRetentionPeriodInDays !== undefined
&& !Token.isUnresolved(props.messageRetentionPeriodInDays)
&& (!Number.isInteger(props.messageRetentionPeriodInDays) || props.messageRetentionPeriodInDays > 365 || props.messageRetentionPeriodInDays < 1)
) {
throw new Error('`messageRetentionPeriodInDays` must be an integer between 1 and 365');
throw new ValidationError('`messageRetentionPeriodInDays` must be an integer between 1 and 365', this);
}

if (props.loggingConfigs) {
Expand All @@ -296,11 +297,11 @@ export class Topic extends TopicBase {
props.signatureVersion !== '1' &&
props.signatureVersion !== '2'
) {
throw new Error(`signatureVersion must be "1" or "2", received: "${props.signatureVersion}"`);
throw new ValidationError(`signatureVersion must be "1" or "2", received: "${props.signatureVersion}"`, this);
}

if (props.displayName && !Token.isUnresolved(props.displayName) && props.displayName.length > 100) {
throw new Error(`displayName must be less than or equal to 100 characters, got ${props.displayName.length}`);
throw new ValidationError(`displayName must be less than or equal to 100 characters, got ${props.displayName.length}`, this);
}

const resource = new CfnTopic(this, 'Resource', {
Expand All @@ -327,13 +328,11 @@ export class Topic extends TopicBase {
}

private renderLoggingConfigs(): CfnTopic.LoggingConfigProperty[] {
return this.loggingConfigs.map(renderLoggingConfig);

function renderLoggingConfig(spec: LoggingConfig): CfnTopic.LoggingConfigProperty {
const renderLoggingConfig = (spec: LoggingConfig): CfnTopic.LoggingConfigProperty => {
if (spec.successFeedbackSampleRate !== undefined) {
const rate = spec.successFeedbackSampleRate;
if (!Number.isInteger(rate) || rate < 0 || rate > 100) {
throw new Error('Success feedback sample rate must be an integer between 0 and 100');
throw new ValidationError('Success feedback sample rate must be an integer between 0 and 100', this);
}
}
return {
Expand All @@ -342,7 +341,9 @@ export class Topic extends TopicBase {
successFeedbackRoleArn: spec.successFeedbackRole?.roleArn,
successFeedbackSampleRate: spec.successFeedbackSampleRate?.toString(),
};
}
};

return this.loggingConfigs.map(renderLoggingConfig);
}

/**
Expand Down

0 comments on commit 7452462

Please sign in to comment.