fix: construct errors are rendered in a messy way

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

@@ -1,5 +1,6 @@
 import type { IConstruct } from 'constructs';
 import { constructInfoFromConstruct } from './private/runtime-info';
+import { captureCallStack, renderCallStackJustMyCode } from './stack-trace';
 import type { AssertionError } from '../../assertions/lib/private/error';
 import type { CloudAssemblyError } from '../../cx-api/lib/private/error';
 
@@ -120,61 +121,31 @@ abstract class ConstructError extends Error {
 
   constructor(msg: string, scope?: IConstruct, name?: string) {
     super(msg);
+
+    const ctr = this.constructor;
+
     Object.setPrototypeOf(this, ConstructError.prototype);
     Object.defineProperty(this, CONSTRUCT_ERROR_SYMBOL, { value: true });
 
-    this.name = name ?? new.target.name;
+    this.name = name ?? ctr.name;
     this.#time = new Date().toISOString();
     this.#constructPath = scope?.node.path;
 
     if (scope) {
-      Error.captureStackTrace(this, scope.constructor);
       try {
-        this.#constructInfo = scope ? constructInfoFromConstruct(scope) : undefined;
+        this.#constructInfo = constructInfoFromConstruct(scope);
       } catch (_) {
         // we don't want to fail if construct info is not available
       }
     }
 
-    const stack = [
-      `${this.name}: ${this.message}`,
-    ];
-
-    if (this.constructInfo) {
-      stack.push(`in ${this.constructInfo?.fqn} at [${this.constructPath}]`);
-    } else {
-      stack.push(`in [${this.constructPath}]`);
-    }
+    // The "stack" field in Node.js includes the error description. If it doesn't, Node will fall back to an
+    // ugly way of rendering the error.
+    this.stack = `${this.name}: ${msg}\n${renderCallStackJustMyCode(captureCallStack(ctr)).join('\n')}`;
 
-    if (this.stack) {
-      stack.push(this.stack);
-    }
-
-    this.stack = this.constructStack(this.stack);
-  }
-
-  /**
-   * Helper message to clean-up the stack and amend with construct information.
-   */
-  private constructStack(prev?: string) {
-    const indent = ' '.repeat(4);
-
-    const stack = [
-      `${this.name}: ${this.message}`,
-    ];
-
-    if (this.constructInfo) {
-      stack.push(`${indent}at path [${this.constructPath}] in ${this.constructInfo?.fqn}`);
-    } else {
-      stack.push(`${indent}at path [${this.constructPath}]`);
-    }
-
-    if (prev) {
-      stack.push('');
-      stack.push(...prev.split('\n').slice(1));
+    if (scope) {
+      this.stack += `\nRelates to construct:\n${renderConstructRootPath(scope)}`;
     }
-
-    return stack.join('\n');
   }
 }
 
@@ -258,3 +229,34 @@ export class ExecutionError extends ConstructError {
     Object.defineProperty(this, EXECUTION_ERROR_SYMBOL, { value: true });
   }
 }
+
+export function renderConstructRootPath(construct: IConstruct) {
+  const rootPath = [];
+
+  let cur: IConstruct | undefined = construct;
+  while (cur !== undefined) {
+    rootPath.push(cur);
+    cur = cur.node.scope;
+  }
+  rootPath.reverse();
+
+  const ret = new Array<string>();
+  for (let i = 0; i < rootPath.length; i++) {
+    const constructId = rootPath[i].node.id || '<.>';
+
+    let suffix = '';
+    try {
+      const constructInfo = constructInfoFromConstruct(rootPath[i]);
+      suffix = ` (${constructInfo?.fqn})`;
+    } catch (_) {
+      // we don't want to fail if construct info is not available
+    }
+
+    const branch = ' └─ ';
+    const indent = i > 0 ? ' '.repeat(branch.length * (i - 1)) + branch : '';
+
+    ret.push(`    ${indent}${constructId}${suffix}`);
+  }
+
+  return ret.join('\n');
+}

packages/aws-cdk-lib/core/lib/stack-trace.ts

@@ -27,16 +27,97 @@ export function captureStackTrace(
     return ['stack traces disabled'];
   }
 
-  const object: { stack?: string } = {};
   const previousLimit = Error.stackTraceLimit;
   try {
     Error.stackTraceLimit = limit;
-    Error.captureStackTrace(object, below);
+    return renderCallStackJustMyCode(captureCallStack(below), false);
   } finally {
     Error.stackTraceLimit = previousLimit;
   }
-  if (!object.stack) {
-    return [];
+}
+
+/**
+ * Capture a call stack using `Error.captureStackTrace`
+ *
+ * Modern Nodes have a `util.getCallSites()` API but it's heavily unstable and
+ * doesn't have all the information of the legacy API yet.
+ */
+export function captureCallStack(upTo: Function | undefined): NodeJS.CallSite[] {
+  Error.prepareStackTrace = (_, trace) => trace;
+  try {
+    const obj: { stack: NodeJS.CallSite[] } = {} as any;
+    Error.captureStackTrace(obj, upTo);
+    if (obj.stack.length === 0) {
+      // Protect against a common mistake: if upTo is not in the call stack, `captureStackTrace` will return an empty array.
+      // If that happens, do it again without an `upTo` function.
+      Error.captureStackTrace(obj);
+    }
+    return obj.stack;
+  } finally {
+    Error.prepareStackTrace = undefined as any;
+  }
+}
+
+/**
+ * Renders an array of CallSites nicely, focusing on the user application code
+ *
+ * We detect "Not My Code" using the following heuristics:
+ *
+ * - If there is '/node_modules/' in the file path, we assume the call stack is a library and we skip it.
+ * - If there is 'node:' in the file path, we assume it is NodeJS internals and we skip it.
+ */
+export function renderCallStackJustMyCode(stack: NodeJS.CallSite[], indent = true): string[] {
+  const moduleRe = /(\/|\\)node_modules(\/|\\)([^/\\]+)/;
+
+  const lines = [];
+  let skipped = new Array<string>();
+
+  let i = 0;
+  while (i < stack.length) {
+    const frame = stack[i++];
+
+    // FIXME: Show the last function we called into when going into library code
+
+    const pat = fileName(frame).match(moduleRe);
+    if (pat) {
+      while (i < stack.length && fileName(stack[i]).includes(pat[0])) {
+        i++;
+      }
+      // The last stack frame has the function that user code call into.
+      skip(`${renderFunctionCall(frame)} in ${pat[3]}`);
+    } else if (fileName(frame).includes('node:')) {
+      skip('node internals');
+      while (i < stack.length && fileName(stack[i]).includes('node:')) {
+        i++;
+      }
+    } else {
+      reportSkipped();
+      const prefix = indent ? '    at ' : '';
+      lines.push(`${prefix}${renderFunctionCall(frame)} (${fileName(frame)}:${frame.getLineNumber()})`);
+    }
+  }
+  reportSkipped();
+  return lines;
+
+  function renderFunctionCall(frame: NodeJS.CallSite): string {
+    return `${frame.isConstructor() ? 'new ' : ''}${frame.getFunctionName() || '<anonymous>'}`;
+  }
+
+  function fileName(frame: NodeJS.CallSite): string {
+    return frame.getScriptNameOrSourceURL() ?? '?';
+  }
+
+  function skip(what: string) {
+    if (!skipped.includes(what)) {
+      skipped.push(what);
+    }
+  }
+
+  function reportSkipped() {
+    if (skipped.length > 0) {
+      const prefix = indent ? '    ' : '';
+      lines.push(`${prefix}...${skipped.join(', ')}...`);
+    }
+    skipped = [];
   }
-  return object.stack.split('\n').slice(1).map(s => s.replace(/^\s*at\s+/, ''));
 }

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

@@ -1,3 +1,5 @@
+import { inspect } from 'util';
+import { Bucket } from '../../aws-s3';
 import { App, Stack } from '../lib';
 import { Errors, UnscopedValidationError, ValidationError } from '../lib/errors';
 
@@ -40,4 +42,57 @@ describe('ValidationError', () => {
     expect(error.name).toBe('ValidationError');
     expect(error.stack).toContain('ValidationError: this is an error');
   });
+
+  test('presentation of a ValidationError', () => {
+    try {
+      const app = new App();
+      const stack = new Stack(app, 'SomeStack');
+
+      const targetBucket = new Bucket(stack, 'TargetBucket');
+
+      throw new ValidationError('ErrorCode', 'There is an error here', targetBucket);
+    } catch (e: any) {
+      // NodeJS will render an uncaught error using inspect()
+      const errorString = inspect(e);
+      expect(anonymizeBetweenParens(errorString)).toMatchInlineSnapshot(`
+"ErrorCode: There is an error here
+    at <anonymous> (...)
+    ...Promise.then.completed in jest-circus...
+    at new Promise (...)
+    ...callAsyncCircusFn in jest-circus, node internals, _runTest in jest-circus, runTestInternal in jest-runner...
+Relates to construct:
+    <.> (...)
+     └─ SomeStack (...)
+         └─ TargetBucket (...)"
+`);
+    }
+  });
+
+  test('presentation of an UnscopedValidationError', () => {
+    try {
+      throw new UnscopedValidationError('ErrorCode', 'There is an error here');
+    } catch (e: any) {
+      // NodeJS will render an uncaught error using inspect()
+      const errorString = inspect(e);
+      expect(anonymizeBetweenParens(errorString)).toMatchInlineSnapshot(`
+"ErrorCode: There is an error here
+    at <anonymous> (...)
+    ...Promise.then.completed in jest-circus...
+    at new Promise (...)
+    ...callAsyncCircusFn in jest-circus, node internals, _runTest in jest-circus, runTestInternal in jest-runner..."
+`);
+    }
+  });
 });
+
+/**
+ * Anonymize info between parentheses.
+ *
+ * - Construct IDs are only injected by jsii, so a js-test test won't have these.
+ * - File paths are only valid on 1 particular disk.
+ */
+function anonymizeBetweenParens(x: string): string {
+  return x.split('\n')
+    .map(s => s.replace(/\([^)]*\)/, '(...)'))
+    .join('\n');
+}