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

this.node.addError does not cause cdk diff to fail #4700

Closed
mipearson opened this issue Oct 26, 2019 · 2 comments
Closed

this.node.addError does not cause cdk diff to fail #4700

mipearson opened this issue Oct 26, 2019 · 2 comments
Assignees
Labels
bug This issue is a bug. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2 package/tools Related to AWS CDK Tools or CLI

Comments

@mipearson
Copy link

❓ General Issue

Not sure if this is a bug or a feature.

cdk diff <stackname> will succeed on a stack with errors, even though a cdk deploy or cdk synth will fail. The cdk diff does not show any indication of an error.

If this is intention it might be worth signposting it somehow.

Environment

  • CDK CLI Version: Unknown command: version (but it's 1.14.0 (build 261a1bf) if I run cdk --version instead)
  • OS: OSX Mojave
  • Language: Typescript, but suspect all as it's within the CDK CLI
@mipearson mipearson added the needs-triage This issue or PR still needs to be triaged. label Oct 26, 2019
@nmussy
Copy link
Contributor

nmussy commented Oct 27, 2019

The errors will be thrown in the processMetadata method:

if (errors && !this.props.ignoreErrors) {
throw new Error('Found errors');
}

Which is only called for the synth and deploy commands, like you've noticed:

this.appStacks.processMetadata(stacks);

appStacks.processMetadata(stacks);

Even if it was intended for diff not to throw, not seeing any warning or error until deployment is a little weird.

@SomayaB SomayaB added the package/tools Related to AWS CDK Tools or CLI label Oct 28, 2019
@SomayaB SomayaB added bug This issue is a bug. and removed needs-triage This issue or PR still needs to be triaged. labels Oct 28, 2019
@shivlaks shivlaks added the p2 label Oct 31, 2019
@rix0rrr rix0rrr added the good first issue Related to contributions. See CONTRIBUTING.md label Dec 2, 2019
nideveloper pushed a commit to nideveloper/aws-cdk that referenced this issue Dec 3, 2019
Adding in appStacks.processMetadata(stacks); command into the diff method so that metadata is process correctly and the diff should fail
@SomayaB SomayaB added the in-progress This issue is being actively worked on. label Dec 3, 2019
shivlaks pushed a commit that referenced this issue Dec 11, 2019
…5284)

Adding in appStacks.processMetadata(stacks); command into the diff method so that metadata is processed correctly and the diff should fail
@shivlaks
Copy link
Contributor

fixed by #5284

ed-at-work pushed a commit to ed-at-work/aws-cdk that referenced this issue Dec 17, 2019
… (aws#5284)

Adding in appStacks.processMetadata(stacks); command into the diff method so that metadata is processed correctly and the diff should fail
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. good first issue Related to contributions. See CONTRIBUTING.md in-progress This issue is being actively worked on. p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
Development

No branches or pull requests

5 participants