Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CLI: Reduce installation noise and improve error handling #22554

Merged
merged 7 commits into from
May 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions code/lib/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
"globby": "^11.0.2",
"jscodeshift": "^0.14.0",
"leven": "^3.1.0",
"ora": "^5.4.1",
"prettier": "^2.8.0",
"prompts": "^2.4.0",
"puppeteer-core": "^2.1.1",
Expand Down
75 changes: 62 additions & 13 deletions code/lib/cli/src/generators/baseGenerator.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import path from 'path';
import fse from 'fs-extra';
import { dedent } from 'ts-dedent';
import ora from 'ora';
import type { NpmOptions } from '../NpmOptions';
import type { SupportedRenderers, SupportedFrameworks, Builder } from '../project_types';
import { SupportedLanguage, externalFrameworks, CoreBuilder } from '../project_types';
import { copyTemplateFiles, paddedLog } from '../helpers';
import { copyTemplateFiles } from '../helpers';
import { configureMain, configurePreview } from './configure';
import type { JsPackageManager } from '../js-package-manager';
import { getPackageDetails } from '../js-package-manager';
Expand All @@ -16,6 +17,9 @@ import {
extractEslintInfo,
suggestESLintPlugin,
} from '../automigrate/helpers/eslintPlugin';
import { HandledError } from '../HandledError';

const logger = console;

const defaultOptions: FrameworkOptions = {
extraPackages: [],
Expand Down Expand Up @@ -168,6 +172,31 @@ export async function baseGenerator(
options: FrameworkOptions = defaultOptions,
framework?: SupportedFrameworks
) {
// This is added so that we can handle the scenario where the user presses Ctrl+C and report a canceled event.
// Given that there are subprocesses running as part of this function, we need to handle the signal ourselves otherwise it might run into race conditions.
// TODO: This should be revisited once we have a better way to handle this.
let isNodeProcessExiting = false;
const setNodeProcessExiting = () => {
isNodeProcessExiting = true;
};
process.on('SIGINT', setNodeProcessExiting);

const stopIfExiting = async <T>(callback: () => Promise<T>) => {
if (isNodeProcessExiting) {
throw new HandledError('Canceled by the user');
}

try {
return await callback();
} catch (error) {
if (isNodeProcessExiting) {
throw new HandledError('Canceled by the user');
}

throw error;
}
};

const {
extraAddons: extraAddonPackages,
extraPackages,
Expand Down Expand Up @@ -254,7 +283,15 @@ export async function baseGenerator(
(packageToInstall) => !installedDependencies.has(getPackageDetails(packageToInstall)[0])
);

const versionedPackages = await packageManager.getVersionedPackages(packages);
logger.log();
const versionedPackagesSpinner = ora({
indent: 2,
text: `Getting the correct version of ${packages.length} packages`,
}).start();
const versionedPackages = await stopIfExiting(async () =>
packageManager.getVersionedPackages(packages)
);
versionedPackagesSpinner.succeed();

await fse.ensureDir(`./${storybookConfigFolder}`);

Expand Down Expand Up @@ -333,23 +370,35 @@ export async function baseGenerator(
}

if (depsToInstall.length > 0) {
paddedLog('Installing Storybook dependencies');
await packageManager.addDependencies({ ...npmOptions, packageJson }, depsToInstall);
const addDependenciesSpinner = ora({
indent: 2,
text: 'Installing Storybook dependencies',
}).start();
await stopIfExiting(async () =>
packageManager.addDependencies({ ...npmOptions, packageJson }, depsToInstall)
);
addDependenciesSpinner.succeed();
}

if (addScripts) {
await packageManager.addStorybookCommandInScripts({
port: 6006,
});
await stopIfExiting(async () =>
packageManager.addStorybookCommandInScripts({
port: 6006,
})
);
}

if (addComponents) {
const templateLocation = hasFrameworkTemplates(framework) ? framework : rendererId;
await copyTemplateFiles({
renderer: templateLocation,
packageManager,
language,
destination: componentsDestinationPath,
});
await stopIfExiting(async () =>
copyTemplateFiles({
renderer: templateLocation,
packageManager,
language,
destination: componentsDestinationPath,
})
);
}

process.off('SIGINT', setNodeProcessExiting);
}
4 changes: 2 additions & 2 deletions code/lib/cli/src/initiate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ const installStorybook = async <Project extends ProjectType>(
try {
return await runGenerator();
} catch (err) {
if (err?.stack) {
if (err?.message !== 'Canceled by the user' && err?.stack) {
logger.error(`\n ${chalk.red(err.stack)}`);
}
throw new HandledError(err);
Expand Down Expand Up @@ -286,7 +286,7 @@ async function doInitiate(options: CommandOptions, pkg: PackageJson): Promise<vo

const storybookInstantiated = isStorybookInstantiated();

if (storybookInstantiated && projectType !== ProjectType.ANGULAR) {
if (options.force === false && storybookInstantiated && projectType !== ProjectType.ANGULAR) {
logger.log();
const { force } = await prompts([
{
Expand Down
3 changes: 2 additions & 1 deletion code/lib/cli/src/js-package-manager/JsPackageManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export abstract class JsPackageManager {
try {
await this.runAddDeps(dependencies, options.installAsDevDependencies);
} catch (e) {
logger.error('An error occurred while installing dependencies.');
logger.error('\nAn error occurred while installing dependencies:');
logger.log(e.message);
throw new HandledError(e);
}
Expand Down Expand Up @@ -421,6 +421,7 @@ export abstract class JsPackageManager {
stdio?: 'inherit' | 'pipe'
): string;
public abstract findInstallations(pattern?: string[]): Promise<InstallationMetadata | undefined>;
public abstract parseErrorFromLogs(logs?: string): string;

public executeCommandSync({
command,
Expand Down
56 changes: 56 additions & 0 deletions code/lib/cli/src/js-package-manager/NPMProxy.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,15 @@
import { NPMProxy } from './NPMProxy';

// mock createLogStream
jest.mock('../utils', () => ({
createLogStream: jest.fn(() => ({
logStream: '',
readLogFile: jest.fn(),
moveLogFile: jest.fn(),
removeLogFile: jest.fn(),
})),
}));

describe('NPM Proxy', () => {
let npmProxy: NPMProxy;

Expand Down Expand Up @@ -426,4 +436,50 @@ describe('NPM Proxy', () => {
`);
});
});

describe('parseErrors', () => {
it('should parse npm errors', () => {
const NPM_ERROR_SAMPLE = `
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: react@undefined
npm ERR! node_modules/react
npm ERR! react@"30" from the root project
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.8.0 || ^17.0.0 || ^18.0.0" from @storybook/[email protected]
npm ERR! node_modules/@storybook/react
npm ERR! dev @storybook/react@"^7.1.0-alpha.17" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR!
npm ERR! For a full report see:
npm ERR! /Users/yannbraga/.npm/_logs/2023-05-12T08_38_18_464Z-eresolve-report.txt
npm ERR! A complete log of this run can be found in:
npm ERR! /Users/yannbraga/.npm/_logs/2023-05-12T08_38_18_464Z-debug-0.log
`;

expect(npmProxy.parseErrorFromLogs(NPM_ERROR_SAMPLE)).toEqual(
'NPM error ERESOLVE - Dependency resolution error.'
);
});

it('should show unknown npm error', () => {
const NPM_ERROR_SAMPLE = `
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: react@undefined
npm ERR! node_modules/react
npm ERR! react@"30" from the root project
`;

expect(npmProxy.parseErrorFromLogs(NPM_ERROR_SAMPLE)).toEqual(`NPM error`);
});
});
});
84 changes: 79 additions & 5 deletions code/lib/cli/src/js-package-manager/NPMProxy.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import sort from 'semver/functions/sort';
import { platform } from 'os';
import dedent from 'ts-dedent';
import { JsPackageManager } from './JsPackageManager';
import type { PackageJson } from './PackageJson';
import type { InstallationMetadata, PackageMetadata } from './types';
import { createLogStream } from '../utils';

type NpmDependency = {
version: string;
Expand All @@ -19,6 +21,41 @@ export type NpmListOutput = {
dependencies: NpmDependencies;
};

const NPM_ERROR_REGEX = /\bERR! code\s+([A-Z]+)\b/;
const NPM_ERROR_CODES = {
E401: 'Authentication failed or is required.',
E403: 'Access to the resource is forbidden.',
E404: 'Requested resource not found.',
EACCES: 'Permission issue.',
EAI_FAIL: 'DNS lookup failed.',
EBADENGINE: 'Engine compatibility check failed.',
EBADPLATFORM: 'Platform not supported.',
ECONNREFUSED: 'Connection refused.',
ECONNRESET: 'Connection reset.',
EEXIST: 'File or directory already exists.',
EINVALIDTYPE: 'Invalid type encountered.',
EISGIT: 'Git operation failed or conflicts with an existing file.',
EJSONPARSE: 'Error parsing JSON data.',
EMISSINGARG: 'Required argument missing.',
ENEEDAUTH: 'Authentication needed.',
ENOAUDIT: 'No audit available.',
ENOENT: 'File or directory does not exist.',
ENOGIT: 'Git not found or failed to run.',
ENOLOCK: 'Lockfile missing.',
ENOSPC: 'Insufficient disk space.',
ENOTFOUND: 'Resource not found.',
EOTP: 'One-time password required.',
EPERM: 'Permission error.',
EPUBLISHCONFLICT: 'Conflict during package publishing.',
ERESOLVE: 'Dependency resolution error.',
EROFS: 'File system is read-only.',
ERR_SOCKET_TIMEOUT: 'Socket timed out.',
ETARGET: 'Package target not found.',
ETIMEDOUT: 'Operation timed out.',
ETOOMANYARGS: 'Too many arguments provided.',
EUNKNOWNTYPE: 'Unknown type encountered.',
};

export class NPMProxy extends JsPackageManager {
readonly type = 'npm';

Expand Down Expand Up @@ -104,17 +141,34 @@ export class NPMProxy extends JsPackageManager {
}

protected async runAddDeps(dependencies: string[], installAsDevDependencies: boolean) {
const { logStream, readLogFile, moveLogFile, removeLogFile } = await createLogStream();
let args = [...dependencies];

if (installAsDevDependencies) {
args = ['-D', ...args];
}

await this.executeCommand({
command: 'npm',
args: ['install', ...this.getInstallArgs(), ...args],
stdio: 'inherit',
});
try {
await this.executeCommand({
command: 'npm',
args: ['install', ...args, ...this.getInstallArgs()],
stdio: ['ignore', logStream, logStream],
});
} catch (err) {
const stdout = await readLogFile();

const errorMessage = this.parseErrorFromLogs(stdout);

await moveLogFile();

throw new Error(
dedent`${errorMessage}

Please check the logfile generated at ./storybook.log for troubleshooting and try again.`
);
}

await removeLogFile();
}

protected async runRemoveDeps(dependencies: string[]) {
Expand Down Expand Up @@ -191,4 +245,24 @@ export class NPMProxy extends JsPackageManager {
infoCommand: 'npm ls --depth=1',
};
}

public parseErrorFromLogs(logs: string): string {
let finalMessage = 'NPM error';
console.log({ logs });
const match = logs.match(NPM_ERROR_REGEX);

if (match) {
const errorCode = match[1] as keyof typeof NPM_ERROR_CODES;
if (errorCode) {
finalMessage = `${finalMessage} ${errorCode}`;
}

const errorMessage = NPM_ERROR_CODES[errorCode];
if (errorMessage) {
finalMessage = `${finalMessage} - ${errorMessage}`;
}
}

return finalMessage.trim();
}
}
26 changes: 26 additions & 0 deletions code/lib/cli/src/js-package-manager/PNPMProxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,4 +375,30 @@ describe('NPM Proxy', () => {
`);
});
});

describe('parseErrors', () => {
it('should parse pnpm errors', () => {
const PNPM_ERROR_SAMPLE = `
ERR_PNPM_NO_MATCHING_VERSION No matching version found for [email protected]
This error happened while installing a direct dependency of /Users/yannbraga/open-source/sandboxes/react-vite/default-js/before-storybook
The latest release of react is "18.2.0".
`;

expect(pnpmProxy.parseErrorFromLogs(PNPM_ERROR_SAMPLE)).toEqual(
'PNPM error ERR_PNPM_NO_MATCHING_VERSION No matching version found for [email protected]'
);
});

it('should show unknown pnpm error', () => {
const PNPM_ERROR_SAMPLE = `
This error happened while installing a direct dependency of /Users/yannbraga/open-source/sandboxes/react-vite/default-js/before-storybook
The latest release of react is "18.2.0".
`;

expect(pnpmProxy.parseErrorFromLogs(PNPM_ERROR_SAMPLE)).toEqual(`PNPM error`);
});
});
});
Loading