Skip to content

Commit 093de92

Browse files
authored
fix(aws-cdk-lib): toolkit is unaware of CDK app errors (#37294)
We want to know about the synth errors that happened in the CDK app in the Toolkit. In order to do that, we are doing the following things: We print the error code between special markers. We have chosen the guillemet to print the error codes: `«InvalidBucketName»`. It nicely braces the error, look visually appealing, and is uncommon enough that it is unlikely to appear in the output of a CDK app already accidentally. The CLI will scan `stdout` and `stderr` for text between these markers. That approach is chosen because it will work regardless of whether the CDK app is executing via jsii or not (`uncaughtException` handlers will only work for direct Node programs, not jsii programs). In order for this masterful scheme not to be foiled by a customer putting the following into their program: ```ts console.log('«IveTrickedYouIntoCollectingCustomerContent»'); ``` Whenever an error with an error code is constructed, we write the code to special file that is indicated by the `CDK_ERROR_FILE` environment variable (which will be set by the Toolkit). Only codes that appear in that file are eligible for scanning from `stderr`/`stdout`, so that we are not tricked into collecting customer content. Why don't we just take the error code in that file as gospel? Because the exception could be caught and the program continued. That an Error object is produced doesn't mean it terminates the program. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent b4bca75 commit 093de92

2 files changed

Lines changed: 70 additions & 5 deletions

File tree

packages/aws-cdk-lib/core/lib/errors.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import * as fs from 'fs';
12
import type { IConstruct } from 'constructs';
23
import { constructInfoFromConstruct } from './private/runtime-info';
34
import { captureCallStack, renderCallStackJustMyCode } from './stack-trace';
@@ -141,11 +142,13 @@ abstract class ConstructError extends Error {
141142

142143
// The "stack" field in Node.js includes the error description. If it doesn't, Node will fall back to an
143144
// ugly way of rendering the error.
144-
this.stack = `${this.name}: ${msg}\n${renderCallStackJustMyCode(captureCallStack(ctr)).join('\n')}`;
145+
this.stack = `«${this.name}» ${msg}\n${renderCallStackJustMyCode(captureCallStack(ctr)).join('\n')}`;
145146

146147
if (scope) {
147148
this.stack += `\nRelates to construct:\n${renderConstructRootPath(scope)}`;
148149
}
150+
151+
maybeWriteErrorCode(this.name);
149152
}
150153
}
151154

@@ -260,3 +263,39 @@ export function renderConstructRootPath(construct: IConstruct) {
260263

261264
return ret.join('\n');
262265
}
266+
267+
const THROWN_ERRORS = new Set<string>();
268+
269+
/**
270+
* If the appropriate environment variable is set, write this error code to a list of error codes in the given file.
271+
*
272+
* The reason we do this is so that the CLI can scan `stderr` for one of the
273+
* error codes between markers, and be confident that when it finds something
274+
* that it's not user content but an actual error we threw.
275+
*
276+
* - Why not just scan `stderr`? Because customers could put customer content
277+
* between those markers, and we would capture user content as an error code (we
278+
* explicitly don't want to do that!)
279+
*
280+
* - Why not take the error code immediately? Because the error could have been
281+
* caught; but we only want to capture the error that terminated the program.
282+
*
283+
* So we're doing a double whammy of writing potential error codes to a file, then
284+
* make sure that we find that error code in `stderr`.
285+
*/
286+
function maybeWriteErrorCode(errorCode: string) {
287+
const file = process.env.CDK_ERROR_FILE;
288+
if (!file) {
289+
return;
290+
}
291+
292+
// Only if this error is new
293+
const oldSize = THROWN_ERRORS.size;
294+
THROWN_ERRORS.add(errorCode);
295+
if (THROWN_ERRORS.size === oldSize) {
296+
return;
297+
}
298+
299+
// Update the indicated file
300+
fs.writeFileSync(file, Array.from(THROWN_ERRORS).sort().join('\n'), 'utf-8');
301+
}

packages/aws-cdk-lib/core/test/errors.test.ts

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { rm, readFile } from 'fs/promises';
2+
import * as os from 'os';
3+
import * as path from 'path';
14
import { inspect } from 'util';
25
import { Bucket } from '../../aws-s3';
36
import { App, Stack } from '../lib';
@@ -30,7 +33,7 @@ describe('ValidationError', () => {
3033
version: expect.stringMatching(/^\d+\.\d+\.\d+$/),
3134
});
3235
expect(error.message).toBe('this is an error');
33-
expect(error.stack).toContain('ValidationError: this is an error');
36+
expect(error.stack).toContain('«ValidationError» this is an error');
3437
expect(error.stack).toContain('└─ MyStack');
3538
});
3639

@@ -40,7 +43,7 @@ describe('ValidationError', () => {
4043
expect(Errors.isConstructError(error)).toBe(true);
4144
expect(Errors.isValidationError(error)).toBe(true);
4245
expect(error.name).toBe('ValidationError');
43-
expect(error.stack).toContain('ValidationError: this is an error');
46+
expect(error.stack).toContain('«ValidationError» this is an error');
4447
});
4548

4649
test('presentation of a ValidationError', () => {
@@ -55,7 +58,7 @@ describe('ValidationError', () => {
5558
// NodeJS will render an uncaught error using inspect()
5659
const errorString = inspect(e);
5760
expect(anonymizeBetweenParens(errorString)).toMatchInlineSnapshot(`
58-
"ErrorCode: There is an error here
61+
"«ErrorCode» There is an error here
5962
at <anonymous> (...)
6063
...Promise.then.completed in jest-circus...
6164
at new Promise (...)
@@ -75,14 +78,37 @@ Relates to construct:
7578
// NodeJS will render an uncaught error using inspect()
7679
const errorString = inspect(e);
7780
expect(anonymizeBetweenParens(errorString)).toMatchInlineSnapshot(`
78-
"ErrorCode: There is an error here
81+
"«ErrorCode» There is an error here
7982
at <anonymous> (...)
8083
...Promise.then.completed in jest-circus...
8184
at new Promise (...)
8285
...jest-circus, node internals, jest-circus, jest-runner..."
8386
`);
8487
}
8588
});
89+
90+
test('writing error codes to disk', async () => {
91+
const file = path.join(os.tmpdir(), 'errors.txt');
92+
await rm(file, { force: true });
93+
try {
94+
process.env.CDK_ERROR_FILE = file;
95+
96+
try {
97+
throw new UnscopedValidationError('Error1', 'bla');
98+
} catch { }
99+
const contents1 = await readFile(file, 'utf-8');
100+
expect(contents1).toEqual('Error1');
101+
102+
try {
103+
throw new UnscopedValidationError('Error2', 'bla');
104+
} catch { }
105+
const contents2 = await readFile(file, 'utf-8');
106+
expect(contents2).toEqual('Error1\nError2');
107+
} finally {
108+
delete process.env.CDK_ERROR_FILE;
109+
await rm(file, { force: true });
110+
}
111+
});
86112
});
87113

88114
/**

0 commit comments

Comments
 (0)