Skip to content

Commit

Permalink
fix(cli): stack monitor prints over error messages (aws#19859)
Browse files Browse the repository at this point in the history
In aws#19742, we limited the amount of text that the status printer
would produce, and tried to get rid of the empty lines at the end
of the rewritable block as well.

Unfortunately, we scrolled up too far, overwriting the error messages
that may occur during resource provisioning. Since these were the only
place where error messages were displayed, it is now not possible to see
why a stack deployment fails.

This PR fixes that.


----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored and Stephen Potter committed Apr 27, 2022
1 parent 690d239 commit f5a0386
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,7 @@ export class CurrentActivityPrinter extends ActivityPrinterBase {

// Display in the same block space, otherwise we're going to have silly empty lines.
this.block.displayLines(lines);
this.block.removeEmptyLines(lines);
this.block.removeEmptyLines();
}

private progressBar(width: number) {
Expand Down Expand Up @@ -745,7 +745,7 @@ function colorFromStatusActivity(status?: string) {
return chalk.red;
}

if (status.startsWith('CREATE_') || status.startsWith('UPDATE_')) {
if (status.startsWith('CREATE_') || status.startsWith('UPDATE_') || status.startsWith('IMPORT_')) {
return chalk.green;
}
// For stacks, it may also be 'UPDDATE_ROLLBACK_IN_PROGRESS'
Expand Down
33 changes: 15 additions & 18 deletions packages/aws-cdk/lib/api/util/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const wrapAnsi = require('wrap-ansi');
*/
export class RewritableBlock {
private lastHeight = 0;
private trailingEmptyLines = 0;

constructor(private readonly stream: NodeJS.WriteStream) {
}
Expand All @@ -22,23 +23,27 @@ export class RewritableBlock {
}

public displayLines(lines: string[]) {
lines = terminalWrap(this.width, expandNewlines(lines)).slice(0, getMaxBlockHeight(this.height, this.lastHeight, lines));
lines = terminalWrap(this.width, expandNewlines(lines));
lines = lines.slice(0, getMaxBlockHeight(this.height, this.lastHeight, lines));

this.stream.write(cursorUp(this.lastHeight));
for (const line of lines) {
this.stream.write(cll() + line + '\n');
}

this.trailingEmptyLines = Math.max(0, this.lastHeight - lines.length);

// Clear remainder of unwritten lines
for (let i = 0; i < this.lastHeight - lines.length; i++) {
for (let i = 0; i < this.trailingEmptyLines; i++) {
this.stream.write(cll() + '\n');
}

// The block can only ever get bigger
this.lastHeight = Math.max(this.lastHeight, lines.length);
}

public removeEmptyLines(lines: string[]) {
this.stream.write(cursorUp(this.lastHeight - lines.length));
public removeEmptyLines() {
this.stream.write(cursorUp(this.trailingEmptyLines));
}
}

Expand All @@ -62,26 +67,18 @@ function cll() {
function terminalWrap(width: number | undefined, lines: string[]) {
if (width === undefined) { return lines; }

const ret = new Array<string>();
for (const line of lines) {
ret.push(...wrapAnsi(line, width - 1, {
hard: true,
trim: true,
wordWrap: false,
}).split('\n'));
}
return ret;
return lines.flatMap(line => wrapAnsi(line, width - 1, {
hard: true,
trim: true,
wordWrap: false,
}).split('\n'));
}

/**
* Make sure there are no hidden newlines in the gin strings
*/
function expandNewlines(lines: string[]): string[] {
const ret = new Array<string>();
for (const line of lines) {
ret.push(...line.split('\n'));
}
return ret;
return lines.flatMap(line => line.split('\n'));
}

function getMaxBlockHeight(windowHeight: number | undefined, lastHeight: number, lines: string[]): number {
Expand Down
23 changes: 23 additions & 0 deletions packages/aws-cdk/test/api/util/display.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,27 @@ describe('Rewritable Block Tests', () => {

expect(output.length).toEqual(6);
});

test('display accounts for newlines in output', () => {
const output = stderr.inspectSync(() => {
block.displayLines(['before\nafter']);
});
expect(output.length).toEqual(3); // cursorup + 2 lines
});

test('removeEmptyLines only removes trailing lines', () => {
stderr.inspectSync(() => {
block.displayLines(Array.from(Array(5).keys()).map(x => `${x}`));
});
stderr.inspectSync(() => {
// Leaves 3 empty lines
block.displayLines(Array.from(Array(2).keys()).map(x => `${x}`));
});

const output = stderr.inspectSync(() => {
block.removeEmptyLines();
});
const expectedEmptyLines = 3;
expect(JSON.stringify(output)).toEqual(JSON.stringify([`\u001b[${expectedEmptyLines}A`]));
});
});

0 comments on commit f5a0386

Please sign in to comment.