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

fix(cli): write notices to stderr or don't write them at all #188

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions packages/@aws-cdk/toolkit-lib/lib/api/io/private/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,24 @@ export const IO = {
interface: 'ErrorPayload',
}),

// Notices
CDK_TOOLKIT_I0100: make.info({
code: 'CDK_TOOLKIT_I0100',
description: 'Notices decoration (the header or footer of a list of notices)',
}),
CDK_TOOLKIT_W0101: make.warn({
code: 'CDK_TOOLKIT_W0101',
description: 'A notice that is marked as a warning',
}),
CDK_TOOLKIT_E0101: make.error({
code: 'CDK_TOOLKIT_E0101',
description: 'A notice that is marked as an error',
}),
CDK_TOOLKIT_I0101: make.info({
code: 'CDK_TOOLKIT_I0101',
description: 'A notice that is marked as informational',
}),

// Assembly codes
CDK_ASSEMBLY_I0010: make.debug({
code: 'CDK_ASSEMBLY_I0010',
Expand Down
83 changes: 83 additions & 0 deletions packages/aws-cdk/lib/cli/ci-systems.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@

interface CiSystem {
/**
* What's the name?
*/
readonly name: string;

/**
* What environment variable indicates that we are running on this system?
*/
readonly detectEnvVar: string;

/**
* Whether or not this CI system can be configured to fail on messages written to stderr
*
* With "can be configured", what we mean is that a checkbox or configuration
* flag to enable this behavior comes out of the box with the CI system and (judgement
* call), this flag is "commonly" used.
*
* Of course every CI system can be scripted to have this behavior, but that's
* not what we mean.
*/
readonly canBeConfiguredToFailOnStdErr: boolean;
}

const CI_SYSTEMS: CiSystem[] = [
{
name: 'Azure DevOps',
// https://learn.microsoft.com/en-us/azure/devops/pipelines/build/variables?view=azure-devops&tabs=yaml
detectEnvVar: 'TF_BUILD',
canBeConfiguredToFailOnStdErr: true,
},
{
name: 'TeamCity',
// https://www.jetbrains.com/help/teamcity/predefined-build-parameters.html
detectEnvVar: 'TEAMCITY_VERSION',
// Can be configured to fail on stderr, when using a PowerShell task
canBeConfiguredToFailOnStdErr: true,
},
{
name: 'GitHub Actions',
// https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#default-environment-variables
detectEnvVar: 'GITHUB_ACTION',
canBeConfiguredToFailOnStdErr: false,
},
{
name: 'CodeBuild',
// https://docs.aws.amazon.com/codebuild/latest/userguide/build-env-ref-env-vars.html
detectEnvVar: 'CODEBUILD_BUILD_ID',
canBeConfiguredToFailOnStdErr: false,
},
{
name: 'CircleCI',
// https://circleci.com/docs/variables/#built-in-environment-variables
detectEnvVar: 'CIRCLECI',
canBeConfiguredToFailOnStdErr: false,
},
{
name: 'Jenkins',
// https://www.jenkins.io/doc/book/pipeline/jenkinsfile/#using-environment-variables
detectEnvVar: 'EXECUTOR_NUMBER',
canBeConfiguredToFailOnStdErr: false,
},
];

export function detectCiSystem(): CiSystem | undefined {
for (const ciSystem of CI_SYSTEMS) {
if (process.env[ciSystem.detectEnvVar]) {
return ciSystem;
}
}
return undefined;
}

/**
* Return whether the CI system we're detecting is safe to write to stderr on
*
* Returns `undefined` if the current CI system cannot be recognized.
*/
export function ciSystemIsStdErrSafe(): boolean | undefined {
const x = detectCiSystem()?.canBeConfiguredToFailOnStdErr;
return x === undefined ? undefined : !x;
}
24 changes: 22 additions & 2 deletions packages/aws-cdk/lib/cli/cli.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import * as cxapi from '@aws-cdk/cx-api';
import * as chalk from 'chalk';
import { CdkToolkit, AssetBuildTime } from './cdk-toolkit';
import { ciSystemIsStdErrSafe } from './ci-systems';
import { parseCommandLineArguments } from './parse-command-line-arguments';
import { checkForPlatformWarnings } from './platform-warnings';

Expand Down Expand Up @@ -89,10 +90,24 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
});
await configuration.load();

const shouldDisplayNotices = configuration.settings.get(['notices']);
if (shouldDisplayNotices !== undefined) {
// Notices either go to stderr, or nowhere
ioHost.noticesDestination = shouldDisplayNotices ? 'stderr' : 'drop';
} else {
// If the user didn't supply either `--notices` or `--no-notices`, we do
// autodetection. The autodetection currently is: do write notices if we are
// not on CI, or are on a CI system where we know that writing to stderr is
// safe. We fail "closed"; that is, we decide to NOT print for unknown CI
// systems, even though technically we maybe could.
const safeToWriteToStdErr = !argv.ci || Boolean(ciSystemIsStdErrSafe());
ioHost.noticesDestination = safeToWriteToStdErr ? 'stderr' : 'drop';
}

const notices = Notices.create({
ioHost,
context: configuration.context,
output: configuration.settings.get(['outdir']),
shouldDisplay: configuration.settings.get(['notices']),
includeAcknowledged: cmd === 'notices' ? !argv.unacknowledged : false,
httpOptions: {
proxyAddress: configuration.settings.get(['proxy']),
Expand Down Expand Up @@ -456,7 +471,12 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n

case 'notices':
ioHost.currentAction = 'notices';
// This is a valid command, but we're postponing its execution
// If the user explicitly asks for notices, they are now the primary output
// of the command and they should go to stdout.
ioHost.noticesDestination = 'stdout';

// This is a valid command, but we're postponing its execution because displaying
// notices automatically happens after every command.
return;

case 'metadata':
Expand Down
92 changes: 68 additions & 24 deletions packages/aws-cdk/lib/logging.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import * as util from 'util';
import * as chalk from 'chalk';
import { IoMessageLevel, IoMessage, CliIoHost, IoMessageSpecificCode, IoMessageCode, IoMessageCodeCategory, IoCodeLevel } from './toolkit/cli-io-host';
import { IoMessageLevel, IoMessage, CliIoHost, IoMessageSpecificCode, IoMessageCode, IoMessageCodeCategory, IoCodeLevel, IIoHost, ToolkitAction } from './toolkit/cli-io-host';

/**
* Logs messages to the global CliIoHost
*
* Internal helper that processes log inputs into a consistent format.
* Handles string interpolation, format strings, and object parameter styles.
* Applies optional styling and sends the message to the IoHost.
Expand All @@ -13,27 +15,8 @@ function formatMessageAndLog(
style?: (str: string) => string,
...args: unknown[]
): void {
// Extract message and code from input, using new default code format
const { message, code = getDefaultCode(level) } = typeof input === 'object' ? input : { message: input };

// Format message if args are provided
const formattedMessage = args.length > 0
? util.format(message, ...args)
: message;

// Apply style if provided
const finalMessage = style ? style(formattedMessage) : formattedMessage;

const ioHost = CliIoHost.instance();
const ioMessage: IoMessage<undefined> = {
time: new Date(),
action: ioHost.currentAction,
level,
message: finalMessage,
code,
};

void ioHost.notify(ioMessage);
const singletonHost = CliIoHost.instance();
new IoHostEmitter(singletonHost, singletonHost.currentAction).emit(level, input, style, ...args);
}

function getDefaultCode(level: IoMessageLevel, category: IoMessageCodeCategory = 'TOOLKIT'): IoMessageCode {
Expand Down Expand Up @@ -132,7 +115,7 @@ export const result = (input: LogInput<'I'>, ...args: unknown[]) => {
* debug({ message: 'ratio: %d%%', code: 'CDK_TOOLKIT_I001' }, ratio) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const debug = (input: LogInput<'I'>, ...args: unknown[]) => {
export const debug = (input: LogInput<'D'>, ...args: unknown[]) => {
return formatMessageAndLog('debug', input, undefined, ...args);
};

Expand All @@ -147,7 +130,7 @@ export const debug = (input: LogInput<'I'>, ...args: unknown[]) => {
* trace({ message: 'method: %s', code: 'CDK_TOOLKIT_I001' }, name) // specifies info code `CDK_TOOLKIT_I001`
* ```
*/
export const trace = (input: LogInput<'I'>, ...args: unknown[]) => {
export const trace = (input: LogInput<'D'>, ...args: unknown[]) => {
return formatMessageAndLog('trace', input, undefined, ...args);
};

Expand Down Expand Up @@ -180,3 +163,64 @@ export const success = (input: LogInput<'I'>, ...args: unknown[]) => {
export const highlight = (input: LogInput<'I'>, ...args: unknown[]) => {
return formatMessageAndLog('info', input, chalk.bold, ...args);
};

/**
* Helper class to emit messages to an IoHost
*
* Messages sent to the IoHost require the current action, so this class holds on
* to an IoHost and an action.
*
* It also contains convenience methods for the various log levels, same as the global
* ones but scoped to a particular `IoHost` instead of the global singleton one.
*/
export class IoHostEmitter {
constructor(private readonly ioHost: IIoHost, private readonly action: ToolkitAction) {
}

public error(input: LogInput<'E'>, ...args: unknown[]) {
this.emit('error', input, undefined, ...args);
}

public warning(input: LogInput<'W'>, ...args: unknown[]) {
this.emit('warn', input, undefined, ...args);
}

public info(input: LogInput<'I'>, ...args: unknown[]) {
this.emit('info', input, undefined, ...args);
}

public result(input: LogInput<'I'>, ...args: unknown[]) {
this.emit('result', input, undefined, ...args);
}

public debug(input: LogInput<'D'>, ...args: unknown[]) {
this.emit('debug', input, undefined, ...args);
}

public trace(input: LogInput<'D'>, ...args: unknown[]) {
this.emit('trace', input, undefined, ...args);
}

public emit(level: IoMessageLevel, input: LogInput<IoCodeLevel>, style?: (str: string) => string, ...args: unknown[]) {
// Extract message and code from input, using new default code format
const { message, code = getDefaultCode(level) } = typeof input === 'object' ? input : { message: input };

// Format message if args are provided
const formattedMessage = args.length > 0
? util.format(message, ...args)
: message;

// Apply style if provided
const finalMessage = style ? style(formattedMessage) : formattedMessage;

const ioMessage: IoMessage<undefined> = {
time: new Date(),
action: this.action,
level,
message: finalMessage,
code,
};

void this.ioHost.notify(ioMessage);
}
}
Loading
Loading