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

chore(toolkit): capture all output from an app #33259

Merged
merged 3 commits into from
Feb 3, 2025
Merged
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
28 changes: 23 additions & 5 deletions packages/@aws-cdk/toolkit/lib/api/cloud-assembly/private/exec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import * as child_process from 'node:child_process';
import * as split2 from 'split2';
import { ToolkitError } from '../../errors';

type EventPublisher = (event: 'open' | 'data_stdout' | 'data_stderr' | 'close', line: string) => void;

interface ExecOptions {
eventPublisher?: EventPublisher;
extraEnv?: { [key: string]: string | undefined };
cwd?: string;
}
Expand All @@ -17,12 +21,10 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
// number of quoting issues introduced by the intermediate shell layer
// (which would be different between Linux and Windows).
//
// - Inherit stderr from controlling terminal. We don't use the captured value
// anyway, and if the subprocess is printing to it for debugging purposes the
// user gets to see it sooner. Plus, capturing doesn't interact nicely with some
// processes like Maven.
// - We have to capture any output to stdout and stderr sp we can pass it on to the IoHost
// To ensure messages get to the user fast, we will emit every full line we receive.
const proc = child_process.spawn(commandAndArgs, {
stdio: ['ignore', 'inherit', 'inherit'],
stdio: ['ignore', 'pipe', 'pipe'],
detached: false,
shell: true,
cwd: options.cwd,
Expand All @@ -32,6 +34,22 @@ export async function execInChildProcess(commandAndArgs: string, options: ExecOp
},
});

const eventPublisher: EventPublisher = options.eventPublisher ?? ((type, line) => {
switch (type) {
case 'data_stdout':
process.stdout.write(line);
return;
case 'data_stderr':
process.stderr.write(line);
return;
case 'open':
case 'close':
return;
}
});
proc.stdout.pipe(split2()).on('data', (line) => eventPublisher('data_stdout', line));
proc.stderr.pipe(split2()).on('data', (line) => eventPublisher('data_stderr', line));

proc.on('error', fail);

proc.on('exit', code => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ import { assemblyFromDirectory, changeDir, determineOutputDirectory, guessExecut
import { ToolkitServices } from '../../../toolkit/private';
import { Context, ILock, RWLock, Settings } from '../../aws-cdk';
import { ToolkitError } from '../../errors';
import { debug } from '../../io/private';
import { debug, error, info } from '../../io/private';
import { AssemblyBuilder, CdkAppSourceProps } from '../source-builder';

export abstract class CloudAssemblySourceBuilder {
/**
* Helper to provide the CloudAssemblySourceBuilder with required toolkit services
* @deprecated this should move to the toolkit really.
*/
protected abstract toolkitServices(): Promise<ToolkitServices>;
protected abstract sourceBuilderServices(): Promise<ToolkitServices>;

/**
* Create a Cloud Assembly from a Cloud Assembly builder function.
Expand All @@ -27,7 +27,7 @@ export abstract class CloudAssemblySourceBuilder {
builder: AssemblyBuilder,
props: CdkAppSourceProps = {},
): Promise<ICloudAssemblySource> {
const services = await this.toolkitServices();
const services = await this.sourceBuilderServices();
const context = new Context({ bag: new Settings(props.context ?? {}) });
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
services,
Expand Down Expand Up @@ -65,7 +65,7 @@ export abstract class CloudAssemblySourceBuilder {
* @returns the CloudAssembly source
*/
public async fromAssemblyDirectory(directory: string): Promise<ICloudAssemblySource> {
const services: ToolkitServices = await this.toolkitServices();
const services: ToolkitServices = await this.sourceBuilderServices();
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
services,
context: new Context(), // @todo there is probably a difference between contextaware and contextlookup sources
Expand All @@ -89,7 +89,7 @@ export abstract class CloudAssemblySourceBuilder {
* @returns the CloudAssembly source
*/
public async fromCdkApp(app: string, props: CdkAppSourceProps = {}): Promise<ICloudAssemblySource> {
const services: ToolkitServices = await this.toolkitServices();
const services: ToolkitServices = await this.sourceBuilderServices();
// @todo this definitely needs to read files from the CWD
const context = new Context({ bag: new Settings(props.context ?? {}) });
const contextAssemblyProps: ContextAwareCloudAssemblyProps = {
Expand Down Expand Up @@ -122,7 +122,20 @@ export abstract class CloudAssemblySourceBuilder {

const env = await prepareDefaultEnvironment(services, { outdir });
return await withContext(context.all, env, props.synthOptions, async (envWithContext, _ctx) => {
await execInChildProcess(commandLine.join(' '), { extraEnv: envWithContext, cwd: props.workingDirectory });
await execInChildProcess(commandLine.join(' '), {
eventPublisher: async (type, line) => {
switch (type) {
case 'data_stdout':
await services.ioHost.notify(info(line, 'CDK_ASSEMBLY_I1001'));
break;
case 'data_stderr':
await services.ioHost.notify(error(line, 'CDK_ASSEMBLY_E1002'));
break;
}
},
extraEnv: envWithContext,
cwd: props.workingDirectory,
});
return assemblyFromDirectory(outdir, services.ioHost);
});
} finally {
Expand Down
22 changes: 4 additions & 18 deletions packages/@aws-cdk/toolkit/lib/api/io/private/codes.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,6 @@
import { IoMessageCode } from '../io-message';

export const CODES = {
// Default codes -- all 0000 codes
CDK_TOOLKIT_I0000: 'Default toolkit info code',
CDK_TOOLKIT_E0000: 'Default toolkit error code',
CDK_TOOLKIT_W0000: 'Default toolkit warning code',
CDK_SDK_I0000: 'Default sdk info code',
CDK_SDK_E0000: 'Default sdk error code',
CDK_SDK_WOOOO: 'Default sdk warning code',
CDK_ASSETS_I0000: 'Default assets info code',
CDK_ASSETS_E0000: 'Default assets error code',
CDK_ASSETS_W0000: 'Default assets warning code',
CDK_ASSEMBLY_I0000: 'Default assembly info code',
CDK_ASSEMBLY_E0000: 'Default assembly error code',
CDK_ASSEMBLY_W0000: 'Default assembly warning code',
Comment on lines -4 to -16
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't want to allow explicitly using this code for now. When generating the docs page we can probably inject the description differently.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would break the place where we generate 0000 codes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does not apparently!


// Toolkit Info codes
CDK_TOOLKIT_I0001: 'Display stack data',
CDK_TOOLKIT_I0002: 'Successfully deployed stacks',
Expand All @@ -31,10 +17,10 @@ export const CODES = {
// Assembly Info codes
CDK_ASSEMBLY_I0042: 'Writing updated context',
CDK_ASSEMBLY_I0241: 'Fetching missing context',

// Assembly Warning codes

// Assembly Error codes
CDK_ASSEMBLY_I1000: 'Cloud assembly output starts',
CDK_ASSEMBLY_I1001: 'Output lines emitted by the cloud assembly to stdout',
CDK_ASSEMBLY_E1002: 'Output lines emitted by the cloud assembly to stderr',
CDK_ASSEMBLY_I1003: 'Cloud assembly output finished',
CDK_ASSEMBLY_E1111: 'Incompatible CDK CLI version. Upgrade needed.',
};

Expand Down
6 changes: 3 additions & 3 deletions packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export interface ToolkitOptions {

/**
* Whether to allow ANSI colors and formatting in IoHost messages.
* Setting this value to `falsez enforces that no color or style shows up
* Setting this value to `false` enforces that no color or style shows up
* in messages sent to the IoHost.
* Setting this value to true is a no-op; it is equivalent to the default.
*
Expand Down Expand Up @@ -144,9 +144,9 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
/**
* Helper to provide the CloudAssemblySourceBuilder with required toolkit services
*/
protected override async toolkitServices(): Promise<ToolkitServices> {
protected override async sourceBuilderServices(): Promise<ToolkitServices> {
return {
ioHost: this.ioHost,
ioHost: withAction(this.ioHost, 'assembly'),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Messages previously came through without an action.

sdkProvider: await this.sdkProvider('assembly'),
};
}
Expand Down
2 changes: 2 additions & 0 deletions packages/@aws-cdk/toolkit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
"@types/fs-extra": "^9.0.13",
"@types/jest": "^29.5.14",
"@types/node": "^18.18.14",
"@types/split2": "^4.2.3",
"aws-cdk": "0.0.0",
"aws-cdk-lib": "0.0.0",
"aws-sdk-client-mock": "^4.0.1",
Expand Down Expand Up @@ -105,6 +106,7 @@
"promptly": "^3.2.0",
"proxy-agent": "^6.4.0",
"semver": "^7.6.3",
"split2": "^4.2.0",
"strip-ansi": "^6.0.1",
"table": "^6.8.2",
"uuid": "^8.3.2",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as cdk from 'aws-cdk-lib/core';

console.log('line one');
const app = new cdk.App();
console.log('line two');
new cdk.Stack(app, 'Stack1');
console.log('line three');
app.synth();
console.log('line four');
11 changes: 11 additions & 0 deletions packages/@aws-cdk/toolkit/test/_fixtures/validation-error/app.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as cdk from 'aws-cdk-lib/core';
import * as sqs from 'aws-cdk-lib/aws-sqs';

const app = new cdk.App();
const stack = new cdk.Stack(app, 'Stack1');
new sqs.Queue(stack, 'Queue1', {
queueName: "Queue1",
fifo: true,
});

app.synth();
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,38 @@ describe('fromCdkApp', () => {
// THEN
expect(JSON.stringify(stack)).toContain('amzn-s3-demo-bucket');
});

test('will capture error output', async () => {
// WHEN
const cx = await appFixture(toolkit, 'validation-error');
try {
await cx.produce();
} catch {
// we are just interested in the output for this test
}

// THEN
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
level: 'error',
code: 'CDK_ASSEMBLY_E1002',
message: expect.stringContaining('ValidationError'),
}));
});

test('will capture all output', async () => {
// WHEN
const cx = await appFixture(toolkit, 'console-output');
await cx.produce();

// THEN
['one', 'two', 'three', 'four'].forEach((line) => {
expect(ioHost.notifySpy).toHaveBeenCalledWith(expect.objectContaining({
level: 'info',
code: 'CDK_ASSEMBLY_I1001',
message: `line ${line}`,
}));
});
});
});

describe('fromAssemblyDirectory', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/lib/toolkit/cli-io-host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ export const levelPriority: Record<IoMessageLevel, number> = {
* The current action being performed by the CLI. 'none' represents the absence of an action.
*/
export type ToolkitAction =
| 'assembly'
| 'bootstrap'
| 'synth'
| 'list'
Expand Down
12 changes: 12 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -9422,6 +9422,13 @@
resolved "https://registry.npmjs.org/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-8.1.5.tgz#5fd3592ff10c1e9695d377020c033116cc2889f2"
integrity sha512-mQkU2jY8jJEF7YHjHvsQO8+3ughTL1mcnn96igfhONmR+fUPSKIkefQYpSe8bsly2Ep7oQbn/6VG5/9/0qcArQ==

"@types/split2@^4.2.3":
version "4.2.3"
resolved "https://registry.npmjs.org/@types/split2/-/split2-4.2.3.tgz#ddd9b6b8518df6e0a7825851fcd98de12e415f0b"
integrity sha512-59OXIlfUsi2k++H6CHgUQKEb2HKRokUA39HY1i1dS8/AIcqVjtAAFdf8u+HxTWK/4FUHMJQlKSZ4I6irCBJ1Zw==
dependencies:
"@types/node" "*"

"@types/stack-utils@^2.0.0":
version "2.0.3"
resolved "https://registry.npmjs.org/@types/stack-utils/-/stack-utils-2.0.3.tgz#6209321eb2c1712a7e7466422b8cb1fc0d9dd5d8"
Expand Down Expand Up @@ -19005,6 +19012,11 @@ split2@^3.0.0, split2@^3.2.2:
dependencies:
readable-stream "^3.0.0"

split2@^4.2.0:
version "4.2.0"
resolved "https://registry.npmjs.org/split2/-/split2-4.2.0.tgz#c9c5920904d148bab0b9f67145f245a86aadbfa4"
integrity sha512-UcjcJOWknrNkF6PLX83qcHM6KHgVKNkV62Y8a5uYDVv9ydGQVwAHMKqHdJje1VTWpljG0WYpCDhrCdAOYH4TWg==

split@^1.0.0, split@^1.0.1:
version "1.0.1"
resolved "https://registry.npmjs.org/split/-/split-1.0.1.tgz#605bd9be303aa59fb35f9229fbea0ddec9ea07d9"
Expand Down
Loading