Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sns): throw ValidationError instead of untyped errors #33045

Merged
merged 5 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-cdk-lib/.eslintrc.js
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
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
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
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
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
Loading