Skip to content

Commit

Permalink
fix(core): messages are displayed multiple times per construct (#24019)
Browse files Browse the repository at this point in the history
Annotations added many times clog terminal space and make debugging difficult. Deduplicate annotations based on the message.

Closes #9565.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
Styerp committed Feb 9, 2023
1 parent 5942978 commit 57770bb
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 25 deletions.
31 changes: 7 additions & 24 deletions packages/@aws-cdk/core/lib/annotations.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import * as cxschema from '@aws-cdk/cloud-assembly-schema';
import * as cxapi from '@aws-cdk/cx-api';
import { IConstruct, Node } from 'constructs';

const DEPRECATIONS_SYMBOL = Symbol.for('@aws-cdk/core.deprecations');
import { IConstruct } from 'constructs';

/**
* Includes API for attaching annotations such as warning messages to constructs.
Expand Down Expand Up @@ -77,38 +75,23 @@ export class Annotations {

// throw if CDK_BLOCK_DEPRECATIONS is set
if (process.env.CDK_BLOCK_DEPRECATIONS) {
throw new Error(`${Node.of(this.scope).path}: ${text}`);
}

// de-dup based on api key
const set = this.deprecationsReported;
if (set.has(api)) {
return;
throw new Error(`${this.scope.node.path}: ${text}`);
}

this.addWarning(text);
set.add(api);
}

/**
* Adds a message metadata entry to the construct node, to be displayed by the CDK CLI.
*
* Records the message once per construct.
* @param level The message level
* @param message The message itself
*/
private addMessage(level: string, message: string) {
Node.of(this.scope).addMetadata(level, message, { stackTrace: this.stackTraces });
}

/**
* Returns the set of deprecations reported on this construct.
*/
private get deprecationsReported() {
let set = (this.scope as any)[DEPRECATIONS_SYMBOL];
if (!set) {
set = new Set();
Object.defineProperty(this.scope, DEPRECATIONS_SYMBOL, { value: set });
const isNew = !this.scope.node.metadata.find((x) => x.data === message);
if (isNew) {
this.scope.node.addMetadata(level, message, { stackTrace: this.stackTraces });
}

return set;
}
}
21 changes: 20 additions & 1 deletion packages/@aws-cdk/core/test/annotations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Construct } from 'constructs';
import { getWarnings } from './util';
import { App, Stack } from '../lib';
import { Annotations } from '../lib/annotations';
import { getWarnings } from './util';

const restore = process.env.CDK_BLOCK_DEPRECATIONS;

Expand Down Expand Up @@ -74,4 +74,23 @@ describe('annotations', () => {
process.env.CDK_BLOCK_DEPRECATIONS = '1';
expect(() => Annotations.of(c1).addDeprecation('foo', 'bar')).toThrow(/MyStack\/Hello: The API foo is deprecated: bar\. This API will be removed in the next major release/);
});

test('addMessage deduplicates the message on the node level', () => {
const app = new App();
const stack = new Stack(app, 'S1');
const c1 = new Construct(stack, 'C1');
Annotations.of(c1).addWarning('You should know this!');
Annotations.of(c1).addWarning('You should know this!');
Annotations.of(c1).addWarning('You should know this!');
Annotations.of(c1).addWarning('You should know this, too!');
expect(getWarnings(app.synth())).toEqual([{
path: '/S1/C1',
message: 'You should know this!',
},
{
path: '/S1/C1',
message: 'You should know this, too!',
}],
);
});
});

0 comments on commit 57770bb

Please sign in to comment.