From 80876cbc5921922bb670a6480a67e4af6d3262f0 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 28 Nov 2024 11:02:46 +0100 Subject: [PATCH 1/3] fix(cli): remove source maps Remove the source maps from the bundled CLI. The source maps are not really useful for customers anyway, and have the following downsides: - They are 30+MB, which we vend to customers for no benefit. - They tend to slow down Node as it loads and processes them. We have reports that on some Node versions this even leads to socket timeouts as the Node process was too busy loading source maps (#19930). There are 2 steps to producing a CLI build: - First compile with TypeScript -> JavaScript. Produces sourcemaps that are still being loaded. - Then bundle JavaScript -> bundle. This removes sourcemaps. Developers running a local (non-bundled) build will benefit from the source maps generated by TypeScript. Two other changes in this PR that came up around this: * Clarify what the `--debug` flag is for (debugging the CDK app) and what it's not for (debugging the CLI) * Only print the stack trace in a CLI error if we're on a developer build; due to the minification printing the stack trace on a bundled copy prints a 1000-character minified line which is not useful to anyone. --- packages/aws-cdk/CONTRIBUTING.md | 17 +++++++++++++++++ packages/aws-cdk/bin/cdk | 5 ++--- packages/aws-cdk/lib/cli.ts | 5 ++++- packages/aws-cdk/lib/config.ts | 2 +- .../aws-cdk/lib/parse-command-line-arguments.ts | 2 +- packages/aws-cdk/lib/version.ts | 4 ++++ packages/aws-cdk/package.json | 1 - tools/@aws-cdk/node-bundle/src/api/bundle.ts | 2 +- 8 files changed, 30 insertions(+), 8 deletions(-) diff --git a/packages/aws-cdk/CONTRIBUTING.md b/packages/aws-cdk/CONTRIBUTING.md index fb915d28cb39c..4a8b79abe277b 100644 --- a/packages/aws-cdk/CONTRIBUTING.md +++ b/packages/aws-cdk/CONTRIBUTING.md @@ -257,3 +257,20 @@ will change, and needs to be regenerated. For you, this means that: 2. When you build the CLI locally, you must ensure your dependencies are up to date by running `yarn install` inside the CLI package. Otherwise, you might get an error like so: `aws-cdk: - [bundle/outdated-attributions] THIRD_PARTY_LICENSES is outdated (fixable)`. +## Source Maps + +The source map handling is not entirely intuitive, so it bears some description here. + +There are 2 steps to producing a CLI build: + +- First we compile TypeScript to JavaScript. This step is configured to produce inline sourcemaps. +- Then we bundle JavaScript -> bundled JavaScript. This removes the inline + sourcemaps, and also is configured to *not* emit a fresh sourcemap file. + +The upshot is that we don't vend a 30+MB sourcemap to customers that they have no use for, +and that we don't slow down Node loading those sourcemaps, while if we are locally developing +and testing the sourcemaps are still present and can be used. + +During the CLI initialization, we always enable source map support: if we are developing +then source maps are present and can be used, while in a production build there will be no +source maps so there's nothing to load anyway. \ No newline at end of file diff --git a/packages/aws-cdk/bin/cdk b/packages/aws-cdk/bin/cdk index 46528824d34a8..067aa3422918e 100755 --- a/packages/aws-cdk/bin/cdk +++ b/packages/aws-cdk/bin/cdk @@ -1,7 +1,6 @@ #!/usr/bin/env node // source maps must be enabled before importing files -if (process.argv.includes('--debug')) { - process.setSourceMapsEnabled(true); -} +process.setSourceMapsEnabled(true); + require('./cdk.js'); diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 3ae17cbd268bd..37dbdd28d65db 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -568,7 +568,10 @@ export function cli(args: string[] = process.argv.slice(2)) { }) .catch((err) => { error(err.message); - if (err.stack) { + + // Log the stack trace if we're on a developer workstation. Otherwise this will be into a minified + // file and the printed code line and stack trace are huge and useless. + if (err.stack && version.isDeveloperBuild()) { debug(err.stack); } process.exitCode = 1; diff --git a/packages/aws-cdk/lib/config.ts b/packages/aws-cdk/lib/config.ts index 349faf1126384..daeaad89d98de 100644 --- a/packages/aws-cdk/lib/config.ts +++ b/packages/aws-cdk/lib/config.ts @@ -21,7 +21,7 @@ export function makeConfig(): CliConfig { 'ignore-errors': { type: 'boolean', default: false, desc: 'Ignores synthesis errors, which will likely produce an invalid output' }, 'json': { type: 'boolean', alias: 'j', desc: 'Use JSON output instead of YAML when templates are printed to STDOUT', default: false }, 'verbose': { type: 'boolean', alias: 'v', desc: 'Show debug logs (specify multiple times to increase verbosity)', default: false, count: true }, - 'debug': { type: 'boolean', desc: 'Enable emission of additional debugging information, such as creation stack traces of tokens', default: false }, + 'debug': { type: 'boolean', desc: 'Debug the CDK app. Log additional information during synthesis, such as creation stack traces of tokens (sets CDK_DEBUG, will slow down synthesis)', default: false }, 'profile': { type: 'string', desc: 'Use the indicated AWS profile as the default environment', requiresArg: true }, 'proxy': { type: 'string', desc: 'Use the indicated proxy. Will read from HTTPS_PROXY environment variable if not specified', requiresArg: true }, 'ca-bundle-path': { type: 'string', desc: 'Path to CA certificate to use when validating HTTPS requests. Will read from AWS_CA_BUNDLE environment variable if not specified', requiresArg: true }, diff --git a/packages/aws-cdk/lib/parse-command-line-arguments.ts b/packages/aws-cdk/lib/parse-command-line-arguments.ts index a8d637cd28a98..1135df7da8911 100644 --- a/packages/aws-cdk/lib/parse-command-line-arguments.ts +++ b/packages/aws-cdk/lib/parse-command-line-arguments.ts @@ -73,7 +73,7 @@ export function parseCommandLineArguments( }) .option('debug', { type: 'boolean', - desc: 'Enable emission of additional debugging information, such as creation stack traces of tokens', + desc: 'Debug the CDK app. Log additional information during synthesis, such as creation stack traces of tokens (sets CDK_DEBUG, will slow down synthesis)', default: false, }) .option('profile', { diff --git a/packages/aws-cdk/lib/version.ts b/packages/aws-cdk/lib/version.ts index 9f21ef093cbc4..bdeee8d51b482 100644 --- a/packages/aws-cdk/lib/version.ts +++ b/packages/aws-cdk/lib/version.ts @@ -15,6 +15,10 @@ const UPGRADE_DOCUMENTATION_LINKS: Record = { export const DISPLAY_VERSION = `${versionNumber()} (build ${commit()})`; +export function isDeveloperBuild(): boolean { + return versionNumber() === '0.0.0'; +} + export function versionNumber(): string { // eslint-disable-next-line @typescript-eslint/no-require-imports return require(path.join(rootDir(), 'package.json')).version.replace(/\+[0-9a-f]+$/, ''); diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json index f91e2a9882c9e..e2caee2278ea0 100644 --- a/packages/aws-cdk/package.json +++ b/packages/aws-cdk/package.json @@ -57,7 +57,6 @@ "entryPoints": [ "lib/index.js" ], - "sourcemap": "linked", "minifyWhitespace": true } }, diff --git a/tools/@aws-cdk/node-bundle/src/api/bundle.ts b/tools/@aws-cdk/node-bundle/src/api/bundle.ts index bf8af98023b4f..a7228c02d18ce 100644 --- a/tools/@aws-cdk/node-bundle/src/api/bundle.ts +++ b/tools/@aws-cdk/node-bundle/src/api/bundle.ts @@ -443,7 +443,7 @@ export class Bundle { bundle: true, target: 'node14', platform: 'node', - sourcemap: this.sourcemap ?? 'inline', + sourcemap: this.sourcemap, metafile: true, minify: this.minify, minifyWhitespace: this.minifyWhitespace, From 981406a546585e6edde45811ac1901d37887daf4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 28 Nov 2024 12:59:23 +0100 Subject: [PATCH 2/3] Add coverage to uncovered line --- packages/aws-cdk/test/version.test.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/version.test.ts b/packages/aws-cdk/test/version.test.ts index 3c86b76fd0b85..d601fb4ec7870 100644 --- a/packages/aws-cdk/test/version.test.ts +++ b/packages/aws-cdk/test/version.test.ts @@ -7,7 +7,7 @@ import * as os from 'os'; import * as sinon from 'sinon'; import * as logging from '../lib/logging'; import * as npm from '../lib/util/npm'; -import { latestVersionIfHigher, VersionCheckTTL, displayVersionMessage } from '../lib/version'; +import { latestVersionIfHigher, VersionCheckTTL, displayVersionMessage, isDeveloperBuild } from '../lib/version'; jest.setTimeout(10_000); @@ -141,3 +141,13 @@ describe('version message', () => { expect(printSpy).not.toHaveBeenCalledWith(expect.stringContaining('Information about upgrading from 99.x to 100.x')); }); }); + +test('isDeveloperBuild call does not throw an error', () => { + // To be frank: this is just to shut CodeCov up. It don't want to make an assertion + // that the value is `true` when running tests, because I won't want to make too + // many assumptions for no good reason. + + isDeveloperBuild(); + + // THEN: should not explode +}); \ No newline at end of file From 646f49f9bed964b219dfdd5e15514604cabdc4fa Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 28 Nov 2024 15:39:46 +0100 Subject: [PATCH 3/3] Exclude main dispatch file from CodeCov --- packages/aws-cdk/lib/cli.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts index 37dbdd28d65db..267124db42de2 100644 --- a/packages/aws-cdk/lib/cli.ts +++ b/packages/aws-cdk/lib/cli.ts @@ -559,6 +559,7 @@ function determineHotswapMode(hotswap?: boolean, hotswapFallback?: boolean, watc return hotswapMode; } +/* istanbul ignore next: we never call this in unit tests */ export function cli(args: string[] = process.argv.slice(2)) { exec(args) .then(async (value) => {