(aws-cdk-lib): asynchronous build with unhandled errors cause deployment malfunctions

[ChristopheBougere]

What is the problem?

I have a CDK project deployed with an OpenSearch Domain with availability zone awareness.
To reduce costs, I wanted to reduce the number of nodes in my dev env.
I changed the dataNodes to 1, as well as the availabilityZoneCount.
This was a mistake, because availabilityZoneCount should be at least 2 (or we should simply set zoneAwareness to undefined to disable it).
However, it resulted in an unhandled error (see log below) and in the complete removal of the construct that contained the OpenSearch domain.

(node:5224) UnhandledPromiseRejectionWarning: Error: Invalid zone awareness configuration; availabilityZoneCount must be 2 or 3
    at new Domain (/Users/pathToMyProject/node_modules/aws-cdk-lib/aws-opensearchservice/lib/domain.ts:648:13)
    at new Search (/Users/pathToMyProject/api/cdk/search.ts:38:23)
    at new ApiStack (/Users/pathToMyProject/api/cdk/index.ts:75:24)
    at new MyProjectStack (/Users/pathToMyProject/cdk/lib/index.ts:51:21)
    at build (/Users/pathToMyProject/cdk/bin/my-project.ts:43:5)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)
(Use `node --trace-warnings ...` to show where the warning was created)
(node:5224) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 13)
(node:5224) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Reproduction Steps

1. Create a project with an OpenSearch domain set in a vpc with 3 data nodes, and zone awareness in 3 AZs
2. Deploy
3. Change the data nodes to 1, and availabilityZoneCount to 1 as well
4. Deploy. You will see an unhandled error in the logs, but the deployment will continue and the construct that use to contain the open search domain will be totally deleted.

What did you expect to happen?

The deployment should have just failed, instead of deleting one of my constructs.

What actually happened?

The error was unhandled, and one of my construct (the one in which an error happened) was totally suppressed.

CDK CLI Version

2.1.0

Framework Version

2.1.0

Node.js Version

v14.16.0

OS

macOS Monterey 12.0.1

Language

Typescript

Language Version

3.9.7

Other information

This is, indeed, a very critical bug 😬

[kaizencc]

Hi @ChristopheBougere! Thanks for the bug report. I wasn't able to reproduce this bug on my end following your steps. Setting the availabilityZoneCount to 1 results in a synth time error for me, due to this check:

aws-cdk/packages/@aws-cdk/aws-opensearchservice/lib/domain.ts

Lines 1222 to 1224 in 15d3889

	if (![2, 3].includes(availabilityZoneCount)) {
	throw new Error('Invalid zone awareness configuration; availabilityZoneCount must be 2 or 3');
	}

Is it possible that you are not using the opensearch.Domain construct - maybe you are using opensearch.CfnDomain? At any rate, could you please provide a code snippet that will help me identify exactly how you came across this deploy-time error.

[github-actions]

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[ChristopheBougere]

Hi @kaizen3031593
Thanks for having a look! So I am using the opensearch.Domain construct. I also had the same error thrown, Invalid zone awareness configuration; availabilityZoneCount must be 2 or 3, but as you can see in the initial stack trace, the promise rejection was unhandled, which I think is the reason the deployment continued after that.
However, this is the first time I see this kind of behaviour in CDK, that's why I'm reporting (maybe related to v2?).
I'll try to provide a code snippet

[ChristopheBougere]

Also, my error seems very similar to this one:
#4927 (comment)
I am also resolving config asynchronously in a very similar way, which might make async promise rejection unhandled.

[ChristopheBougere]

I can confirm that this is not an issue with the opensearch module itself, it happens for any error thrown at deployment time.

The issue comes from the way errors are handled.

I solved it by adding a try/catch with a process.exit in my bin/my-project.ts file:

Before

#!/usr/bin/env node

import { App } from 'aws-cdk-lib';

async function build() {
    const app = new App();

    // resolving config...

    new MyProjectStack(app, 'my-project');
}

build();

After

#!/usr/bin/env node

import { App } from 'aws-cdk-lib';

async function build() {
  try {
    const app = new App();

    // resolving config...

    new MyProjectStack(app, 'my-project');

  } catch {
    console.error('CDK unhandled error:', error);
    process.exit(1);
  }
}

build();

@kaizen3031593 Feel free to close the issue. However, I feel there might have been a change in how unhandled errors are catched in cdk v2 (prior to uprade, cdk thrown errors use to fail deployment properly).

[kaizencc]

@ChristopheBougere I am going to reroute your issue to CDK v2 and @njlynch but I am not sure what, if anything, we can do about building asynchronously.

[njlynch]

Generally, we haven't historically supported async processing within constructs. See #8273 for a long thread on some of the rationale (and counter-arguments) to why that is.

That being said, I'm intrigued by the claim that it worked in v1 but not v2. Can you provide a reproducible example that we can synth/deploy on v1 vs v2 and see what's changed?

[github-actions]

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[ChristopheBougere]

Thanks for pointing to this thread @njlynch
I've tried to provide a repro, and it ended up in CDK v1 having the same behavior than v2 :/
I was surprised that it didn't happen to me before.
While I understand the reasons for not supporting async processing, catching this kind of exception could still be helpful to avoid some errors (maybe with a CLI warning if cdk detect async usage?).

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.