Skip to content

Commit 7452462

Browse files
authored
feat(sns): throw ValidationError instead of untyped errors (#33045)
### 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*
1 parent 0b2db62 commit 7452462

File tree

5 files changed

+41
-35
lines changed

5 files changed

+41
-35
lines changed

packages/aws-cdk-lib/.eslintrc.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [
1515

1616

1717
// no-throw-default-error
18-
const modules = ['aws-s3', 'aws-lambda', 'aws-rds'];
18+
const modules = ['aws-s3', 'aws-lambda', 'aws-rds', 'aws-sns'];
1919
baseConfig.overrides.push({
2020
files: modules.map(m => `./${m}/lib/**`),
2121
rules: { "@cdklabs/no-throw-default-error": ['error'] },

packages/aws-cdk-lib/aws-sns/lib/subscription-filter.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { UnscopedValidationError } from '../../core/lib/errors';
2+
13
/**
24
* Conditions that can be applied to string attributes.
35
*/
@@ -131,10 +133,10 @@ export class SubscriptionFilter {
131133
const conditions = new Array<any>();
132134

133135
if (stringConditions.whitelist && stringConditions.allowlist) {
134-
throw new Error('`whitelist` is deprecated; please use `allowlist` instead');
136+
throw new UnscopedValidationError('`whitelist` is deprecated; please use `allowlist` instead');
135137
}
136138
if (stringConditions.blacklist && stringConditions.denylist) {
137-
throw new Error('`blacklist` is deprecated; please use `denylist` instead');
139+
throw new UnscopedValidationError('`blacklist` is deprecated; please use `denylist` instead');
138140
}
139141
const allowlist = stringConditions.allowlist ?? stringConditions.whitelist;
140142
const denylist = stringConditions.denylist ?? stringConditions.blacklist;
@@ -165,7 +167,7 @@ export class SubscriptionFilter {
165167
const conditions = new Array<any>();
166168

167169
if (numericConditions.whitelist && numericConditions.allowlist) {
168-
throw new Error('`whitelist` is deprecated; please use `allowlist` instead');
170+
throw new UnscopedValidationError('`whitelist` is deprecated; please use `allowlist` instead');
169171
}
170172
const allowlist = numericConditions.allowlist ?? numericConditions.whitelist;
171173

packages/aws-cdk-lib/aws-sns/lib/subscription.ts

+21-19
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ITopic } from './topic-base';
66
import { PolicyStatement, ServicePrincipal } from '../../aws-iam';
77
import { IQueue } from '../../aws-sqs';
88
import { Resource } from '../../core';
9+
import { ValidationError } from '../../core/lib/errors';
910

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

120121
if (props.filterPolicy) {
121122
if (Object.keys(props.filterPolicy).length > 5) {
122-
throw new Error('A filter policy can have a maximum of 5 attribute names.');
123+
throw new ValidationError('A filter policy can have a maximum of 5 attribute names.', this);
123124
}
124125

125126
this.filterPolicy = Object.entries(props.filterPolicy)
@@ -131,22 +132,22 @@ export class Subscription extends Resource {
131132
let total = 1;
132133
Object.values(this.filterPolicy).forEach(filter => { total *= filter.length; });
133134
if (total > 150) {
134-
throw new Error(`The total combination of values (${total}) must not exceed 150.`);
135+
throw new ValidationError(`The total combination of values (${total}) must not exceed 150.`, this);
135136
}
136137
} else if (props.filterPolicyWithMessageBody) {
137138
if (Object.keys(props.filterPolicyWithMessageBody).length > 5) {
138-
throw new Error('A filter policy can have a maximum of 5 attribute names.');
139+
throw new ValidationError('A filter policy can have a maximum of 5 attribute names.', this);
139140
}
140141
this.filterPolicyWithMessageBody = props.filterPolicyWithMessageBody;
141142
}
142143

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

147148
// Format filter policy
148149
const filterPolicy = this.filterPolicyWithMessageBody
149-
? buildFilterPolicyWithMessageBody(this.filterPolicyWithMessageBody)
150+
? buildFilterPolicyWithMessageBody(this, this.filterPolicyWithMessageBody)
150151
: this.filterPolicy;
151152

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

168169
private renderDeliveryPolicy(deliveryPolicy: DeliveryPolicy, protocol: SubscriptionProtocol): any {
169170
if (![SubscriptionProtocol.HTTP, SubscriptionProtocol.HTTPS].includes(protocol)) {
170-
throw new Error(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`);
171+
throw new ValidationError(`Delivery policy is only supported for HTTP and HTTPS subscriptions, got: ${protocol}`, this);
171172
}
172173
const { healthyRetryPolicy, throttlePolicy } = deliveryPolicy;
173174
if (healthyRetryPolicy) {
@@ -176,45 +177,45 @@ export class Subscription extends Resource {
176177
const maxDelayTarget = healthyRetryPolicy.maxDelayTarget;
177178
if (minDelayTarget !== undefined) {
178179
if (minDelayTarget.toMilliseconds() % 1000 !== 0) {
179-
throw new Error(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`);
180+
throw new ValidationError(`minDelayTarget must be a whole number of seconds, got: ${minDelayTarget}`, this);
180181
}
181182
const minDelayTargetSecs = minDelayTarget.toSeconds();
182183
if (minDelayTargetSecs < 1 || minDelayTargetSecs > delayTargetLimitSecs) {
183-
throw new Error(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${minDelayTargetSecs}s`);
184+
throw new ValidationError(`minDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${minDelayTargetSecs}s`, this);
184185
}
185186
}
186187
if (maxDelayTarget !== undefined) {
187188
if (maxDelayTarget.toMilliseconds() % 1000 !== 0) {
188-
throw new Error(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`);
189+
throw new ValidationError(`maxDelayTarget must be a whole number of seconds, got: ${maxDelayTarget}`, this);
189190
}
190191
const maxDelayTargetSecs = maxDelayTarget.toSeconds();
191192
if (maxDelayTargetSecs < 1 || maxDelayTargetSecs > delayTargetLimitSecs) {
192-
throw new Error(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${maxDelayTargetSecs}s`);
193+
throw new ValidationError(`maxDelayTarget must be between 1 and ${delayTargetLimitSecs} seconds inclusive, got: ${maxDelayTargetSecs}s`, this);
193194
}
194195
if ((minDelayTarget !== undefined) && minDelayTarget.toSeconds() > maxDelayTargetSecs) {
195-
throw new Error('minDelayTarget must not exceed maxDelayTarget');
196+
throw new ValidationError('minDelayTarget must not exceed maxDelayTarget', this);
196197
}
197198
}
198199

199200
const numRetriesLimit = 100;
200201
if (healthyRetryPolicy.numRetries && (healthyRetryPolicy.numRetries < 0 || healthyRetryPolicy.numRetries > numRetriesLimit)) {
201-
throw new Error(`numRetries must be between 0 and ${numRetriesLimit} inclusive, got: ${healthyRetryPolicy.numRetries}`);
202+
throw new ValidationError(`numRetries must be between 0 and ${numRetriesLimit} inclusive, got: ${healthyRetryPolicy.numRetries}`, this);
202203
}
203204
const { numNoDelayRetries, numMinDelayRetries, numMaxDelayRetries } = healthyRetryPolicy;
204205
if (numNoDelayRetries && (numNoDelayRetries < 0 || !Number.isInteger(numNoDelayRetries))) {
205-
throw new Error(`numNoDelayRetries must be an integer zero or greater, got: ${numNoDelayRetries}`);
206+
throw new ValidationError(`numNoDelayRetries must be an integer zero or greater, got: ${numNoDelayRetries}`, this);
206207
}
207208
if (numMinDelayRetries && (numMinDelayRetries < 0 || !Number.isInteger(numMinDelayRetries))) {
208-
throw new Error(`numMinDelayRetries must be an integer zero or greater, got: ${numMinDelayRetries}`);
209+
throw new ValidationError(`numMinDelayRetries must be an integer zero or greater, got: ${numMinDelayRetries}`, this);
209210
}
210211
if (numMaxDelayRetries && (numMaxDelayRetries < 0 || !Number.isInteger(numMaxDelayRetries))) {
211-
throw new Error(`numMaxDelayRetries must be an integer zero or greater, got: ${numMaxDelayRetries}`);
212+
throw new ValidationError(`numMaxDelayRetries must be an integer zero or greater, got: ${numMaxDelayRetries}`, this);
212213
}
213214
}
214215
if (throttlePolicy) {
215216
const maxReceivesPerSecond = throttlePolicy.maxReceivesPerSecond;
216217
if (maxReceivesPerSecond !== undefined && (maxReceivesPerSecond < 1 || !Number.isInteger(maxReceivesPerSecond))) {
217-
throw new Error(`maxReceivesPerSecond must be an integer greater than zero, got: ${maxReceivesPerSecond}`);
218+
throw new ValidationError(`maxReceivesPerSecond must be an integer greater than zero, got: ${maxReceivesPerSecond}`, this);
218219
}
219220
}
220221
return {
@@ -320,6 +321,7 @@ export enum SubscriptionProtocol {
320321
}
321322

322323
function buildFilterPolicyWithMessageBody(
324+
scope: Construct,
323325
inputObject: { [key: string]: FilterOrPolicy },
324326
depth = 1,
325327
totalCombinationValues = [1],
@@ -328,7 +330,7 @@ function buildFilterPolicyWithMessageBody(
328330

329331
for (const [key, filterOrPolicy] of Object.entries(inputObject)) {
330332
if (filterOrPolicy.isPolicy()) {
331-
result[key] = buildFilterPolicyWithMessageBody(filterOrPolicy.policyDoc, depth + 1, totalCombinationValues);
333+
result[key] = buildFilterPolicyWithMessageBody(scope, filterOrPolicy.policyDoc, depth + 1, totalCombinationValues);
332334
} else if (filterOrPolicy.isFilter()) {
333335
const filter = filterOrPolicy.filterDoc.conditions;
334336
result[key] = filter;
@@ -338,7 +340,7 @@ function buildFilterPolicyWithMessageBody(
338340

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

344346
return result;

packages/aws-cdk-lib/aws-sns/lib/topic-base.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Subscription } from './subscription';
66
import * as notifications from '../../aws-codestarnotifications';
77
import * as iam from '../../aws-iam';
88
import { IResource, Resource, ResourceProps, Token } from '../../core';
9+
import { ValidationError } from '../../core/lib/errors';
910

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

117118
const subscription = new Subscription(scope, id, {

packages/aws-cdk-lib/aws-sns/lib/topic.ts

+12-11
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { ITopic, TopicBase } from './topic-base';
44
import { IRole } from '../../aws-iam';
55
import { IKey } from '../../aws-kms';
66
import { ArnFormat, Lazy, Names, Stack, Token } from '../../core';
7+
import { ValidationError } from '../../core/lib/errors';
78

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

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

232233
class Import extends TopicBase {
@@ -259,17 +260,17 @@ export class Topic extends TopicBase {
259260
this.enforceSSL = props.enforceSSL;
260261

261262
if (props.contentBasedDeduplication && !props.fifo) {
262-
throw new Error('Content based deduplication can only be enabled for FIFO SNS topics.');
263+
throw new ValidationError('Content based deduplication can only be enabled for FIFO SNS topics.', this);
263264
}
264265
if (props.messageRetentionPeriodInDays && !props.fifo) {
265-
throw new Error('`messageRetentionPeriodInDays` is only valid for FIFO SNS topics.');
266+
throw new ValidationError('`messageRetentionPeriodInDays` is only valid for FIFO SNS topics.', this);
266267
}
267268
if (
268269
props.messageRetentionPeriodInDays !== undefined
269270
&& !Token.isUnresolved(props.messageRetentionPeriodInDays)
270271
&& (!Number.isInteger(props.messageRetentionPeriodInDays) || props.messageRetentionPeriodInDays > 365 || props.messageRetentionPeriodInDays < 1)
271272
) {
272-
throw new Error('`messageRetentionPeriodInDays` must be an integer between 1 and 365');
273+
throw new ValidationError('`messageRetentionPeriodInDays` must be an integer between 1 and 365', this);
273274
}
274275

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

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

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

329330
private renderLoggingConfigs(): CfnTopic.LoggingConfigProperty[] {
330-
return this.loggingConfigs.map(renderLoggingConfig);
331-
332-
function renderLoggingConfig(spec: LoggingConfig): CfnTopic.LoggingConfigProperty {
331+
const renderLoggingConfig = (spec: LoggingConfig): CfnTopic.LoggingConfigProperty => {
333332
if (spec.successFeedbackSampleRate !== undefined) {
334333
const rate = spec.successFeedbackSampleRate;
335334
if (!Number.isInteger(rate) || rate < 0 || rate > 100) {
336-
throw new Error('Success feedback sample rate must be an integer between 0 and 100');
335+
throw new ValidationError('Success feedback sample rate must be an integer between 0 and 100', this);
337336
}
338337
}
339338
return {
@@ -342,7 +341,9 @@ export class Topic extends TopicBase {
342341
successFeedbackRoleArn: spec.successFeedbackRole?.roleArn,
343342
successFeedbackSampleRate: spec.successFeedbackSampleRate?.toString(),
344343
};
345-
}
344+
};
345+
346+
return this.loggingConfigs.map(renderLoggingConfig);
346347
}
347348

348349
/**

0 commit comments

Comments
 (0)