diff --git a/packages/create-release-branch/CHANGELOG.md b/packages/create-release-branch/CHANGELOG.md index ad4e2d3..8e00660 100644 --- a/packages/create-release-branch/CHANGELOG.md +++ b/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 diff --git a/packages/create-release-branch/src/cli.ts b/packages/create-release-branch/src/cli.ts index 41dd9b9..e1dfdb3 100644 --- a/packages/create-release-branch/src/cli.ts +++ b/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 { } 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; }); diff --git a/packages/create-release-branch/src/core/cli-output.test.ts b/packages/create-release-branch/src/core/cli-output.test.ts index fcc1af9..c869eef 100644 --- a/packages/create-release-branch/src/core/cli-output.test.ts +++ b/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' } }, { diff --git a/packages/create-release-branch/src/core/cli-output.ts b/packages/create-release-branch/src/core/cli-output.ts index fb2aeaa..8456a4f 100644 --- a/packages/create-release-branch/src/core/cli-output.ts +++ b/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(label: string, run: () => Promise): Promise; task( 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; } } diff --git a/packages/create-release-branch/src/core/release-rules.test.ts b/packages/create-release-branch/src/core/release-rules.test.ts index afe3090..3bf8bc2 100644 --- a/packages/create-release-branch/src/core/release-rules.test.ts +++ b/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', () => { diff --git a/packages/create-release-branch/src/core/release-rules.ts b/packages/create-release-branch/src/core/release-rules.ts index 03f61c4..539439a 100644 --- a/packages/create-release-branch/src/core/release-rules.ts +++ b/packages/create-release-branch/src/core/release-rules.ts @@ -1,5 +1,4 @@ import { diff } from 'semver'; -import YAML from 'yaml'; import type { Package } from '../project/package.js'; import type { Project } from '../project/project.js'; @@ -48,13 +47,17 @@ export type DependencyFinding = { /** * A problem with the release choices. * - * @property message - The message, as a single string or as separate lines. + * @property packageName - The name of the package that the problem is about. * @property entryIndex - The index of the choice in `Object.entries(choices)`, * for problems that point at a single entry. + * @property problem - What is wrong, written to follow the package name. + * @property hint - How to fix it. */ export type ReleaseChoiceError = { - message: string | string[]; + packageName?: string; entryIndex?: number; + problem: string; + hint?: string; }; /** @@ -208,29 +211,6 @@ function findCandidateDependencies( return []; } -/** - * Builds the YAML example, indented for an error message, that lists the - * given packages with the "intentionally-skip" directive. - * - * @param packageNames - The names of the packages to list. - * @returns The indented YAML example. - */ -function buildIntentionallySkipExample(packageNames: string[]): string { - return YAML.stringify({ - packages: packageNames.reduce( - (object, packageName) => ({ - ...object, - [packageName]: INTENTIONALLY_SKIP_PACKAGE_DIRECTIVE, - }), - {}, - ), - }) - .trim() - .split('\n') - .map((line) => ` ${line}`) - .join('\n'); -} - /** * Checks the release choices against the project: the names of the packages, * the version specifiers, and the dependencies and dependents that must be @@ -270,9 +250,12 @@ export function checkReleaseChoices( if (changedPackage === undefined) { errors.push({ - message: `${JSON.stringify(changedPackageName)} is not a package in the project`, + packageName: changedPackageName, entryIndex, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', }); + return; } if ( @@ -282,37 +265,36 @@ export function checkReleaseChoices( !isValidSemver(versionSpecifierOrDirective) ) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3")`, - ], + packageName: changedPackageName, entryIndex, + problem: `"${ + typeof versionSpecifierOrDirective === 'string' + ? versionSpecifierOrDirective + : JSON.stringify(versionSpecifierOrDirective) + }" is not a valid version specifier`, + hint: 'Use major, minor, patch, or a version like 1.2.3.', }); } if (isValidSemver(versionSpecifierOrDirective)) { + const currentVersion = changedPackage.validatedManifest.version; const comparison = new SemVer(versionSpecifierOrDirective).compare( - changedPackage.validatedManifest.version, + currentVersion, ); if (comparison === 0) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `("${changedPackageName}" is already at version "${versionSpecifierOrDirective}")`, - ], + packageName: changedPackageName, entryIndex, + problem: `is already at version ${versionSpecifierOrDirective}`, + hint: 'Use a higher version, or major, minor, or patch.', }); } else if (comparison < 0) { errors.push({ - message: [ - `${JSON.stringify(versionSpecifierOrDirective)} is not a valid version specifier for package "${changedPackageName}"`, - `("${changedPackageName}" is at a greater version "${String( - project.workspacePackages[changedPackageName].validatedManifest - .version, - )}")`, - ], + packageName: changedPackageName, entryIndex, + problem: `"${versionSpecifierOrDirective}" is lower than the current version ${String(currentVersion)}`, + hint: 'Use a higher version, or major, minor, or patch.', }); } } @@ -328,15 +310,9 @@ export function checkReleaseChoices( if (missingDirectDependentNames.length > 0) { errors.push({ - message: [ - `The following direct dependents of package '${changedPackageName}', which is being released with a major version bump, are missing from the release spec.`, - missingDirectDependentNames - .map((dependent) => ` - ${dependent}`) - .join('\n'), - ` 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:`, - buildIntentionallySkipExample(missingDirectDependentNames), - ].join('\n\n'), + packageName: changedPackageName, + problem: `has a major bump, but its direct dependents are not in the release spec: ${missingDirectDependentNames.join(', ')}`, + hint: 'Add them to the release spec, or set them to intentionally-skip.', }); } @@ -351,15 +327,9 @@ export function checkReleaseChoices( if (missingPeerDependentNames.length > 0) { errors.push({ - message: [ - `The following dependents of package '${changedPackageName}', which is being released with a major version bump, are missing from the release spec.`, - missingPeerDependentNames - .map((dependent) => ` - ${dependent}`) - .join('\n'), - ` Consider including them in the release spec so that they are compatible with the new '${changedPackageName}' version.`, - ` If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`, - buildIntentionallySkipExample(missingPeerDependentNames), - ].join('\n\n'), + packageName: changedPackageName, + problem: `has a major bump, but its peer dependents are not in the release spec: ${missingPeerDependentNames.join(', ')}`, + hint: 'Add them so they stay compatible, or set them to intentionally-skip if they can wait.', }); } @@ -372,15 +342,9 @@ export function checkReleaseChoices( if (missingDependencies.length > 0) { errors.push({ - message: [ - `The following packages, which are dependencies or peer dependencies of the package '${changedPackageName}' being released, are missing from the release spec.`, - missingDependencies - .map((dependency) => ` - ${dependency}`) - .join('\n'), - ` These packages may have changes that '${changedPackageName}' relies upon. Consider including them in the release spec.`, - ` If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example:`, - buildIntentionallySkipExample(missingDependencies), - ].join('\n\n'), + packageName: changedPackageName, + problem: `depends on packages with unreleased changes that are not in the release spec: ${missingDependencies.join(', ')}`, + hint: 'Add them to the release spec, or set them to intentionally-skip if they can wait.', }); } diff --git a/packages/create-release-branch/src/core/release-steps.test.ts b/packages/create-release-branch/src/core/release-steps.test.ts index a088604..d9964e0 100644 --- a/packages/create-release-branch/src/core/release-steps.test.ts +++ b/packages/create-release-branch/src/core/release-steps.test.ts @@ -185,11 +185,10 @@ describe('release-steps', () => { expect(result).toStrictEqual({ errors: [ { - message: [ - '"foo" 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: '"foo" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', }, ], dependencyFindings: {}, @@ -202,7 +201,7 @@ describe('release-steps', () => { const project = buildMockProject({ workspacePackages: { a: buildMockPackage('a', '1.0.0') }, }); - const planErrors = [{ message: 'Nothing to release' }]; + const planErrors = [{ problem: 'Nothing to release' }]; const plan = vi.fn().mockResolvedValue({ errors: planErrors }); const mode = buildMockMode({ plan }); diff --git a/packages/create-release-branch/src/core/user-facing-error.test.ts b/packages/create-release-branch/src/core/user-facing-error.test.ts new file mode 100644 index 0000000..cb0e185 --- /dev/null +++ b/packages/create-release-branch/src/core/user-facing-error.test.ts @@ -0,0 +1,83 @@ +import { describe, expect, it } from 'vitest'; + +import type { CliOutput } from './cli-output.js'; +import { describeError, UserFacingError } from './user-facing-error.js'; + +const format: CliOutput['format'] = { + name: (text) => `<${text}>`, + dim: (text) => `(${text})`, +}; + +class TestError extends UserFacingError { + describe( + errorFormat: CliOutput['format'], + ): ReturnType { + return { title: 'Test title', lines: [errorFormat.name('a')] }; + } +} + +describe('user-facing-error', () => { + describe('describeError', () => { + it('uses the description of a user-facing error', () => { + const error = new TestError('Some message', { + cause: new Error('Some cause'), + }); + + expect(describeError(error, format, { debug: true })).toStrictEqual({ + title: 'Test title', + lines: [''], + }); + }); + + it('prints causes and hides the stack trace', () => { + const error = new Error('Could not read file\nmore details', { + cause: new Error('Permission denied', { cause: 'EACCES' }), + }); + + expect(describeError(error, format, { debug: false })).toStrictEqual({ + title: 'Could not read file', + lines: [ + 'more details', + 'Caused by: Permission denied', + 'Caused by: EACCES', + ], + }); + }); + + it('prints the stack trace when debugging is on', () => { + const error = new Error('Something failed', { + cause: new Error('Some cause'), + }); + error.stack = 'Error: Something failed\n at line 1\n at line 2'; + + expect(describeError(error, format, { debug: true })).toStrictEqual({ + title: 'Something failed', + lines: [ + 'Caused by: Some cause', + '', + 'Error: Something failed', + ' at line 1', + ' at line 2', + ], + }); + }); + + it('stops at a cause that is already in the chain', () => { + const error = new Error('Something failed'); + const cause = new Error('Some cause', { cause: error }); + error.cause = cause; + + expect(describeError(error, format, { debug: false })).toStrictEqual({ + title: 'Something failed', + lines: ['Caused by: Some cause'], + }); + }); + + it('describes a value that is not an error', () => { + expect(describeError('oops', format, { debug: true })).toStrictEqual({ + title: 'oops', + lines: [], + }); + }); + }); +}); diff --git a/packages/create-release-branch/src/core/user-facing-error.ts b/packages/create-release-branch/src/core/user-facing-error.ts new file mode 100644 index 0000000..4892260 --- /dev/null +++ b/packages/create-release-branch/src/core/user-facing-error.ts @@ -0,0 +1,65 @@ +import type { CliOutput } from './cli-output.js'; + +/** + * An error that describes itself to the user, with a title and lines of + * details, instead of being printed with its message, causes, and stack. + */ +export abstract class UserFacingError extends Error { + /** + * Describes the error to the user. + * + * @param format - Styles parts of the text. + * @returns The title and the lines of details. + */ + abstract describe(format: CliOutput['format']): { + title: string; + lines: string[]; + }; +} + +/** + * Gets the message of an error, or the text of a value that is not an error. + * + * @param error - The error. + * @returns The message. + */ +function messageOf(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} + +/** + * Describes an error to the user. A `UserFacingError` describes itself. Any + * other error is described by its message, then one `Caused by` line for each + * error in its chain of causes, then its stack trace when debugging. + * + * @param error - The error. + * @param format - Styles parts of the text. + * @param options - The options. + * @param options.debug - Whether to include the stack trace. + * @returns The title and the lines of details. + */ +export function describeError( + error: unknown, + format: CliOutput['format'], + { debug }: { debug: boolean }, +): { title: string; lines: string[] } { + if (error instanceof UserFacingError) { + return error.describe(format); + } + + const [title, ...lines] = messageOf(error).split('\n'); + + const seen = new Set([error]); + let cause = error instanceof Error ? error.cause : undefined; + while (cause !== undefined && !seen.has(cause)) { + seen.add(cause); + lines.push(`Caused by: ${messageOf(cause)}`); + cause = cause instanceof Error ? cause.cause : undefined; + } + + if (debug && error instanceof Error && error.stack !== undefined) { + lines.push('', ...error.stack.split('\n')); + } + + return { title, lines }; +} diff --git a/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts b/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts index 16ec854..bd69df9 100644 --- a/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts +++ b/packages/create-release-branch/src/frontends/editor/editor-front-end.test.ts @@ -1093,8 +1093,10 @@ describe('editor-front-end', () => { { errors: [ { - message: '"foo" is not a package in the project', + packageName: 'foo', entryIndex: 0, + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', }, ], dependencyFindings: {}, diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts index 059597a..c3ee874 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.test.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.test.ts @@ -18,6 +18,7 @@ import type { Project } from '../../project/project.js'; import * as miscUtils from '../../utils/misc-utils.js'; import { generateReleaseSpecificationTemplateForMonorepo, + ReleaseSpecificationError, waitForUserToEditReleaseSpecification, validateReleaseSpecification as validateReleaseSpecificationWithCheck, } from './release-specification.js'; @@ -404,9 +405,13 @@ packages: validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( expect.objectContaining({ - message: expect.stringMatching( - /^Your release spec does not appear to be valid YAML\.\n/u, - ) as string, + problems: [ + { + problem: 'is not valid YAML', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10', + }, + ], + specPath: releaseSpecificationPath, cause: expect.objectContaining({ message: expect.stringMatching( /^Missing closing "quote/u, @@ -432,7 +437,14 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - /^Your release spec could not be processed because it needs to be an object/u, + expect.objectContaining({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + }), ); }); }); @@ -452,7 +464,14 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - /^Your release spec could not be processed because it needs to be an object/u, + expect.objectContaining({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + }), ); }); }); @@ -481,14 +500,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - new RegExp( - [ - '^Your release spec could not be processed due to the following issues:\n', - '\\* Line 2: "foo" is not a package in the project', - '\\* Line 3: "bar" is not a package in the project', - ].join('\n'), - 'u', - ), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'foo', + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + { + lineNumber: 3, + packageName: 'bar', + problem: 'is not a package in the project', + hint: 'Check the name, or remove the line.', + }, + ], + }), ); }); }); @@ -518,16 +545,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - new RegExp( - [ - '^Your release spec could not be processed due to the following issues:\n', - '\\* Line 2: "asdflksdaf" 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"\\)', - '\\* Line 3: "1.2\\.\\.\\.3\\." is not a valid version specifier for package "b"', - ' \\(must be "major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1\\.2\\.3"\\)', - ].join('\n'), - 'u', - ), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: '"asdflksdaf" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: '"1.2...3." is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + ], + }), ); }); }); @@ -557,14 +590,22 @@ packages: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - ` -Your release spec could not be processed due to the following issues: - -* Line 2: "1.2.3" is not a valid version specifier for package "a" - ("a" is already at version "1.2.3") -* Line 3: "4.5.6" is not a valid version specifier for package "b" - ("b" is already at version "4.5.6") -`.trim(), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: 'is already at version 1.2.3', + hint: 'Use a higher version, or major, minor, or patch.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: 'is already at version 4.5.6', + hint: 'Use a higher version, or major, minor, or patch.', + }, + ], + }), ); }); }); @@ -594,14 +635,22 @@ Your release spec could not be processed due to the following issues: await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - ` -Your release spec could not be processed due to the following issues: - -* Line 2: "1.2.2" is not a valid version specifier for package "a" - ("a" is at a greater version "1.2.3") -* Line 3: "4.5.5" is not a valid version specifier for package "b" - ("b" is at a greater version "4.5.6") -`.trim(), + expect.objectContaining({ + problems: [ + { + lineNumber: 2, + packageName: 'a', + problem: '"1.2.2" is lower than the current version 1.2.3', + hint: 'Use a higher version, or major, minor, or patch.', + }, + { + lineNumber: 3, + packageName: 'b', + problem: '"4.5.5" is lower than the current version 4.5.6', + hint: 'Use a higher version, or major, minor, or patch.', + }, + ], + }), ); }); }); @@ -640,21 +689,13 @@ Your release spec could not be processed due to the following issues: validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following 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 they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -695,21 +736,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following 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 they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -750,21 +783,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following 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 they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -805,21 +830,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following 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 they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -861,21 +878,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following 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 they are compatible with the new 'a' version. +The release spec has 1 problem: - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -917,21 +926,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -973,21 +974,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. - - - b +The release spec has 1 problem: - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1029,21 +1022,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following dependents of package 'a', which is being released with a major version bump, are missing from the release spec. +The release spec has 1 problem: - - b - - Consider including them in the release spec so that they are compatible with the new 'a' version. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its peer dependents are not in the release spec: b + Add them so they stay compatible, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1084,21 +1069,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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 +The release spec has 1 problem: -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1139,21 +1116,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: +The release spec has 1 problem: -* 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 - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1194,21 +1163,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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 +The release spec has 1 problem: -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1249,21 +1210,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: +The release spec has 1 problem: -* 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 - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1305,21 +1258,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1361,21 +1306,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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: +The release spec has 1 problem: - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1417,21 +1354,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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. +The release spec has 1 problem: - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1473,21 +1402,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* 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. +The release spec has 1 problem: - If you do not want to do this, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + has a major bump, but its direct dependents are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1708,21 +1629,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b +The release spec has 1 problem: - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1763,21 +1676,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b +The release spec has 1 problem: - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1819,21 +1724,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b +The release spec has 1 problem: - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -1875,21 +1772,13 @@ ${releaseSpecificationPath} validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( ` -Your release spec could not be processed due to the following issues: - -* The following packages, which are dependencies or peer dependencies of the package 'a' being released, are missing from the release spec. - - - b +The release spec has 1 problem: - These packages may have changes that 'a' relies upon. Consider including them in the release spec. - - If you are ABSOLUTELY SURE these packages are safe to omit, however, and want to postpone the release of a package, then list it with a directive of "intentionally-skip". For example: - - packages: - b: intentionally-skip - -The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool. +a + depends on packages with unreleased changes that are not in the release spec: b + Add them to the release spec, or set them to intentionally-skip if they can wait. +Fix the release spec and run the tool again. It was kept at: ${releaseSpecificationPath} `.trim(), ); @@ -2076,7 +1965,7 @@ ${releaseSpecificationPath} }); }); - it("keeps today's line numbers for a generated template", async () => { + it("keeps today's line numbers in the new layout", async () => { await withSandbox(async (sandbox) => { const project = buildMockProject({ workspacePackages: { @@ -2100,9 +1989,92 @@ ${releaseSpecificationPath} await expect( validateReleaseSpecification(project, releaseSpecificationPath), ).rejects.toThrow( - '* Line 29: "foo" is not a valid version specifier for package "a"', + [ + 'Line 29 a', + ' "foo" is not a valid version specifier', + ' Use major, minor, patch, or a version like 1.2.3.', + ].join('\n'), ); }); }); }); + + describe('ReleaseSpecificationError', () => { + const format = { + name: (text: string): string => `<${text}>`, + dim: (text: string): string => `(${text})`, + }; + + it('describes each problem with its line, package, problem, and hint', () => { + const error = new ReleaseSpecificationError({ + problems: [ + { + lineNumber: 7, + packageName: 'a', + problem: '"foo" is not a valid version specifier', + hint: 'Use major, minor, patch, or a version like 1.2.3.', + }, + { + packageName: 'b', + problem: 'has a major bump', + hint: 'Add them.', + }, + { + problem: 'is not valid YAML', + hint: 'Fix the syntax: Missing closing "quote at line 1, column 10', + }, + ], + specPath: '/path/to/RELEASE_SPEC.yml', + }); + + expect(error.describe(format)).toStrictEqual({ + title: 'The release spec has 3 problems:', + lines: [ + 'Line 7 ', + ' "foo" is not a valid version specifier', + ' (Use major, minor, patch, or a version like 1.2.3.)', + '', + '', + ' has a major bump', + ' (Add them.)', + '', + 'The release spec', + ' is not valid YAML', + ' (Fix the syntax: Missing closing "quote at line 1, column 10)', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '(/path/to/RELEASE_SPEC.yml)', + ], + }); + }); + + it('uses the singular for one problem', () => { + const error = new ReleaseSpecificationError({ + problems: [{ packageName: 'a', problem: 'is not a package' }], + specPath: '/path/to/RELEASE_SPEC.yml', + }); + + expect(error.describe(format)).toStrictEqual({ + title: 'The release spec has 1 problem:', + lines: [ + '', + ' is not a package', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '(/path/to/RELEASE_SPEC.yml)', + ], + }); + expect(error.message).toBe( + [ + 'The release spec has 1 problem:', + '', + 'a', + ' is not a package', + '', + 'Fix the release spec and run the tool again. It was kept at:', + '/path/to/RELEASE_SPEC.yml', + ].join('\n'), + ); + }); + }); }); diff --git a/packages/create-release-branch/src/frontends/editor/release-specification.ts b/packages/create-release-branch/src/frontends/editor/release-specification.ts index fc98976..6d996cf 100644 --- a/packages/create-release-branch/src/frontends/editor/release-specification.ts +++ b/packages/create-release-branch/src/frontends/editor/release-specification.ts @@ -1,12 +1,14 @@ import YAML from 'yaml'; import type { Candidate } from '../../core/candidates.js'; +import type { CliOutput } from '../../core/cli-output.js'; import type { ModeName } from '../../core/mode.js'; import { SKIP_PACKAGE_DIRECTIVE } from '../../core/release-rules.js'; import type { ReleaseChoiceError, ReleaseChoices, } from '../../core/release-rules.js'; +import { UserFacingError } from '../../core/user-facing-error.js'; import { readFile } from '../../utils/fs.js'; import { debug, @@ -75,6 +77,88 @@ const TEMPLATE_TEXT_BY_MODE: Record = { }, }; +/** + * A problem in the release spec. + * + * @property lineNumber - The line of the release spec that has the problem. + * @property packageName - The name of the package that has the problem. + * @property problem - What is wrong, written to follow the package name, or + * "The release spec" if there is no package. + * @property hint - How to fix it. + */ +export type ReleaseSpecificationProblem = { + lineNumber?: number; + packageName?: string; + problem: string; + hint?: string; +}; + +/** + * The problems in a release spec that the user has edited. The release spec + * is kept, so the user can fix it and run the tool again. + */ +export class ReleaseSpecificationError extends UserFacingError { + problems: ReleaseSpecificationProblem[]; + + specPath: string; + + /** + * Creates the error. + * + * @param args - The arguments. + * @param args.problems - The problems in the release spec. + * @param args.specPath - The path to the release spec. + * @param options - The options. + * @param options.cause - The error that caused the problems. + */ + constructor( + { + problems, + specPath, + }: { problems: ReleaseSpecificationProblem[]; specPath: string }, + options?: { cause: unknown }, + ) { + super(undefined, options); + this.problems = problems; + this.specPath = specPath; + const { title, lines } = this.describe({ + name: (text) => text, + dim: (text) => text, + }); + this.message = [title, '', ...lines].join('\n'); + } + + describe(format: CliOutput['format']): { title: string; lines: string[] } { + const count = this.problems.length; + const title = `The release spec has ${count} ${count === 1 ? 'problem' : 'problems'}:`; + const problemLines = this.problems.map( + ({ lineNumber, packageName, problem, hint }) => { + const subject = + packageName === undefined + ? 'The release spec' + : format.name(packageName); + const linePrefix = + lineNumber === undefined ? '' : `Line ${lineNumber} `; + const indent = ' '.repeat(Math.max(linePrefix.length, 2)); + return [ + `${linePrefix}${subject}`, + `${indent}${problem}`, + ...(hint === undefined ? [] : [`${indent}${format.dim(hint)}`]), + ]; + }, + ); + + return { + title, + lines: [ + ...problemLines.flatMap((lines) => [...lines, '']), + 'Fix the release spec and run the tool again. It was kept at:', + format.dim(this.specPath), + ], + }; + } +} + /** * Generates a skeleton for a release specification, which describes how a * project should be updated. @@ -156,7 +240,7 @@ export async function waitForUserToEditReleaseSpecification( * @param check - Checks the release choices. It returns the plan only when * there are no errors. * @returns The plan that `check` returns. - * @throws If there are any issues with the file. + * @throws A `ReleaseSpecificationError` if there are any issues with the file. */ export async function validateReleaseSpecification( releaseSpecificationPath: string, @@ -173,22 +257,24 @@ export async function validateReleaseSpecification( packages: ReleaseChoices; }; - const afterwordForAllErrorMessages = [ - "The release spec file has been retained for you to edit again and make the necessary fixes. Once you've done this, re-run this tool.", - releaseSpecificationPath, - ].join('\n\n'); - try { unvalidatedReleaseSpecification = YAML.parse( releaseSpecificationContents, ) as { packages: ReleaseChoices }; } catch (error) { - throw wrapError( - [ - 'Your release spec does not appear to be valid YAML.', - afterwordForAllErrorMessages, - ].join('\n\n'), - error, + // Typecast: `YAML.parse` throws a `YAMLParseError`, which is an `Error`. + const [firstLineOfMessage] = (error as Error).message.split('\n'); + throw new ReleaseSpecificationError( + { + problems: [ + { + problem: 'is not valid YAML', + hint: `Fix the syntax: ${firstLineOfMessage.replace(/:$/u, '')}`, + }, + ], + specPath: releaseSpecificationPath, + }, + { cause: error }, ); } @@ -196,54 +282,28 @@ export async function validateReleaseSpecification( !isObject(unvalidatedReleaseSpecification) || unvalidatedReleaseSpecification.packages === undefined ) { - const message = [ - `Your release spec could not be processed because it needs to be an object with a \`packages\` property. The value of \`packages\` must itself be an object, where each key is a workspace package in the project and each value is a version specifier ("major", "minor", or "patch"; or a version string with major, minor, and patch parts, such as "1.2.3").`, - `Here is the parsed version of the file you provided:`, - JSON.stringify(unvalidatedReleaseSpecification, null, 2), - afterwordForAllErrorMessages, - ].join('\n\n'); - throw new Error(message); + throw new ReleaseSpecificationError({ + problems: [ + { + problem: + 'needs a packages object, where each key is a package name and each value is a version specifier', + }, + ], + specPath: releaseSpecificationPath, + }); } const result = await check(unvalidatedReleaseSpecification.packages); if (!('plan' in result)) { - const errors = result.errors.map(({ message, entryIndex }) => ({ - message, - lineNumber: + throw new ReleaseSpecificationError({ + problems: result.errors.map(({ entryIndex, ...problem }) => entryIndex === undefined - ? undefined - : indexOfFirstUsableLine + entryIndex + 2, - })); - const message = [ - 'Your release spec could not be processed due to the following issues:', - errors - .flatMap((error) => { - const itemPrefix = '* '; - - if (error.lineNumber === undefined) { - return `${itemPrefix}${String(error.message)}`; - } - - const lineNumberPrefix = `Line ${error.lineNumber}: `; - - if (Array.isArray(error.message)) { - return [ - `${itemPrefix}${lineNumberPrefix}${error.message[0]}`, - ...error.message.slice(1).map((line) => { - const indentedLineLength = - itemPrefix.length + lineNumberPrefix.length + line.length; - return line.padStart(indentedLineLength, ' '); - }), - ]; - } - - return `${itemPrefix}${lineNumberPrefix}${error.message}`; - }) - .join('\n'), - afterwordForAllErrorMessages, - ].join('\n\n'); - throw new Error(message); + ? problem + : { lineNumber: indexOfFirstUsableLine + entryIndex + 2, ...problem }, + ), + specPath: releaseSpecificationPath, + }); } return result.plan; diff --git a/packages/create-release-branch/src/frontends/ui/server.test.ts b/packages/create-release-branch/src/frontends/ui/server.test.ts index 33f191e..a8e065e 100644 --- a/packages/create-release-branch/src/frontends/ui/server.test.ts +++ b/packages/create-release-branch/src/frontends/ui/server.test.ts @@ -274,12 +274,16 @@ describe('server', () => { status: 'error', errors: [ { - message: expect.stringContaining( - "The following direct dependents of package 'a'", - ) as string, + message: [ + 'a: has a major bump, but its direct dependents are not in the release spec: b', + 'Add them to the release spec, or set them to intentionally-skip.', + ], }, { - message: '"nonexistent" is not a package in the project', + message: [ + 'nonexistent: is not a package in the project', + 'Check the name, or remove the line.', + ], lineNumber: 3, }, ], @@ -291,6 +295,36 @@ describe('server', () => { expect(closeServer).not.toHaveBeenCalled(); }); + it('returns the problem and hint as the message', async () => { + const mode = buildMockMode({ + plan: vi.fn().mockResolvedValue({ + errors: [ + { packageName: 'a', problem: 'is wrong', hint: 'Fix it.' }, + { problem: 'is broken' }, + ], + }), + }); + + await withApp( + { + context: buildContext(mode), + newReleaseVersion: '2.0.0', + closeServer: vi.fn(), + }, + async (url) => { + const response = await postChoices(`${url}/api/release`, {}); + + expect(await response.json()).toStrictEqual({ + status: 'error', + errors: [ + { message: ['a: is wrong', 'Fix it.'] }, + { message: 'is broken' }, + ], + }); + }, + ); + }); + it('applies, finishes, and closes the server', async () => { const plan = { releases: [{ name: 'a', newVersion: '1.1.0' }], diff --git a/packages/create-release-branch/src/frontends/ui/server.ts b/packages/create-release-branch/src/frontends/ui/server.ts index 591d440..1dabd99 100644 --- a/packages/create-release-branch/src/frontends/ui/server.ts +++ b/packages/create-release-branch/src/frontends/ui/server.ts @@ -193,10 +193,17 @@ export function createApp({ if (!('plan' in result)) { res.json({ status: 'error', - errors: result.errors.map(({ message, entryIndex }) => - entryIndex === undefined - ? { message } - : { message, lineNumber: entryIndex + 2 }, + errors: result.errors.map( + ({ packageName, entryIndex, problem, hint }) => { + const text = + packageName === undefined + ? problem + : `${packageName}: ${problem}`; + const message = hint === undefined ? text : [text, hint]; + return entryIndex === undefined + ? { message } + : { message, lineNumber: entryIndex + 2 }; + }, ), }); return;