Skip to content

Commit

Permalink
fix(cli): turn validation errors into metadata errors
Browse files Browse the repository at this point in the history
Validation errors are currently turned into exceptions. This causes
issues where missing context values lead to validation errors (such as
in the case of `DnsValidatedCertificate`): the error would have been
solved by retrieving the context value, but because an exception is
thrown the CLI stops immediately and doesn't re-execute.

I feel it also makes sense to treat these errors as construct errors,
as opposed to exceptions which I feel should be more "you're misusing
the API" kind of errors, rather than "something is wrong somewhere"
kind of errors.

The change I made is the smallest change to achieve the desired effect.
If we agree that we want this change, it would probably be better to
do it differently, just have `.validate()` implementors call
`this.node.addError()` immediately.

Fixes #2076.
  • Loading branch information
rix0rrr committed Jun 21, 2019
1 parent 5baa31f commit 41114db
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 5 deletions.
9 changes: 5 additions & 4 deletions packages/@aws-cdk/cdk/lib/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,11 @@ export class ConstructNode {
const validate = options.skipValidation === undefined ? true : !options.skipValidation;
if (validate) {
const errors = this.validate(root);
if (errors.length > 0) {
const errorList = errors.map(e => `[${e.source.node.path}] ${e.message}`).join('\n ');
throw new Error(`Validation failed with the following errors:\n ${errorList}`);
}
// Turn the errors into metadata errors, so they will prevent deploying but will not
// prevent CDK from executing again.
errors.forEach(e => {
e.source.node.addError(e.message);
});
}

// synthesize (leaves first)
Expand Down
6 changes: 5 additions & 1 deletion packages/@aws-cdk/cdk/test/test.app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,11 @@ export = {
new Child(parent, 'C1');
new Child(parent, 'C2');

test.throws(() => app.synth(), /Validation failed with the following errors/);
const assembly = app.synth();
test.deepEqual(assembly.getStack('Parent').messages.filter(m => m.level === cxapi.SynthesisMessageLevel.ERROR).map(m => m.entry.data), [
'Error from C1',
'Error from C2',
]);

test.done();
},
Expand Down

0 comments on commit 41114db

Please sign in to comment.