feat(create-release-branch): show actionable errors

packages/create-release-branch/CHANGELOG.md

@@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Show colors and progress spinners in the CLI output ([#154](https://github.com/MetaMask/release-tools/pull/154))
 - Show the CLI output in one block, with a spinner for long steps and the output of a failing Yarn step ([#159](https://github.com/MetaMask/release-tools/pull/159))
+- Show errors without a stack trace (set `DEBUG=create-release-branch:*` to see it), with their causes, and list release spec problems by line and package ([#160](https://github.com/MetaMask/release-tools/pull/160))
 
 ### Fixed
 

packages/create-release-branch/src/cli.ts

@@ -1,7 +1,7 @@
-import util from 'util';
-
 import { main } from './cli/main.js';
 import { createCliOutput } from './core/cli-output.js';
+import { describeError } from './core/user-facing-error.js';
+import { debug } from './utils/misc-utils.js';
 
 /**
  * The entrypoint to this tool.
@@ -16,8 +16,13 @@ async function cli(): Promise<void> {
 }
 
 cli().catch((error) => {
-  createCliOutput({ stdout: process.stdout, stderr: process.stderr }).error(
-    `${util.inspect(error)}\n`,
-  );
+  const output = createCliOutput({
+    stdout: process.stdout,
+    stderr: process.stderr,
+  });
+  const { title, lines } = describeError(error, output.format, {
+    debug: debug.enabled,
+  });
+  output.failure(title, lines);
   process.exitCode = 1;
 });

packages/create-release-branch/src/core/cli-output.test.ts

@@ -1,5 +1,6 @@
 import {
   S_BAR,
+  S_BAR_END,
   S_ERROR,
   S_INFO,
   S_STEP_SUBMIT,
@@ -8,7 +9,7 @@ import {
   spinner,
   taskLog,
 } from '@clack/prompts';
-import { stripVTControlCharacters } from 'node:util';
+import { stripVTControlCharacters, styleText } from 'node:util';
 import { MockWritable } from 'stdio-mock';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
@@ -408,6 +409,75 @@ describe('cli-output', () => {
       expect(taskLog).not.toHaveBeenCalled();
     });
 
+    it('failure writes the title, the lines, and the cancel line to stderr', () => {
+      const stdout = new MockWritable();
+      const stderr = new MockWritable();
+      const output = createCliOutput({ stdout, stderr });
+
+      output.failure('Something failed', ['Line 1', '', 'Line 2']);
+
+      expect(textOf(stderr)).toBe(
+        [
+          logLine(S_ERROR, 'Something failed'),
+          `${S_BAR}\n${S_BAR}  Line 1\n${S_BAR}\n${S_BAR}  Line 2\n${S_BAR}\n`,
+          `${S_BAR_END}  The release did not finish.\n\n`,
+        ].join(''),
+      );
+      expect(stdout.data()).toStrictEqual([]);
+    });
+
+    it('failure writes a gutter line before the closing line when there are no lines', () => {
+      const stdout = new MockWritable();
+      const stderr = new MockWritable();
+      const output = createCliOutput({ stdout, stderr });
+
+      output.failure('Something failed', []);
+
+      expect(textOf(stderr)).toBe(
+        [
+          logLine(S_ERROR, 'Something failed'),
+          `${S_BAR}\n`,
+          `${S_BAR_END}  The release did not finish.\n\n`,
+        ].join(''),
+      );
+    });
+
+    it('failure keeps the gutter on every line of a multi-line entry', () => {
+      const stdout = new MockWritable();
+      const stderr = new MockWritable();
+      const output = createCliOutput({ stdout, stderr });
+
+      output.failure('Something failed', ['Line 1\nLine 2']);
+
+      expect(textOf(stderr)).toBe(
+        [
+          logLine(S_ERROR, 'Something failed'),
+          `${S_BAR}\n${S_BAR}  Line 1\n${S_BAR}  Line 2\n${S_BAR}\n`,
+          `${S_BAR_END}  The release did not finish.\n\n`,
+        ].join(''),
+      );
+    });
+
+    it('formats names and dim text for stderr only when stderr is styled', () => {
+      const styled = createCliOutput({
+        stdout: new MockWritable(),
+        stderr: buildTerminalStream(),
+      });
+      const plain = createCliOutput({
+        stdout: buildTerminalStream(),
+        stderr: new MockWritable(),
+      });
+
+      expect(styled.format.name('a')).toBe(
+        styleText('cyan', 'a', { validateStream: false }),
+      );
+      expect(styled.format.dim('a')).toBe(
+        styleText('dim', 'a', { validateStream: false }),
+      );
+      expect(plain.format.name('a')).toBe('a');
+      expect(plain.format.dim('a')).toBe('a');
+    });
+
     it.each([
       { keyName: 'Ctrl+C', input: '\u0003', key: { ctrl: true, name: 'c' } },
       {

packages/create-release-branch/src/core/cli-output.ts

@@ -1,4 +1,4 @@
-import { log, spinner, taskLog } from '@clack/prompts';
+import { cancel, log, spinner, taskLog } from '@clack/prompts';
 import { Writable } from 'node:stream';
 import { stripVTControlCharacters, styleText } from 'node:util';
 
@@ -13,6 +13,10 @@ import { stripVTControlCharacters, styleText } from 'node:util';
  * out.
  * @property warn - Writes a warning to standard error.
  * @property error - Writes an error to standard error.
+ * @property failure - Writes the error that stopped the run, with lines of
+ * details, and ends the output, to standard error.
+ * @property format - Styles parts of the text for standard error: `name` for
+ * names, such as package names, and `dim` for secondary text.
  * @property step - Runs a long-running action and returns its result.
  * @property task - Runs a long-running action that prints lines of output,
  * such as a command, and returns its result. The lines are shown only while
@@ -23,6 +27,8 @@ export type CliOutput = {
   success(text: string): void;
   warn(text: string): void;
   error(text: string): void;
+  failure(title: string, lines: string[]): void;
+  format: { name(text: string): string; dim(text: string): string };
   step<Result>(label: string, run: () => Promise<Result>): Promise<Result>;
   task<Result>(
     label: string,
@@ -185,6 +191,12 @@ export function createCliOutput({
 }): CliOutput {
   const toStdout = { output: styled.stdout ? stdout : withoutColors(stdout) };
   const toStderr = { output: styled.stderr ? stderr : withoutColors(stderr) };
+  const format = {
+    name: (text: string): string =>
+      styled.stderr ? styleText('cyan', text, { stream: stderr }) : text,
+    dim: (text: string): string =>
+      styled.stderr ? styleText('dim', text, { stream: stderr }) : text,
+  };
 
   return {
     info(text) {
@@ -199,6 +211,15 @@ export function createCliOutput({
     error(text) {
       log.error(trimTrailingNewlines(text), toStderr);
     },
+    failure(title, lines) {
+      const details = lines.flatMap((line) => line.split('\n'));
+      log.error(title, toStderr);
+      // clack writes one gutter line before the details, so an empty entry
+      // after them adds the same gutter line before the closing line.
+      log.message(details.length > 0 ? [...details, ''] : [], toStderr);
+      cancel('The release did not finish.', toStderr);
+    },
+    format,
     async step(label, run) {
       if (!animate) {
         try {
@@ -236,10 +257,7 @@ export function createCliOutput({
           return result;
         } catch (error) {
           log.error(label, toStderr);
-          log.message(
-            lines.map((line) => styleText('dim', line, { stream: stderr })),
-            toStderr,
-          );
+          log.message(lines.map(format.dim), toStderr);
           throw error;
         }
       }

packages/create-release-branch/src/core/release-rules.test.ts

@@ -10,6 +10,7 @@ import {
   IncrementableVersionParts,
   normalizeReleaseChoices,
 } from './release-rules.js';
+import type { ReleaseChoices } from './release-rules.js';
 
 describe('release-rules', () => {
   describe('checkReleaseChoices', () => {
@@ -52,18 +53,10 @@ describe('release-rules', () => {
       expect(checkReleaseChoices(project, { a: 'major' })).toStrictEqual({
         errors: [
           {
-            message: `
-The following direct dependents of package 'a', which is being released with a major version bump, are missing from the release spec.
-
-  - b
-
-  Consider including them in the release spec so that the dependency tree of consuming projects can be kept small.
-
-  If you do not want to do this, then list it with a directive of "intentionally-skip". For example:
-
-    packages:
-      b: intentionally-skip
-`.trim(),
+            packageName: 'a',
+            problem:
+              'has a major bump, but its direct dependents are not in the release spec: b',
+            hint: 'Add them to the release spec, or set them to intentionally-skip.',
           },
         ],
         dependencyFindings: {
@@ -125,18 +118,163 @@ The following direct dependents of package 'a', which is being released with a m
         checkReleaseChoices(project, { a: 'asdf', b: 'minor' }).errors,
       ).toStrictEqual([
         {
-          message: [
-            '"asdf" is not a valid version specifier for package "a"',
-            '(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")',
-          ],
+          packageName: 'a',
           entryIndex: 0,
+          problem: '"asdf" is not a valid version specifier',
+          hint: 'Use major, minor, patch, or a version like 1.2.3.',
         },
         {
-          message: '"b" is not a package in the project',
+          packageName: 'b',
           entryIndex: 1,
+          problem: 'is not a package in the project',
+          hint: 'Check the name, or remove the line.',
         },
       ]);
     });
+
+    it('reports an unknown package with an exact version only as unknown', () => {
+      const project = buildMockProject({
+        workspacePackages: {
+          a: buildMockPackage('a', '1.0.0'),
+        },
+      });
+
+      expect(checkReleaseChoices(project, { b: '1.2.3' }).errors).toStrictEqual(
+        [
+          {
+            packageName: 'b',
+            entryIndex: 0,
+            problem: 'is not a package in the project',
+            hint: 'Check the name, or remove the line.',
+          },
+        ],
+      );
+    });
+
+    it.each([
+      {
+        rule: 'an unknown package',
+        workspacePackages: { a: buildMockPackage('a', '1.0.0') },
+        choices: { b: 'minor' },
+        error: {
+          packageName: 'b',
+          entryIndex: 0,
+          problem: 'is not a package in the project',
+          hint: 'Check the name, or remove the line.',
+        },
+      },
+      {
+        rule: 'an invalid version specifier',
+        workspacePackages: { a: buildMockPackage('a', '1.0.0') },
+        choices: { a: 'asdf' },
+        error: {
+          packageName: 'a',
+          entryIndex: 0,
+          problem: '"asdf" is not a valid version specifier',
+          hint: 'Use major, minor, patch, or a version like 1.2.3.',
+        },
+      },
+      {
+        rule: 'a version specifier that is not a string',
+        workspacePackages: { a: buildMockPackage('a', '1.0.0') },
+        choices: { a: { version: '2.0.0' } } as unknown as ReleaseChoices,
+        error: {
+          packageName: 'a',
+          entryIndex: 0,
+          problem: '"{"version":"2.0.0"}" is not a valid version specifier',
+          hint: 'Use major, minor, patch, or a version like 1.2.3.',
+        },
+      },
+      {
+        rule: 'the current version',
+        workspacePackages: { a: buildMockPackage('a', '1.0.0') },
+        choices: { a: '1.0.0' },
+        error: {
+          packageName: 'a',
+          entryIndex: 0,
+          problem: 'is already at version 1.0.0',
+          hint: 'Use a higher version, or major, minor, or patch.',
+        },
+      },
+      {
+        rule: 'a lower version',
+        workspacePackages: { a: buildMockPackage('a', '1.0.0') },
+        choices: { a: '0.9.0' },
+        error: {
+          packageName: 'a',
+          entryIndex: 0,
+          problem: '"0.9.0" is lower than the current version 1.0.0',
+          hint: 'Use a higher version, or major, minor, or patch.',
+        },
+      },
+      {
+        rule: 'missing direct dependents',
+        workspacePackages: {
+          a: buildMockPackage('a', '1.0.0'),
+          b: buildMockPackage('b', {
+            validatedManifest: { dependencies: { a: '1.0.0' } },
+          }),
+          c: buildMockPackage('c', {
+            validatedManifest: { dependencies: { a: '1.0.0' } },
+          }),
+        },
+        choices: { a: 'major' },
+        error: {
+          packageName: 'a',
+          problem:
+            'has a major bump, but its direct dependents are not in the release spec: b, c',
+          hint: 'Add them to the release spec, or set them to intentionally-skip.',
+        },
+      },
+      {
+        rule: 'missing peer dependents',
+        workspacePackages: {
+          a: buildMockPackage('a', '1.0.0'),
+          b: buildMockPackage('b', {
+            validatedManifest: { peerDependencies: { a: '1.0.0' } },
+          }),
+          c: buildMockPackage('c', {
+            validatedManifest: { peerDependencies: { a: '1.0.0' } },
+          }),
+        },
+        choices: { a: 'major' },
+        error: {
+          packageName: 'a',
+          problem:
+            'has a major bump, but its peer dependents are not in the release spec: b, c',
+          hint: 'Add them so they stay compatible, or set them to intentionally-skip if they can wait.',
+        },
+      },
+      {
+        rule: 'missing changed dependencies',
+        workspacePackages: {
+          a: buildMockPackage('a', {
+            validatedManifest: {
+              dependencies: { b: '1.0.0' },
+              peerDependencies: { c: '1.0.0' },
+            },
+          }),
+          b: buildMockPackage('b', { hasChangesSinceLatestRelease: true }),
+          c: buildMockPackage('c', { hasChangesSinceLatestRelease: true }),
+        },
+        choices: { a: 'minor' },
+        error: {
+          packageName: 'a',
+          problem:
+            'depends on packages with unreleased changes that are not in the release spec: b, c',
+          hint: 'Add them to the release spec, or set them to intentionally-skip if they can wait.',
+        },
+      },
+    ])(
+      'reports $rule with its problem and hint',
+      ({ workspacePackages, choices, error }) => {
+        const project = buildMockProject({ workspacePackages });
+
+        expect(checkReleaseChoices(project, choices).errors).toStrictEqual([
+          error,
+        ]);
+      },
+    );
   });
 
   describe('normalizeReleaseChoices', () => {