Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
2 tasks
john-aws opened this issue Nov 5, 2023 · 2 comments
Open
2 tasks

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

john-aws opened this issue Nov 5, 2023 · 2 comments
Labels
cli Issues related to the CDK CLI feature-request A feature should be added or improved. p2

Comments

@john-aws
Copy link

john-aws commented Nov 5, 2023

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

  • I may be able to implement this feature request
  • This feature might incur a breaking change

CDK version used

2.104.0 (build 3b99abe)

Environment details (OS name and version, etc.)

Mac OS X

@john-aws john-aws added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Nov 5, 2023
@khushail
Copy link
Contributor

khushail commented Nov 6, 2023

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.

@khushail khushail added p2 needs-review and removed needs-triage This issue or PR still needs to be triaged. labels Nov 6, 2023
@khushail khushail changed the title Validation errors: no need for full stack trace (cli): no need for full stack trace while validating errors Nov 6, 2023
@khushail khushail added cli Issues related to the CDK CLI and removed needs-review labels Nov 8, 2023
@john-aws
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Issues related to the CDK CLI feature-request A feature should be added or improved. p2
Projects
None yet
Development

No branches or pull requests

2 participants