fix(cli): don't double up on plugin errors and don't show pointless stack trace (#262)

Improves the error output for plugin loading. We are loosing the green
highlight of the plugin name because I doesn't feel right to have
formatting inside the error. On the upside, we are loosing stack traces
that are irrelevant to the user.

---

Old:
<img width="803" alt="image"
src="https://github.com/user-attachments/assets/82acc6de-98a0-436e-a9bf-56c06fbfea14"
/>

New: 
<img width="421" alt="image"
src="https://github.com/user-attachments/assets/b93858a7-49e1-4bf5-8c3b-9a2f4239e89d"
/>


Old:
<img width="978" alt="image"
src="https://github.com/user-attachments/assets/b2110990-9e5e-4fdb-8139-f1a992cd2c82"
/>


New: 
<img width="846" alt="image"
src="https://github.com/user-attachments/assets/553c9106-5ed0-4b71-9c29-c1db87f28444"
/>

Also changes all of `cli.ts` to use the `IoHost`

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

Author: mrgrain

.projenrc.ts

@@ -805,6 +805,7 @@ const cli = configureProject(
     tsconfig: {
       compilerOptions: {
         ...defaultTsOptions,
+        lib: ['es2019', 'es2022.error'],
 
         // Changes the meaning of 'import' for libraries whose top-level export is a function
         // 'aws-cdk' has been written against `false` for interop

packages/@aws-cdk/integ-runner/test/workers/integ-worker.test.ts

@@ -8,6 +8,9 @@ import { runIntegrationTestsInParallel, runIntegrationTests } from '../../lib/wo
 let stderrMock: jest.SpyInstance;
 let pool: workerpool.WorkerPool;
 let spawnSyncMock: jest.SpyInstance;
+
+jest.setTimeout(20_000);
+
 beforeAll(() => {
   pool = workerpool.pool(path.join(__dirname, 'mock-extract_worker.ts'), {
     workerType: 'thread',

packages/@aws-cdk/tmp-toolkit-helpers/src/api/toolkit-error.ts

@@ -37,6 +37,13 @@ export class ToolkitError extends Error {
     return this.isToolkitError(x) && CONTEXT_PROVIDER_ERROR_SYMBOL in x;
   }
 
+  /**
+   * An AssemblyError with an original error as cause
+   */
+  public static withCause(message: string, error: unknown): ToolkitError {
+    return new ToolkitError(message, 'toolkit', error);
+  }
+
   /**
    * The type of the error, defaults to "toolkit".
    */
@@ -47,13 +54,19 @@ export class ToolkitError extends Error {
    */
   public readonly source: 'toolkit' | 'user';
 
-  constructor(message: string, type: string = 'toolkit') {
+  /**
+   * The specific original cause of the error, if available
+   */
+  public readonly cause?: unknown;
+
+  constructor(message: string, type: string = 'toolkit', cause?: unknown) {
     super(message);
     Object.setPrototypeOf(this, ToolkitError.prototype);
     Object.defineProperty(this, TOOLKIT_ERROR_SYMBOL, { value: true });
     this.name = new.target.name;
     this.type = type;
     this.source = 'toolkit';
+    this.cause = cause;
   }
 }
 
@@ -106,17 +119,11 @@ export class AssemblyError extends ToolkitError {
    */
   public readonly stacks?: cxapi.CloudFormationStackArtifact[];
 
-  /**
-   * The specific original cause of the error, if available
-   */
-  public readonly cause?: unknown;
-
   private constructor(message: string, stacks?: cxapi.CloudFormationStackArtifact[], cause?: unknown) {
-    super(message, 'assembly');
+    super(message, 'assembly', cause);
     Object.setPrototypeOf(this, AssemblyError.prototype);
     Object.defineProperty(this, ASSEMBLY_ERROR_SYMBOL, { value: true });
     this.stacks = stacks;
-    this.cause = cause;
   }
 }
 

packages/@aws-cdk/tmp-toolkit-helpers/test/api/toolkit-error.test.ts

@@ -2,6 +2,7 @@ import { AssemblyError, AuthenticationError, ContextProviderError, ToolkitError
 
 describe('toolkit error', () => {
   let toolkitError = new ToolkitError('Test toolkit error');
+  let toolkitCauseError = ToolkitError.withCause('Test toolkit error', new Error('other error'));
   let authError = new AuthenticationError('Test authentication error');
   let contextProviderError = new ContextProviderError('Test context provider error');
   let assemblyError = AssemblyError.withStacks('Test authentication error', []);
@@ -25,6 +26,11 @@ describe('toolkit error', () => {
     expect(ToolkitError.isToolkitError(contextProviderError)).toBe(true);
   });
 
+  test('ToolkitError.withCause', () => {
+    expect((toolkitCauseError.cause as any)?.message).toBe('other error');
+    expect(ToolkitError.isToolkitError(toolkitCauseError)).toBe(true);
+  });
+
   test('isAuthenticationError works', () => {
     expect(authError.source).toBe('user');
 
@@ -42,7 +48,7 @@ describe('toolkit error', () => {
       expect(ToolkitError.isAssemblyError(authError)).toBe(false);
     });
 
-    test('AssemblyError.fromCause', () => {
+    test('AssemblyError.withCause', () => {
       expect(assemblyCauseError.source).toBe('user');
       expect((assemblyCauseError.cause as any)?.message).toBe('other error');
 

packages/aws-cdk/lib/api/plugin/plugin.ts

@@ -1,10 +1,7 @@
 import { inspect } from 'util';
 import type { CredentialProviderSource, IPluginHost, Plugin } from '@aws-cdk/cli-plugin-contract';
-
-import * as chalk from 'chalk';
 import { type ContextProviderPlugin, isContextProviderPlugin } from './context-provider-plugin';
 import { ToolkitError } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api';
-import { error } from '../../logging';
 
 export let TESTING = false;
 
@@ -44,15 +41,13 @@ export class PluginHost implements IPluginHost {
       const plugin = require(moduleSpec);
       /* eslint-enable */
       if (!isPlugin(plugin)) {
-        error(`Module ${chalk.green(moduleSpec)} is not a valid plug-in, or has an unsupported version.`);
-        throw new ToolkitError(`Module ${moduleSpec} does not define a valid plug-in.`);
+        throw new ToolkitError(`Module ${moduleSpec} is not a valid plug-in, or has an unsupported version.`);
       }
       if (plugin.init) {
         plugin.init(this);
       }
     } catch (e: any) {
-      error(`Unable to load ${chalk.green(moduleSpec)}: ${e.stack}`);
-      throw new ToolkitError(`Unable to load plug-in: ${moduleSpec}: ${e}`);
+      throw ToolkitError.withCause(`Unable to load plug-in '${moduleSpec}'`, e);
     }
 
     function isPlugin(x: any): x is Plugin {

packages/aws-cdk/lib/cli/cli.ts

@@ -6,6 +6,7 @@ import type { IoMessageLevel } from './io-host';
 import { CliIoHost } from './io-host';
 import { parseCommandLineArguments } from './parse-command-line-arguments';
 import { checkForPlatformWarnings } from './platform-warnings';
+import { prettyPrintError } from './pretty-print-error';
 import type { Command } from './user-configuration';
 import { Configuration } from './user-configuration';
 import * as version from './version';
@@ -32,7 +33,6 @@ import { docs } from '../commands/docs';
 import { doctor } from '../commands/doctor';
 import { cliInit, printAvailableTemplates } from '../commands/init';
 import { getMigrateScanType } from '../commands/migrate';
-import { result, debug, error, info } from '../logging';
 import { Notices } from '../notices';
 /* eslint-disable max-len */
 /* eslint-disable @typescript-eslint/no-shadow */ // yargs
@@ -80,11 +80,11 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
   try {
     await checkForPlatformWarnings();
   } catch (e) {
-    debug(`Error while checking for platform warnings: ${e}`);
+    ioHost.defaults.debug(`Error while checking for platform warnings: ${e}`);
   }
 
-  debug('CDK Toolkit CLI version:', version.displayVersion());
-  debug('Command line arguments:', argv);
+  ioHost.defaults.debug('CDK Toolkit CLI version:', version.displayVersion());
+  ioHost.defaults.debug('Command line arguments:', argv);
 
   const configuration = new Configuration({
     commandLineArguments: {
@@ -156,7 +156,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
         if (loaded.has(resolved)) {
           continue;
         }
-        debug(`Loading plug-in: ${chalk.green(plugin)} from ${chalk.blue(resolved)}`);
+        ioHost.defaults.debug(`Loading plug-in: ${chalk.green(plugin)} from ${chalk.blue(resolved)}`);
         PluginHost.instance.load(plugin);
         loaded.add(resolved);
       }
@@ -166,8 +166,11 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
       try {
         return require.resolve(plugin);
       } catch (e: any) {
-        error(`Unable to resolve plugin ${chalk.green(plugin)}: ${e.stack}`);
-        throw new ToolkitError(`Unable to resolve plug-in: ${plugin}`);
+        // according to Node.js docs `MODULE_NOT_FOUND` is the only possible error here
+        // @see https://nodejs.org/api/modules.html#requireresolverequest-options
+        // Not using `withCause()` here, since the node error contains a "Require Stack"
+        // as part of the error message that is inherently useless to our users.
+        throw new ToolkitError(`Unable to resolve plug-in: Cannot find module '${plugin}'`);
       }
     }
   }
@@ -199,7 +202,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
   async function main(command: string, args: any): Promise<number | void> {
     ioHost.currentAction = command as any;
     const toolkitStackName: string = ToolkitInfo.determineName(configuration.settings.get(['toolkitStackName']));
-    debug(`Toolkit stack: ${chalk.bold(toolkitStackName)}`);
+    ioHost.defaults.debug(`Toolkit stack: ${chalk.bold(toolkitStackName)}`);
 
     const cloudFormation = new Deployments({
       sdkProvider,
@@ -280,7 +283,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
 
       case 'bootstrap':
         ioHost.currentAction = 'bootstrap';
-        const source: BootstrapSource = determineBootstrapVersion(args);
+        const source: BootstrapSource = determineBootstrapVersion(ioHost, args);
 
         if (args.showTemplate) {
           const bootstrapper = new Bootstrapper(source, asIoHelper(ioHost, ioHost.currentAction));
@@ -521,7 +524,7 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
         });
       case 'version':
         ioHost.currentAction = 'version';
-        return result(version.displayVersion());
+        return ioHost.defaults.result(version.displayVersion());
 
       default:
         throw new ToolkitError('Unknown command: ' + command);
@@ -532,13 +535,13 @@ export async function exec(args: string[], synthesizer?: Synthesizer): Promise<n
 /**
  * Determine which version of bootstrapping
  */
-function determineBootstrapVersion(args: { template?: string }): BootstrapSource {
+function determineBootstrapVersion(ioHost: CliIoHost, args: { template?: string }): BootstrapSource {
   let source: BootstrapSource;
   if (args.template) {
-    info(`Using bootstrapping template from ${args.template}`);
+    ioHost.defaults.info(`Using bootstrapping template from ${args.template}`);
     source = { source: 'custom', templateFile: args.template };
   } else if (process.env.CDK_LEGACY_BOOTSTRAP) {
-    info('CDK_LEGACY_BOOTSTRAP set, using legacy-style bootstrapping');
+    ioHost.defaults.info('CDK_LEGACY_BOOTSTRAP set, using legacy-style bootstrapping');
     source = { source: 'legacy' };
   } else {
     // in V2, the "new" bootstrapping is the default
@@ -598,13 +601,9 @@ export function cli(args: string[] = process.argv.slice(2)) {
       }
     })
     .catch((err) => {
-      error(err.message);
-
       // 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);
-      }
+      prettyPrintError(err, version.isDeveloperBuild());
       process.exitCode = 1;
     });
 }

packages/aws-cdk/lib/cli/io-host/cli-io-host.ts

@@ -4,7 +4,7 @@ import * as chalk from 'chalk';
 import * as promptly from 'promptly';
 import { ToolkitError } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api';
 import type { IIoHost, IoMessage, IoMessageCode, IoMessageLevel, IoRequest, ToolkitAction } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api';
-import { IO, isMessageRelevantForLevel } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
+import { asIoHelper, IO, IoDefaultMessages, isMessageRelevantForLevel } from '../../../../@aws-cdk/tmp-toolkit-helpers/src/api/io/private';
 import { StackActivityProgress } from '../../commands/deploy';
 import { CurrentActivityPrinter, HistoryActivityPrinter } from '../activity-printer';
 import type { ActivityPrinterProps, IActivityPrinter } from '../activity-printer';
@@ -204,6 +204,11 @@ export class CliIoHost implements IIoHost {
     return this._progress;
   }
 
+  public get defaults() {
+    const helper = asIoHelper(this, this.currentAction as any);
+    return new IoDefaultMessages(helper);
+  }
+
   /**
    * Executes a block of code with corked logging. All log messages during execution
    * are buffered and only written when all nested cork blocks complete (when CORK_COUNTER reaches 0).

packages/aws-cdk/lib/cli/pretty-print-error.ts

@@ -0,0 +1,36 @@
+/* eslint-disable no-console */
+import * as chalk from 'chalk';
+
+export function prettyPrintError(error: unknown, debug = false) {
+  const err = ensureError(error);
+  console.error(chalk.red(err.message));
+
+  if (err.cause) {
+    const cause = ensureError(err.cause);
+    console.error(chalk.yellow(cause.message));
+    printTrace(err, debug);
+  }
+
+  printTrace(err, debug);
+}
+
+function printTrace(err: Error, debug = false) {
+  // 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 && debug) {
+    console.debug(chalk.gray(err.stack));
+  }
+}
+
+function ensureError(value: unknown): Error {
+  if (value instanceof Error) return value;
+
+  let stringified = '[Unable to stringify the thrown value]';
+  try {
+    stringified = JSON.stringify(value);
+  } catch {
+  }
+
+  const error = new Error(`An unexpected error was thrown: ${stringified}`);
+  return error;
+}

packages/aws-cdk/test/api/plugin/plugin-host.test.ts

@@ -90,5 +90,11 @@ test('plugin that registers an invalid Context Provider throws', () => {
     };
   }, { virtual: true });
 
-  expect(() => host.load(THE_PLUGIN)).toThrow(/does not look like a ContextProviderPlugin/);
+  try {
+    host.load(THE_PLUGIN);
+    expect(true).toBe(false); // should not happen
+  } catch(e) {
+    expect(e).toHaveProperty('cause');
+    expect(e.cause?.message).toMatch(/does not look like a ContextProviderPlugin/);
+  }
 });