Merge pull request #22554 from storybookjs/fix/reduce-package-install…

…-noise

CLI: Reduce installation noise and improve error handling

code/lib/cli/package.json

@@ -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",

code/lib/cli/src/generators/baseGenerator.ts

@@ -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';
@@ -16,6 +17,9 @@ import {
   extractEslintInfo,
   suggestESLintPlugin,
 } from '../automigrate/helpers/eslintPlugin';
+import { HandledError } from '../HandledError';
+
+const logger = console;
 
 const defaultOptions: FrameworkOptions = {
   extraPackages: [],
@@ -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,
@@ -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}`);
 
@@ -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);
 }

code/lib/cli/src/initiate.ts

@@ -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);
@@ -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([
       {

code/lib/cli/src/js-package-manager/JsPackageManager.ts

@@ -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);
       }
@@ -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,

code/lib/cli/src/js-package-manager/NPMProxy.test.ts

@@ -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;
 
@@ -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: before-storybook@1.0.0
+        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/react@7.1.0-alpha.17
+        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: before-storybook@1.0.0
+        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`);
+    });
+  });
 });

code/lib/cli/src/js-package-manager/NPMProxy.ts

@@ -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;
@@ -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';
 
@@ -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[]) {
@@ -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();
+  }
 }

code/lib/cli/src/js-package-manager/PNPMProxy.test.ts

@@ -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 react@29.2.0
+
+        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 react@29.2.0'
+      );
+    });
+
+    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`);
+    });
+  });
 });