Skip to content

Commit b3c5fe8

Browse files
authored
fix(cli): notices refresh doesn't respect the --no-notices flag (#19226)
The `--no-notices` flag is only being considered for actually displaying the notices, but not in the refresh step, used to mask latency. Also handling request timeout on the `Promise` level. If the promise doesn't resolve within the time limit, resolve with an empty array, no matter what. Fixes #19201. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 7c3099e commit b3c5fe8

File tree

3 files changed

+29
-15
lines changed

3 files changed

+29
-15
lines changed

packages/aws-cdk/lib/cli.ts

+9-7
Original file line numberDiff line numberDiff line change
@@ -235,10 +235,6 @@ if (!process.stdout.isTTY) {
235235
}
236236

237237
async function initCommandLine() {
238-
void refreshNotices()
239-
.then(_ => debug('Notices refreshed'))
240-
.catch(e => debug(`Notices refresh failed: ${e}`));
241-
242238
const argv = await parseCommandLineArguments();
243239
if (argv.verbose) {
244240
setLogLevel(argv.verbose);
@@ -254,6 +250,12 @@ async function initCommandLine() {
254250
});
255251
await configuration.load();
256252

253+
if (shouldDisplayNotices()) {
254+
void refreshNotices()
255+
.then(_ => debug('Notices refreshed'))
256+
.catch(e => debug(`Notices refresh failed: ${e}`));
257+
}
258+
257259
const sdkProvider = await SdkProvider.withAwsCliCompatibleDefaults({
258260
profile: configuration.settings.get(['profile']),
259261
ec2creds: argv.ec2creds,
@@ -326,10 +328,10 @@ async function initCommandLine() {
326328
});
327329
}
328330
}
331+
}
329332

330-
function shouldDisplayNotices(): boolean {
331-
return configuration.settings.get(['notices']) ?? true;
332-
}
333+
function shouldDisplayNotices(): boolean {
334+
return configuration.settings.get(['notices']) ?? true;
333335
}
334336

335337
async function main(command: string, args: any): Promise<number | void> {

packages/aws-cdk/lib/notices.ts

+6-8
Original file line numberDiff line numberDiff line change
@@ -43,13 +43,14 @@ export async function displayNotices(props: DisplayNoticesProps) {
4343

4444
export async function generateMessage(dataSource: NoticeDataSource, props: DisplayNoticesProps) {
4545
const data = await dataSource.fetch();
46-
const individualMessages = formatNotices(filterNotices(data, {
46+
const filteredNotices = filterNotices(data, {
4747
outdir: props.outdir,
4848
acknowledgedIssueNumbers: new Set(props.acknowledgedIssueNumbers),
49-
}));
49+
});
5050

51-
if (individualMessages.length > 0) {
52-
return finalMessage(individualMessages, data[0].issueNumber);
51+
if (filteredNotices.length > 0) {
52+
const individualMessages = formatNotices(filteredNotices);
53+
return finalMessage(individualMessages, filteredNotices[0].issueNumber);
5354
}
5455
return '';
5556
}
@@ -108,18 +109,15 @@ export class WebsiteNoticeDataSource implements NoticeDataSource {
108109
const timeout = 3000;
109110

110111
return new Promise((resolve) => {
112+
setTimeout(() => resolve([]), timeout);
111113
try {
112114
const req = https.get('https://cli.cdk.dev-tools.aws.dev/notices.json',
113115
{ timeout },
114116
res => {
115-
const startTime = Date.now();
116117
if (res.statusCode === 200) {
117118
res.setEncoding('utf8');
118119
let rawData = '';
119120
res.on('data', (chunk) => {
120-
if (Date.now() - startTime > timeout) {
121-
resolve([]);
122-
}
123121
rawData += chunk;
124122
});
125123
res.on('end', () => {

packages/aws-cdk/test/integ/cli/cli.integtest.ts

+14
Original file line numberDiff line numberDiff line change
@@ -757,6 +757,20 @@ integTest('templates on disk contain metadata resource, also in nested assemblie
757757
expect(JSON.parse(nestedTemplateContents).Resources.CDKMetadata).toBeTruthy();
758758
}));
759759

760+
integTest('skips notice refresh', withDefaultFixture(async (fixture) => {
761+
const output = await fixture.cdkSynth({
762+
options: ['--no-notices'],
763+
modEnv: {
764+
INTEG_STACK_SET: 'stage-using-context',
765+
},
766+
allowErrExit: true,
767+
});
768+
769+
// Neither succeeds nor fails, but skips the refresh
770+
await expect(output).not.toContain('Notices refreshed');
771+
await expect(output).not.toContain('Notices refresh failed');
772+
}));
773+
760774
async function listChildren(parent: string, pred: (x: string) => Promise<boolean>) {
761775
const ret = new Array<string>();
762776
for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) {

0 commit comments

Comments
 (0)