Skip to content

Commit

Permalink
chore: improve error reporting for asset staging (#24596)
Browse files Browse the repository at this point in the history 
Ensure the asset staging error reporting includes the full command being run, which helps with troubleshooting failures. Also, indent the content of `STDOUT` and `STDERR` to facilitate reading when those traces are long.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
@RomainMuller
RomainMuller authored Mar 14, 2023
1 parent 30beb10 commit 1105514
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 22 deletions.
22 changes: 19 additions & 3 deletions packages/@aws-cdk/core/lib/private/asset-staging.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ export class AssetBundlingVolumeCopy extends AssetBundlingBase {
export function dockerExec(args: string[], options?: SpawnSyncOptions) {
const prog = process.env.CDK_DOCKER ?? 'docker';
const proc = spawnSync(prog, args, options ?? {
encoding: 'utf-8',
stdio: [ // show Docker output
'ignore', // ignore stdio
process.stderr, // redirect stdout to stderr
Expand All @@ -211,10 +212,25 @@ export function dockerExec(args: string[], options?: SpawnSyncOptions) {
}

if (proc.status !== 0) {
if (proc.stdout || proc.stderr) {
throw new Error(`[Status ${proc.status}] stdout: ${proc.stdout?.toString().trim()}\n\n\nstderr: ${proc.stderr?.toString().trim()}`);
const reason = proc.signal != null
? `signal ${proc.signal}`
: `status ${proc.status}`;
const command = [prog, ...args.map((arg) => /[^a-z0-9_-]/i.test(arg) ? JSON.stringify(arg) : arg)].join(' ');

function prependLines(firstLine: string, text: Buffer | string | undefined): string[] {
if (!text || text.length === 0) {
return [];
}
const padding = ' '.repeat(firstLine.length);
return text.toString('utf-8').split('\n').map((line, idx) => `${idx === 0 ? firstLine : padding}${line}`);
}
throw new Error(`${prog} exited with status ${proc.status}`);

throw new Error([
`${prog} exited with ${reason}`,
...prependLines('--> STDOUT: ', proc.stdout ) ?? [],
...prependLines('--> STDERR: ', proc.stderr ) ?? [],
`--> Command: ${command}`,
].join('\n'));
}

return proc;
Expand Down
18 changes: 9 additions & 9 deletions packages/@aws-cdk/core/test/bundling.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('bundling with image from asset', () => {
Expand Down Expand Up @@ -188,7 +188,7 @@ describe('bundling', () => {
});

const image = DockerImage.fromRegistry('alpine');
expect(() => image.run()).toThrow(/\[Status -1\]/);
expect(() => image.run()).toThrow(/exited with status -1/);
});

test('BundlerDockerImage json is the bundler image name by default', () => {
Expand Down Expand Up @@ -294,7 +294,7 @@ describe('bundling', () => {
'alpine',
'--cool-entrypoint-arg',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('cp utility copies from an image', () => {
Expand Down Expand Up @@ -404,7 +404,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('adding user provided network options', () => {
Expand Down Expand Up @@ -437,7 +437,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('adding user provided docker volume options', () => {
Expand Down Expand Up @@ -475,7 +475,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('ensure selinux docker mount', () => {
Expand Down Expand Up @@ -516,7 +516,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('ensure selinux docker mount on linux with selinux disabled', () => {
Expand Down Expand Up @@ -557,7 +557,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('ensure no selinux docker mount if selinuxenabled isn\'t an available command', () => {
Expand Down Expand Up @@ -598,7 +598,7 @@ describe('bundling', () => {
'-w', '/working-directory',
'alpine',
'cool', 'command',
], { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
], { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});

test('ensure correct Docker CLI arguments are returned', () => {
Expand Down
20 changes: 10 additions & 10 deletions packages/@aws-cdk/core/test/private/asset-staging.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,20 +33,20 @@ describe('bundling', () => {
// volume Creation
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'volume', 'create', sinon.match(/assetInput.*/g),
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'volume', 'create', sinon.match(/assetOutput.*/g),
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// volume removal
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'volume', 'rm', sinon.match(/assetInput.*/g),
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'volume', 'rm', sinon.match(/assetOutput.*/g),
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// prepare copy container
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
Expand All @@ -58,29 +58,29 @@ describe('bundling', () => {
'sh',
'-c',
`mkdir -p ${AssetStaging.BUNDLING_INPUT_DIR} && chown -R ${options.user} ${AssetStaging.BUNDLING_OUTPUT_DIR} && chown -R ${options.user} ${AssetStaging.BUNDLING_INPUT_DIR}`,
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// delete copy container
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'rm', sinon.match(/copyContainer.*/g),
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// copy files to copy container
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'cp', `${options.sourcePath}/.`, `${helper.copyContainerName}:${AssetStaging.BUNDLING_INPUT_DIR}`,
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// copy files from copy container to host
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match([
'cp', `${helper.copyContainerName}:${AssetStaging.BUNDLING_OUTPUT_DIR}/.`, options.bundleDir,
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

// actual docker run
expect(spawnSyncStub.calledWith(DOCKER_CMD, sinon.match.array.contains([
'run', '--rm',
'--volumes-from', helper.copyContainerName,
'alpine',
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);

});

Expand Down Expand Up @@ -109,6 +109,6 @@ describe('bundling', () => {
'run', '--rm',
'-v',
'alpine',
]), { stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
]), { encoding: 'utf-8', stdio: ['ignore', process.stderr, 'inherit'] })).toEqual(true);
});
});

0 comments on commit 1105514

Please sign in to comment.