Skip to content

Commit 3b2846e

Browse files
authored
refactor(toolkit): new message level result (#33234)
### Reason for this change The CDK CLI has certain semantic rules around which messages are written to which output stream (`stdout` or `stderr`). These rules are mostly generalizable based on the level of a message. However until now there was one major exception: The main results of a CLI command should always be written to `stdout`, regardless of level. In practice these messages were always attributed to the `info` level. With the recent refactorings towards a `CliIoHost`, we needed to include an additional property `forceStdout` on `IoMessages`. This always was a bad crutch and unintended to stay. ### Description of changes Removal of the `forceStdout` message property, in favor of a new explicit message level: `result`. In terms of priority, a `result` is lower than an `error`, but higher than a `warn`. This is intuitive: A user that wants to ignore all warnings, will still want to see results. Use result in the CLU and the new Toolkit class. ### Describe any new or updated permissions being added n/a ### Description of how you validated changes Existing and extended tests. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent c40a9e2 commit 3b2846e

File tree

14 files changed

+125
-128
lines changed

14 files changed

+125
-128
lines changed

packages/@aws-cdk/toolkit/lib/api/io/io-message.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { ToolkitAction } from '../../toolkit';
44
* The reporting level of the message.
55
* All messages are always reported, it's up to the IoHost to decide what to log.
66
*/
7-
export type IoMessageLevel = 'error' | 'warn' | 'info' | 'debug' | 'trace';
7+
export type IoMessageLevel = 'error'| 'result' | 'warn' | 'info' | 'debug' | 'trace';
88

99
/**
1010
* Valid reporting categories for messages.
@@ -89,6 +89,7 @@ const levels = [
8989
'debug',
9090
'info',
9191
'warn',
92+
'result',
9293
'error',
9394
] as const;
9495

packages/@aws-cdk/toolkit/lib/api/io/private/messages.ts

+17-14
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export const prompt = <T, U>(code: VALID_CODE, message: string, defaultResponse:
6464
};
6565

6666
/**
67-
* Logs an error level message.
67+
* Creates an error level message.
6868
*/
6969
export const error = <T>(message: string, code?: VALID_CODE, payload?: T) => {
7070
return formatMessage({
@@ -76,34 +76,37 @@ export const error = <T>(message: string, code?: VALID_CODE, payload?: T) => {
7676
};
7777

7878
/**
79-
* Logs an warning level message.
79+
* Creates a result level message and represents the most important message for a given action.
80+
*
81+
* They should be used sparsely, with an action usually having no or exactly one result.
82+
* However actions that operate on Cloud Assemblies might include a result per Stack.
83+
* Unlike other messages, results must always have a code and a payload.
8084
*/
81-
export const warn = <T>(message: string, code?: VALID_CODE, payload?: T) => {
85+
export const result = <T>(message: string, code: VALID_CODE, payload: T) => {
8286
return formatMessage({
83-
level: 'warn',
87+
level: 'result',
8488
code,
8589
message,
8690
data: payload,
8791
});
8892
};
8993

9094
/**
91-
* Logs an info level message.
95+
* Creates a warning level message.
9296
*/
93-
export const info = <T>(message: string, code?: VALID_CODE, payload?: T) => {
97+
export const warn = <T>(message: string, code?: VALID_CODE, payload?: T) => {
9498
return formatMessage({
95-
level: 'info',
99+
level: 'warn',
96100
code,
97101
message,
98102
data: payload,
99103
});
100104
};
101105

102106
/**
103-
* Logs an info level message to stdout.
104-
* @deprecated
107+
* Creates an info level message.
105108
*/
106-
export const data = <T>(message: string, code?: VALID_CODE, payload?: T) => {
109+
export const info = <T>(message: string, code?: VALID_CODE, payload?: T) => {
107110
return formatMessage({
108111
level: 'info',
109112
code,
@@ -113,7 +116,7 @@ export const data = <T>(message: string, code?: VALID_CODE, payload?: T) => {
113116
};
114117

115118
/**
116-
* Logs a debug level message.
119+
* Creates a debug level message.
117120
*/
118121
export const debug = <T>(message: string, code?: VALID_CODE, payload?: T) => {
119122
return formatMessage({
@@ -125,7 +128,7 @@ export const debug = <T>(message: string, code?: VALID_CODE, payload?: T) => {
125128
};
126129

127130
/**
128-
* Logs a trace level message.
131+
* Creates a trace level message.
129132
*/
130133
export const trace = <T>(message: string, code?: VALID_CODE, payload?: T) => {
131134
return formatMessage({
@@ -137,7 +140,7 @@ export const trace = <T>(message: string, code?: VALID_CODE, payload?: T) => {
137140
};
138141

139142
/**
140-
* Logs an info level success message in green text.
143+
* Creates an info level success message in green text.
141144
* @deprecated
142145
*/
143146
export const success = <T>(message: string, code?: VALID_CODE, payload?: T) => {
@@ -150,7 +153,7 @@ export const success = <T>(message: string, code?: VALID_CODE, payload?: T) => {
150153
};
151154

152155
/**
153-
* Logs an info level message in bold text.
156+
* Creates an info level message in bold text.
154157
* @deprecated
155158
*/
156159
export const highlight = <T>(message: string, code?: VALID_CODE, payload?: T) => {

packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts

+5-5
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { CachedCloudAssemblySource, IdentityCloudAssemblySource, StackAssembly,
1919
import { ALL_STACKS, CloudAssemblySourceBuilder } from '../api/cloud-assembly/private';
2020
import { ToolkitError } from '../api/errors';
2121
import { IIoHost, IoMessageCode, IoMessageLevel } from '../api/io';
22-
import { asSdkLogger, withAction, Timer, confirm, error, highlight, info, success, warn, ActionAwareIoHost, debug } from '../api/io/private';
22+
import { asSdkLogger, withAction, Timer, confirm, error, highlight, info, success, warn, ActionAwareIoHost, debug, result } from '../api/io/private';
2323

2424
/**
2525
* The current action being performed by the CLI. 'none' represents the absence of an action.
@@ -149,7 +149,7 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
149149
const firstStack = stacks.firstStack!;
150150
const template = firstStack.template;
151151
const obscuredTemplate = obscureTemplate(template);
152-
await ioHost.notify(info(message, 'CDK_TOOLKIT_I0001', {
152+
await ioHost.notify(result(message, 'CDK_TOOLKIT_I0001', {
153153
...assemblyData,
154154
stack: {
155155
stackName: firstStack.stackName,
@@ -161,7 +161,7 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
161161
}));
162162
} else {
163163
// not outputting template to stdout, let's explain things to the user a little bit...
164-
await ioHost.notify(success(message, 'CDK_TOOLKIT_I0002', assemblyData));
164+
await ioHost.notify(result(chalk.green(message), 'CDK_TOOLKIT_I0002', assemblyData));
165165
await ioHost.notify(info(`Supply a stack id (${stacks.stackArtifacts.map((s) => chalk.green(s.hierarchicalId)).join(', ')}) to display its template.`));
166166
}
167167

@@ -650,15 +650,15 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
650650
const startRollbackTime = Timer.start();
651651
const deployments = await this.deploymentsForAction('rollback');
652652
try {
653-
const result = await deployments.rollbackStack({
653+
const stackResult = await deployments.rollbackStack({
654654
stack,
655655
roleArn: options.roleArn,
656656
toolkitStackName: this.toolkitStackName,
657657
force: options.orphanFailedResources,
658658
validateBootstrapStackVersion: options.validateBootstrapStackVersion,
659659
orphanLogicalIds: options.orphanLogicalIds,
660660
});
661-
if (!result.notInRollbackableState) {
661+
if (!stackResult.notInRollbackableState) {
662662
anyRollbackable = true;
663663
}
664664
const elapsedRollbackTime = startRollbackTime.end();

packages/@aws-cdk/toolkit/test/actions/synth.test.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ describe('synth', () => {
1818
// THEN
1919
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
2020
action: 'synth',
21-
level: 'info',
21+
level: 'result',
2222
message: expect.stringContaining('Successfully synthesized'),
2323
}));
2424
});
@@ -31,7 +31,7 @@ describe('synth', () => {
3131
// THEN
3232
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
3333
action: 'synth',
34-
level: 'info',
34+
level: 'result',
3535
message: expect.stringContaining('Successfully synthesized'),
3636
}));
3737
});
@@ -44,7 +44,7 @@ describe('synth', () => {
4444
// THEN
4545
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
4646
action: 'synth',
47-
level: 'info',
47+
level: 'result',
4848
code: 'CDK_TOOLKIT_I0001',
4949
message: expect.stringContaining('Successfully synthesized'),
5050
data: expect.objectContaining({
@@ -65,7 +65,7 @@ describe('synth', () => {
6565
// THEN
6666
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
6767
action: 'synth',
68-
level: 'info',
68+
level: 'result',
6969
code: 'CDK_TOOLKIT_I0002',
7070
message: expect.stringContaining('Successfully synthesized'),
7171
data: expect.objectContaining({

packages/@aws-cdk/toolkit/test/api/io/io-message.test.ts

+7-5
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ import { isMessageRelevantForLevel } from '../../../lib/api/io/io-message';
22

33
describe('IoMessageLevel', () => {
44
test.each`
5-
msgLevel | logLevel | isRelevant
6-
${'error'} | ${'trace'} | ${true}
7-
${'info'} | ${'trace'} | ${true}
8-
${'info'} | ${'warn'} | ${false}
9-
${'trace'} | ${'error'} | ${false}
5+
msgLevel | logLevel | isRelevant
6+
${'error'} | ${'trace'} | ${true}
7+
${'info'} | ${'trace'} | ${true}
8+
${'result'} | ${'warn'} | ${true}
9+
${'info'} | ${'warn'} | ${false}
10+
${'trace'} | ${'error'} | ${false}
11+
${'warn'} | ${'result'} | ${false}
1012
`('with msgLevel=$msgLevel and logLevel=$msgLevel, logging should be $shouldLog', async ({ msgLevel, logLevel, isRelevant }) => {
1113
expect(isMessageRelevantForLevel({ level: msgLevel }, logLevel)).toBe(isRelevant);
1214
});

packages/aws-cdk/lib/cli/cdk-toolkit.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ import {
4747
import { printSecurityDiff, printStackDiff, RequireApproval } from '../diff';
4848
import { ResourceImporter, removeNonImportResources } from '../import';
4949
import { listStacks } from '../list-stacks';
50-
import { data, debug, error, highlight, info, success, warning } from '../logging';
50+
import { result as logResult, debug, error, highlight, info, success, warning } from '../logging';
5151
import { ResourceMigrator } from '../migrator';
5252
import { deserializeStructure, obscureTemplate, serializeStructure } from '../serialize';
5353
import { CliIoHost } from '../toolkit/cli-io-host';
@@ -526,7 +526,7 @@ export class CdkToolkit {
526526

527527
info('Stack ARN:');
528528

529-
data(deployResult.stackArn);
529+
logResult(deployResult.stackArn);
530530
} catch (e: any) {
531531
// It has to be exactly this string because an integration test tests for
532532
// "bold(stackname) failed: ResourceNotReady: <error>"
@@ -891,7 +891,7 @@ export class CdkToolkit {
891891

892892
// just print stack IDs
893893
for (const stack of stacks) {
894-
data(stack.id);
894+
logResult(stack.id);
895895
}
896896

897897
return 0; // exit-code
@@ -1275,7 +1275,7 @@ export class CdkToolkit {
12751275
* Print a serialized object (YAML or JSON) to stdout.
12761276
*/
12771277
function printSerializedObject(obj: any, json: boolean) {
1278-
data(serializeStructure(obj, json));
1278+
logResult(serializeStructure(obj, json));
12791279
}
12801280

12811281
export interface DiffOptions {

packages/aws-cdk/lib/cli/cli.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import { docs } from '../commands/docs';
2323
import { doctor } from '../commands/doctor';
2424
import { getMigrateScanType } from '../commands/migrate';
2525
import { cliInit, printAvailableTemplates } from '../init';
26-
import { data, debug, error, info } from '../logging';
26+
import { result, debug, error, info } from '../logging';
2727
import { Notices } from '../notices';
2828
import { Command, Configuration } from './user-configuration';
2929
import { IoMessageLevel, CliIoHost } from '../toolkit/cli-io-host';
@@ -490,7 +490,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
490490
});
491491
case 'version':
492492
ioHost.currentAction = 'version';
493-
return data(version.DISPLAY_VERSION);
493+
return result(version.DISPLAY_VERSION);
494494

495495
default:
496496
throw new ToolkitError('Unknown command: ' + command);

packages/aws-cdk/lib/commands/context.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { minimatch } from 'minimatch';
33
import { Context } from '../api/context';
44
import { PROJECT_CONFIG, PROJECT_CONTEXT, USER_DEFAULTS } from '../cli/user-configuration';
55
import * as version from '../cli/version';
6-
import { error, warning, info, data } from '../logging';
6+
import { error, warning, info, result } from '../logging';
77
import { ToolkitError } from '../toolkit/error';
88
import { renderTable } from '../util';
99

@@ -58,7 +58,7 @@ export async function contextHandler(options: ContextOptions): Promise<number> {
5858
if (options.json) {
5959
/* istanbul ignore next */
6060
const contextValues = options.context.all;
61-
data(JSON.stringify(contextValues, undefined, 2));
61+
result(JSON.stringify(contextValues, undefined, 2));
6262
} else {
6363
listContext(options.context);
6464
}

packages/aws-cdk/lib/logging.ts

+14-16
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import { IoMessageLevel, IoMessage, CliIoHost, IoMessageSpecificCode, IoMessageC
99
*/
1010
function formatMessageAndLog(
1111
level: IoMessageLevel,
12-
forceStdout: boolean,
1312
input: LogInput<IoCodeLevel>,
1413
style?: (str: string) => string,
1514
...args: unknown[]
@@ -32,7 +31,6 @@ function formatMessageAndLog(
3231
level,
3332
message: finalMessage,
3433
code,
35-
forceStdout,
3634
};
3735

3836
void ioHost.notify(ioMessage);
@@ -75,7 +73,7 @@ type LogInput<L extends IoCodeLevel> = string | LogParams<L>;
7573
* ```
7674
*/
7775
export const error = (input: LogInput<'E'>, ...args: unknown[]) => {
78-
return formatMessageAndLog('error', false, input, undefined, ...args);
76+
return formatMessageAndLog('error', input, undefined, ...args);
7977
};
8078

8179
/**
@@ -90,7 +88,7 @@ export const error = (input: LogInput<'E'>, ...args: unknown[]) => {
9088
* ```
9189
*/
9290
export const warning = (input: LogInput<'W'>, ...args: unknown[]) => {
93-
return formatMessageAndLog('warn', false, input, undefined, ...args);
91+
return formatMessageAndLog('warn', input, undefined, ...args);
9492
};
9593

9694
/**
@@ -105,22 +103,22 @@ export const warning = (input: LogInput<'W'>, ...args: unknown[]) => {
105103
* ```
106104
*/
107105
export const info = (input: LogInput<'I'>, ...args: unknown[]) => {
108-
return formatMessageAndLog('info', false, input, undefined, ...args);
106+
return formatMessageAndLog('info', input, undefined, ...args);
109107
};
110108

111109
/**
112-
* Logs an info level message to stdout.
110+
* Logs an result. In the CLI, this always goes to stdout.
113111
*
114112
* Can be used in multiple ways:
115113
* ```ts
116-
* data(`${JSON.stringify(stats)}`) // infers default info code `CDK_TOOLKIT_I000`
117-
* data('{"count": %d}', count) // infers default info code `CDK_TOOLKIT_I000`
118-
* data({ message: 'stats: %j', code: 'CDK_DATA_I001' }) // specifies info code `CDK_DATA_I001`
119-
* data({ message: 'stats: %j', code: 'CDK_DATA_I001' }, stats) // specifies info code `CDK_DATA_I001`
114+
* result(`${JSON.stringify(stats)}`) // infers default info code `CDK_TOOLKIT_I000`
115+
* result('{"count": %d}', count) // infers default info code `CDK_TOOLKIT_I000`
116+
* result({ message: 'stats: %j', code: 'CDK_DATA_I001' }) // specifies info code `CDK_DATA_I001`
117+
* result({ message: 'stats: %j', code: 'CDK_DATA_I001' }, stats) // specifies info code `CDK_DATA_I001`
120118
* ```
121119
*/
122-
export const data = (input: LogInput<'I'>, ...args: unknown[]) => {
123-
return formatMessageAndLog('info', true, input, undefined, ...args);
120+
export const result = (input: LogInput<'I'>, ...args: unknown[]) => {
121+
return formatMessageAndLog('result', input, undefined, ...args);
124122
};
125123

126124
/**
@@ -135,7 +133,7 @@ export const data = (input: LogInput<'I'>, ...args: unknown[]) => {
135133
* ```
136134
*/
137135
export const debug = (input: LogInput<'I'>, ...args: unknown[]) => {
138-
return formatMessageAndLog('debug', false, input, undefined, ...args);
136+
return formatMessageAndLog('debug', input, undefined, ...args);
139137
};
140138

141139
/**
@@ -150,7 +148,7 @@ export const debug = (input: LogInput<'I'>, ...args: unknown[]) => {
150148
* ```
151149
*/
152150
export const trace = (input: LogInput<'I'>, ...args: unknown[]) => {
153-
return formatMessageAndLog('trace', false, input, undefined, ...args);
151+
return formatMessageAndLog('trace', input, undefined, ...args);
154152
};
155153

156154
/**
@@ -165,7 +163,7 @@ export const trace = (input: LogInput<'I'>, ...args: unknown[]) => {
165163
* ```
166164
*/
167165
export const success = (input: LogInput<'I'>, ...args: unknown[]) => {
168-
return formatMessageAndLog('info', false, input, chalk.green, ...args);
166+
return formatMessageAndLog('info', input, chalk.green, ...args);
169167
};
170168

171169
/**
@@ -180,5 +178,5 @@ export const success = (input: LogInput<'I'>, ...args: unknown[]) => {
180178
* ```
181179
*/
182180
export const highlight = (input: LogInput<'I'>, ...args: unknown[]) => {
183-
return formatMessageAndLog('info', false, input, chalk.bold, ...args);
181+
return formatMessageAndLog('info', input, chalk.bold, ...args);
184182
};

0 commit comments

Comments
 (0)