aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.59k stars 3.89k forks source link

(cli): no need for full stack trace while validating errors #27847

Open john-aws opened 11 months ago

john-aws commented 11 months ago

Describe the feature

There is a class of common user validation errors that are being handled almost as if they were internal failures in CDK. Rather than CDK simply emit a validation error such as "Error: There is already a Construct with name 'SomeResourceName' on line 23 of xyz.ts", CDK instead throws an ugly exception with full internal stack trace, implying that something much more significant has happened. The error message is more alarming than necessary and less readable than would be ideal.

An example is:

Error: There is already a Construct with name 'SomeResourceName' in some-class [some-name]

The error in this case is a simple, and perhaps common, mistake made by the CDK user and that is to have accidentally given 2 different resources the same name (perhaps as a result of an errant copy/paste).

The cdk deploy output looks something like this:

/Users/me/src/someproject/node_modules/constructs/src/construct.ts:447
      throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
            ^
Error: There is already a Construct with name 'SomeResourceName' in SomeClass [some-class]
    at Node.addChild (/Users/me/src/someproject/node_modules/constructs/src/construct.ts:447:13)
    at new Node (/Users/me/src/someproject/node_modules/constructs/src/construct.ts:71:17)
    at new Construct (/Users/me/src/someproject/node_modules/constructs/src/construct.ts:499:17)
    at new Resource (/Users/me/src/someproject/node_modules/aws-cdk-lib/core/lib/resource.js:1:1309)
    at new SecurityGroupBase (/Users/me/src/someproject/node_modules/aws-cdk-lib/aws-ec2/lib/security-group.js:1:1051)
    at new SecurityGroup (/Users/me/src/someproject/node_modules/aws-cdk-lib/aws-ec2/lib/security-group.js:1:5332)
    at new SomeResource (/Users/me/src/someproject/lib/xxx.ts:244:35)
    at Object.<anonymous> (/Users/me/src/someproject/bin/xxx.ts:11:1)
    at Module._compile (node:internal/modules/cjs/loader:1256:14)
    at Module.m._compile (/Users/me/src/someproject/node_modules/ts-node/src/index.ts:1618:23)

Subprocess exited with error 1

Use Case

Simple user errors result in error messages that are much less readable than would be ideal.

Proposed Solution

Emit the error message with source line number, minus all the stack trace.

Other Information

No response

Acknowledgements

CDK version used

2.104.0 (build 3b99abe)

Environment details (OS name and version, etc.)

Mac OS X

khushail commented 11 months ago

Hi @john-aws , thanks for reaching out. AFAIK, when the CDK Constucts are mapped to CDK Tree, has validate() function, which returns string[], to determine if the construct is in valid state or not.This function is called during synth, hence the output is generated as such, as can be seen here for every scenario. Depending on each subtype of error checking, the resulting errors are being generated. I am not sure of omitting the stack trace as such descriptive error stack might be useful in other scenarios.

However I am marking this as P2 and would discuss with team for further action.

john-aws commented 11 months ago

Another example is when the user responds n to the deploy-time question:

Do you wish to deploy these changes (y/n)?

This results in a lengthy stack trace:

Deployment failed: Error: Aborted by user
    at /Users/me/src/someproject/node_modules/aws-cdk/lib/index.js:429:153044
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async withCorkedLogging (/Users/me/src/someproject/node_modules/aws-cdk/lib/index.js:1:34855)
    at async Object.deployStack2 [as deployStack] (/Users/me/src/someproject/node_modules/aws-cdk/lib/index.js:429:152489)
    at async /Users/me/src/someproject/node_modules/aws-cdk/lib/index.js:429:137002

Aborted by user

A more appropriate response would simply be:

Deployment was cancelled by user