fix(cli): remove source maps (#32317)

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.

Closes #19930.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Author: rix0rrr

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.

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');

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) => {
@@ -568,7 +569,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;

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 },

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', {

packages/aws-cdk/lib/version.ts

@@ -15,6 +15,10 @@ const UPGRADE_DOCUMENTATION_LINKS: Record<number, string> = {
 
 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]+$/, '');

packages/aws-cdk/package.json

@@ -57,7 +57,6 @@
       "entryPoints": [
         "lib/index.js"
       ],
-      "sourcemap": "linked",
       "minifyWhitespace": true
     }
   },

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
+});

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,