Skip to content

Commit 5cfea39

Browse files
mrgrainmrgrain
authored
feat(ecs): throw ValidationErrors instead of untyped errors (aws#34427)
### Issue Relates to aws#32569 ### Reason for this change untyped Errors are not recommended ### Description of changes `ValidationError`s everywhere ### Describe any new or updated permissions being added None ### 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*
1 parent bc7c4e8 commit 5cfea39

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+299
-288
lines changed

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

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ baseConfig.rules['import/no-extraneous-dependencies'] = [
1717
baseConfig.rules['@cdklabs/no-throw-default-error'] = ['error'];
1818
// not yet supported
1919
const noThrowDefaultErrorNotYetSupported = [
20-
'aws-ecs-patterns',
21-
'aws-ecs',
2220
'aws-globalaccelerator',
2321
'aws-globalaccelerator-endpoints',
2422
'aws-iam',

‎packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-load-balanced-service-base.ts‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ import {
1414
import { IRole } from '../../../aws-iam';
1515
import { ARecord, IHostedZone, RecordTarget, CnameRecord } from '../../../aws-route53';
1616
import { LoadBalancerTarget } from '../../../aws-route53-targets';
17-
import * as cdk from '../../../core';
18-
import { Duration } from '../../../core';
17+
import { CfnOutput, Duration, Stack, Token, ValidationError } from '../../../core';
1918

2019
/**
2120
* Describes the type of DNS record the service should create
@@ -150,7 +149,7 @@ export interface ApplicationLoadBalancedServiceBaseProps {
150149
*
151150
* @default - defaults to 60 seconds if at least one load balancer is in-use and it is not already set
152151
*/
153-
readonly healthCheckGracePeriod?: cdk.Duration;
152+
readonly healthCheckGracePeriod?: Duration;
154153

155154
/**
156155
* The maximum number of tasks, specified as a percentage of the Amazon ECS
@@ -423,7 +422,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
423422
*/
424423
public get loadBalancer(): ApplicationLoadBalancer {
425424
if (!this._applicationLoadBalancer) {
426-
throw new Error('.loadBalancer can only be accessed if the class was constructed with an owned, not imported, load balancer');
425+
throw new ValidationError('.loadBalancer can only be accessed if the class was constructed with an owned, not imported, load balancer', this);
427426
}
428427
return this._applicationLoadBalancer;
429428
}
@@ -462,12 +461,12 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
462461
super(scope, id);
463462

464463
if (props.cluster && props.vpc) {
465-
throw new Error('You can only specify either vpc or cluster. Alternatively, you can leave both blank');
464+
throw new ValidationError('You can only specify either vpc or cluster. Alternatively, you can leave both blank', this);
466465
}
467466
this.cluster = props.cluster || this.getDefaultCluster(this, props.vpc);
468467

469-
if (props.desiredCount !== undefined && !cdk.Token.isUnresolved(props.desiredCount) && props.desiredCount < 1) {
470-
throw new Error('You must specify a desiredCount greater than 0');
468+
if (props.desiredCount !== undefined && !Token.isUnresolved(props.desiredCount) && props.desiredCount < 1) {
469+
throw new ValidationError('You must specify a desiredCount greater than 0', this);
471470
}
472471

473472
this.desiredCount = props.desiredCount || 1;
@@ -478,7 +477,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
478477
if (props.idleTimeout) {
479478
const idleTimeout = props.idleTimeout.toSeconds();
480479
if (idleTimeout > Duration.seconds(4000).toSeconds() || idleTimeout < Duration.seconds(1).toSeconds()) {
481-
throw new Error('Load balancer idle timeout must be between 1 and 4000 seconds.');
480+
throw new ValidationError('Load balancer idle timeout must be between 1 and 4000 seconds.', this);
482481
}
483482
}
484483

@@ -493,12 +492,12 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
493492
const loadBalancer = props.loadBalancer ?? new ApplicationLoadBalancer(this, 'LB', lbProps);
494493

495494
if (props.certificate !== undefined && props.protocol !== undefined && props.protocol !== ApplicationProtocol.HTTPS) {
496-
throw new Error('The HTTPS protocol must be used when a certificate is given');
495+
throw new ValidationError('The HTTPS protocol must be used when a certificate is given', this);
497496
}
498497
const protocol = props.protocol ?? (props.certificate ? ApplicationProtocol.HTTPS : ApplicationProtocol.HTTP);
499498

500499
if (protocol !== ApplicationProtocol.HTTPS && props.redirectHTTP === true) {
501-
throw new Error('The HTTPS protocol must be used when redirecting HTTP traffic');
500+
throw new ValidationError('The HTTPS protocol must be used when redirecting HTTP traffic', this);
502501
}
503502

504503
const targetProps: AddApplicationTargetsProps = {
@@ -519,7 +518,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
519518
this.certificate = props.certificate;
520519
} else {
521520
if (typeof props.domainName === 'undefined' || typeof props.domainZone === 'undefined') {
522-
throw new Error('A domain name and zone is required when using the HTTPS protocol');
521+
throw new ValidationError('A domain name and zone is required when using the HTTPS protocol', this);
523522
}
524523

525524
this.certificate = new Certificate(this, 'Certificate', {
@@ -547,7 +546,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
547546
let domainName = loadBalancer.loadBalancerDnsName;
548547
if (typeof props.domainName !== 'undefined') {
549548
if (typeof props.domainZone === 'undefined') {
550-
throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
549+
throw new ValidationError('A Route53 hosted domain zone name is required to configure the specified domain name', this);
551550
}
552551

553552
switch (props.recordType ?? ApplicationLoadBalancedServiceRecordType.ALIAS) {
@@ -577,8 +576,8 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
577576
this._applicationLoadBalancer = loadBalancer;
578577
}
579578

580-
new cdk.CfnOutput(this, 'LoadBalancerDNS', { value: loadBalancer.loadBalancerDnsName });
581-
new cdk.CfnOutput(this, 'ServiceURL', { value: protocol.toLowerCase() + '://' + domainName });
579+
new CfnOutput(this, 'LoadBalancerDNS', { value: loadBalancer.loadBalancerDnsName });
580+
new CfnOutput(this, 'ServiceURL', { value: protocol.toLowerCase() + '://' + domainName });
582581
}
583582

584583
/**
@@ -587,7 +586,7 @@ export abstract class ApplicationLoadBalancedServiceBase extends Construct {
587586
protected getDefaultCluster(scope: Construct, vpc?: IVpc): Cluster {
588587
// magic string to avoid collision with user-defined constructs
589588
const DEFAULT_CLUSTER_ID = `EcsDefaultClusterMnL3mNNYN${vpc ? vpc.node.id : ''}`;
590-
const stack = cdk.Stack.of(scope);
589+
const stack = Stack.of(scope);
591590
return stack.node.tryFindChild(DEFAULT_CLUSTER_ID) as Cluster || new Cluster(stack, DEFAULT_CLUSTER_ID, { vpc });
592591
}
593592

‎packages/aws-cdk-lib/aws-ecs-patterns/lib/base/application-multiple-target-groups-service-base.ts‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
import { IRole } from '../../../aws-iam';
1717
import { ARecord, IHostedZone, RecordTarget } from '../../../aws-route53';
1818
import { LoadBalancerTarget } from '../../../aws-route53-targets';
19-
import { CfnOutput, Duration, Stack } from '../../../core';
19+
import { CfnOutput, Duration, Stack, ValidationError } from '../../../core';
2020

2121
/**
2222
* The properties for the base ApplicationMultipleTargetGroupsEc2Service or ApplicationMultipleTargetGroupsFargateService service.
@@ -431,7 +431,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
431431
for (const listenerProps of lbProps.listeners) {
432432
const protocol = this.createListenerProtocol(listenerProps.protocol, listenerProps.certificate);
433433
if (listenerProps.certificate !== undefined && protocol !== undefined && protocol !== ApplicationProtocol.HTTPS) {
434-
throw new Error('The HTTPS protocol must be used when a certificate is given');
434+
throw new ValidationError('The HTTPS protocol must be used when a certificate is given', this);
435435
}
436436
protocolType.add(protocol);
437437
const listener = this.configListener(protocol, {
@@ -491,7 +491,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
491491
return listener;
492492
}
493493
}
494-
throw new Error(`Listener ${name} is not defined. Did you define listener with name ${name}?`);
494+
throw new ValidationError(`Listener ${name} is not defined. Did you define listener with name ${name}?`, this);
495495
}
496496

497497
protected registerECSTargets(service: BaseService, container: ContainerDefinition, targets: ApplicationTargetProps[]): ApplicationTargetGroup {
@@ -519,7 +519,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
519519
this.targetGroups.push(targetGroup);
520520
}
521521
if (this.targetGroups.length === 0) {
522-
throw new Error('At least one target group should be specified.');
522+
throw new ValidationError('At least one target group should be specified.', this);
523523
}
524524
return this.targetGroups[0];
525525
}
@@ -561,20 +561,20 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
561561

562562
private validateInput(props: ApplicationMultipleTargetGroupsServiceBaseProps) {
563563
if (props.cluster && props.vpc) {
564-
throw new Error('You can only specify either vpc or cluster. Alternatively, you can leave both blank');
564+
throw new ValidationError('You can only specify either vpc or cluster. Alternatively, you can leave both blank', this);
565565
}
566566

567567
if (props.desiredCount !== undefined && props.desiredCount < 1) {
568-
throw new Error('You must specify a desiredCount greater than 0');
568+
throw new ValidationError('You must specify a desiredCount greater than 0', this);
569569
}
570570

571571
if (props.loadBalancers) {
572572
if (props.loadBalancers.length === 0) {
573-
throw new Error('At least one load balancer must be specified');
573+
throw new ValidationError('At least one load balancer must be specified', this);
574574
}
575575
for (const lbProps of props.loadBalancers) {
576576
if (lbProps.listeners.length === 0) {
577-
throw new Error('At least one listener must be specified');
577+
throw new ValidationError('At least one listener must be specified', this);
578578
}
579579
}
580580
}
@@ -585,7 +585,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
585585
if (prop.idleTimeout) {
586586
const idleTimeout = prop.idleTimeout.toSeconds();
587587
if (idleTimeout > Duration.seconds(4000).toSeconds() || idleTimeout < Duration.seconds(1).toSeconds()) {
588-
throw new Error('Load balancer idle timeout must be between 1 and 4000 seconds.');
588+
throw new ValidationError('Load balancer idle timeout must be between 1 and 4000 seconds.', this);
589589
}
590590
}
591591
}
@@ -609,7 +609,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
609609

610610
private createListenerCertificate(listenerName: string, certificate?: ICertificate, domainName?: string, domainZone?: IHostedZone): ICertificate {
611611
if (typeof domainName === 'undefined' || typeof domainZone === 'undefined') {
612-
throw new Error('A domain name and zone is required when using the HTTPS protocol');
612+
throw new ValidationError('A domain name and zone is required when using the HTTPS protocol', this);
613613
}
614614

615615
if (certificate !== undefined) {
@@ -635,7 +635,7 @@ export abstract class ApplicationMultipleTargetGroupsServiceBase extends Constru
635635
let domainName = loadBalancer.loadBalancerDnsName;
636636
if (typeof name !== 'undefined') {
637637
if (typeof zone === 'undefined') {
638-
throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
638+
throw new ValidationError('A Route53 hosted domain zone name is required to configure the specified domain name', this);
639639
}
640640

641641
const record = new ARecord(this, `DNS${loadBalancer.node.id}`, {

‎packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-load-balanced-service-base.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { IRole } from '../../../aws-iam';
99
import { ARecord, CnameRecord, IHostedZone, RecordTarget } from '../../../aws-route53';
1010
import { LoadBalancerTarget } from '../../../aws-route53-targets';
1111
import * as cdk from '../../../core';
12+
import { ValidationError } from '../../../core';
1213

1314
/**
1415
* Describes the type of DNS record the service should create
@@ -329,7 +330,7 @@ export abstract class NetworkLoadBalancedServiceBase extends Construct {
329330
*/
330331
public get loadBalancer(): NetworkLoadBalancer {
331332
if (!this._networkLoadBalancer) {
332-
throw new Error('.loadBalancer can only be accessed if the class was constructed with an owned, not imported, load balancer');
333+
throw new ValidationError('.loadBalancer can only be accessed if the class was constructed with an owned, not imported, load balancer', this);
333334
}
334335
return this._networkLoadBalancer;
335336
}
@@ -357,12 +358,12 @@ export abstract class NetworkLoadBalancedServiceBase extends Construct {
357358
super(scope, id);
358359

359360
if (props.cluster && props.vpc) {
360-
throw new Error('You can only specify either vpc or cluster. Alternatively, you can leave both blank');
361+
throw new ValidationError('You can only specify either vpc or cluster. Alternatively, you can leave both blank', this);
361362
}
362363
this.cluster = props.cluster || this.getDefaultCluster(this, props.vpc);
363364

364365
if (props.desiredCount !== undefined && props.desiredCount < 1) {
365-
throw new Error('You must specify a desiredCount greater than 0');
366+
throw new ValidationError('You must specify a desiredCount greater than 0', this);
366367
}
367368

368369
this.desiredCount = props.desiredCount || 1;
@@ -392,7 +393,7 @@ export abstract class NetworkLoadBalancedServiceBase extends Construct {
392393

393394
if (typeof props.domainName !== 'undefined') {
394395
if (typeof props.domainZone === 'undefined') {
395-
throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
396+
throw new ValidationError('A Route53 hosted domain zone name is required to configure the specified domain name', this);
396397
}
397398

398399
switch (props.recordType ?? NetworkLoadBalancedServiceRecordType.ALIAS) {

‎packages/aws-cdk-lib/aws-ecs-patterns/lib/base/network-multiple-target-groups-service-base.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { NetworkListener, NetworkLoadBalancer, NetworkTargetGroup } from '../../
88
import { IRole } from '../../../aws-iam';
99
import { ARecord, IHostedZone, RecordTarget } from '../../../aws-route53';
1010
import { LoadBalancerTarget } from '../../../aws-route53-targets';
11-
import { CfnOutput, Duration, Stack } from '../../../core';
11+
import { CfnOutput, Duration, Stack, ValidationError } from '../../../core';
1212

1313
/**
1414
* The properties for the base NetworkMultipleTargetGroupsEc2Service or NetworkMultipleTargetGroupsFargateService service.
@@ -380,7 +380,7 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
380380
return listener;
381381
}
382382
}
383-
throw new Error(`Listener ${name} is not defined. Did you define listener with name ${name}?`);
383+
throw new ValidationError(`Listener ${name} is not defined. Did you define listener with name ${name}?`, this);
384384
}
385385

386386
protected registerECSTargets(service: BaseService, container: ContainerDefinition, targets: NetworkTargetProps[]): NetworkTargetGroup {
@@ -397,7 +397,7 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
397397
this.targetGroups.push(targetGroup);
398398
}
399399
if (this.targetGroups.length === 0) {
400-
throw new Error('At least one target group should be specified.');
400+
throw new ValidationError('At least one target group should be specified.', this);
401401
}
402402
return this.targetGroups[0];
403403
}
@@ -423,20 +423,20 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
423423

424424
private validateInput(props: NetworkMultipleTargetGroupsServiceBaseProps) {
425425
if (props.cluster && props.vpc) {
426-
throw new Error('You can only specify either vpc or cluster. Alternatively, you can leave both blank');
426+
throw new ValidationError('You can only specify either vpc or cluster. Alternatively, you can leave both blank', this);
427427
}
428428

429429
if (props.desiredCount !== undefined && props.desiredCount < 1) {
430-
throw new Error('You must specify a desiredCount greater than 0');
430+
throw new ValidationError('You must specify a desiredCount greater than 0', this);
431431
}
432432

433433
if (props.loadBalancers) {
434434
if (props.loadBalancers.length === 0) {
435-
throw new Error('At least one load balancer must be specified');
435+
throw new ValidationError('At least one load balancer must be specified', this);
436436
}
437437
for (const lbProps of props.loadBalancers) {
438438
if (lbProps.listeners.length === 0) {
439-
throw new Error('At least one listener must be specified');
439+
throw new ValidationError('At least one listener must be specified', this);
440440
}
441441
}
442442
}
@@ -461,7 +461,7 @@ export abstract class NetworkMultipleTargetGroupsServiceBase extends Construct {
461461
private createDomainName(loadBalancer: NetworkLoadBalancer, name?: string, zone?: IHostedZone) {
462462
if (typeof name !== 'undefined') {
463463
if (typeof zone === 'undefined') {
464-
throw new Error('A Route53 hosted domain zone name is required to configure the specified domain name');
464+
throw new ValidationError('A Route53 hosted domain zone name is required to configure the specified domain name', this);
465465
}
466466

467467
new ARecord(this, `DNS${loadBalancer.node.id}`, {

‎packages/aws-cdk-lib/aws-ecs-patterns/lib/base/queue-processing-service-base.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import {
66
ICluster, LogDriver, PropagatedTagSource, Secret,
77
} from '../../../aws-ecs';
88
import { IQueue, Queue } from '../../../aws-sqs';
9-
import { CfnOutput, Duration, FeatureFlags, Stack } from '../../../core';
9+
import { CfnOutput, Duration, FeatureFlags, Stack, ValidationError } from '../../../core';
1010
import * as cxapi from '../../../cx-api';
1111

1212
/**
@@ -335,13 +335,13 @@ export abstract class QueueProcessingServiceBase extends Construct {
335335
super(scope, id);
336336

337337
if (props.cluster && props.vpc) {
338-
throw new Error('You can only specify either vpc or cluster. Alternatively, you can leave both blank');
338+
throw new ValidationError('You can only specify either vpc or cluster. Alternatively, you can leave both blank', this);
339339
}
340340
this.cluster = props.cluster || this.getDefaultCluster(this, props.vpc);
341341

342342
if (props.queue && (props.retentionPeriod || props.visibilityTimeout || props.maxReceiveCount)) {
343343
const errorProps = ['retentionPeriod', 'visibilityTimeout', 'maxReceiveCount'].filter(prop => props.hasOwnProperty(prop));
344-
throw new Error(`${errorProps.join(', ')} can be set only when queue is not set. Specify them in the QueueProps of the queue`);
344+
throw new ValidationError(`${errorProps.join(', ')} can be set only when queue is not set. Specify them in the QueueProps of the queue`, this);
345345
}
346346
// Create the SQS queue and it's corresponding DLQ if one is not provided
347347
if (props.queue) {
@@ -367,7 +367,7 @@ export abstract class QueueProcessingServiceBase extends Construct {
367367
this.scalingSteps = props.scalingSteps ?? defaultScalingSteps;
368368

369369
if (props.cooldown && props.cooldown.toSeconds() > 999999999) {
370-
throw new Error(`cooldown cannot be more than 999999999, found: ${props.cooldown.toSeconds()}`);
370+
throw new ValidationError(`cooldown cannot be more than 999999999, found: ${props.cooldown.toSeconds()}`, this);
371371
}
372372
this.cooldown = props.cooldown;
373373

@@ -398,7 +398,7 @@ export abstract class QueueProcessingServiceBase extends Construct {
398398
}
399399

400400
if (!this.desiredCount && !this.maxCapacity) {
401-
throw new Error('maxScalingCapacity must be set and greater than 0 if desiredCount is 0');
401+
throw new ValidationError('maxScalingCapacity must be set and greater than 0 if desiredCount is 0', this);
402402
}
403403

404404
new CfnOutput(this, 'SQSQueue', { value: this.sqsQueue.queueName });

0 commit comments

Comments
 (0)