From 21804fd26d70fd3b1b56f13c8e8cdd901c779f54 Mon Sep 17 00:00:00 2001 From: Prasanth Louis Date: Tue, 16 Apr 2024 21:42:46 -0400 Subject: [PATCH] chore(rds): improve error messages for providing a vpc to cluster (#29706) Just an idea/I was curious. Personally, when I stumbled upon this error I got a bit confused because I wasn't sure what instanceProps was. I think the message 'Provide either vpc or instanceProps.vpc, but not both" provides the same explanation for both cases? Or we rephrase the error message to not use "If instanceProps was provided", because the code can figure whether instanceProps was provided with another if condition Then the message would just be "VPC must be provided". Hence, someone who is unaware of instanceProps, doesn't need to be notified of it. Just an idea though and it's nitpicking! Thoughts? ### Reason for this change Better error message when running into this error ### Description of changes Consolidated logs statements ### Description of how you validated changes I have not validated anything because this was just an idea/I was curious ### 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* --- packages/aws-cdk-lib/aws-rds/lib/cluster.ts | 4 +--- packages/aws-cdk-lib/aws-rds/test/cluster.test.ts | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk-lib/aws-rds/lib/cluster.ts b/packages/aws-cdk-lib/aws-rds/lib/cluster.ts index 5dbaa0a8b0604..05997a4a09ec2 100644 --- a/packages/aws-cdk-lib/aws-rds/lib/cluster.ts +++ b/packages/aws-cdk-lib/aws-rds/lib/cluster.ts @@ -595,10 +595,8 @@ abstract class DatabaseClusterNew extends DatabaseClusterBase { constructor(scope: Construct, id: string, props: DatabaseClusterBaseProps) { super(scope, id); - if ((props.vpc && props.instanceProps?.vpc)) { + if ((props.vpc && props.instanceProps?.vpc) || (!props.vpc && !props.instanceProps?.vpc)) { throw new Error('Provide either vpc or instanceProps.vpc, but not both'); - } else if (!props.vpc && !props.instanceProps?.vpc) { - throw new Error('If instanceProps is not provided then `vpc` must be provided.'); } if ((props.vpcSubnets && props.instanceProps?.vpcSubnets)) { throw new Error('Provide either vpcSubnets or instanceProps.vpcSubnets, but not both'); diff --git a/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts b/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts index 20ca5417b6303..d27abfb4a8065 100644 --- a/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts +++ b/packages/aws-cdk-lib/aws-rds/test/cluster.test.ts @@ -67,7 +67,7 @@ describe('cluster new api', () => { iamAuthentication: true, }); // THEN - }).toThrow(/If instanceProps is not provided then `vpc` must be provided./); + }).toThrow(/Provide either vpc or instanceProps.vpc, but not both/); }); test('when both vpc and instanceProps.vpc are provided', () => {