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 1 commit
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
58 changes: 47 additions & 11 deletions code/lib/cli/src/generators/baseGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
extractEslintInfo,
suggestESLintPlugin,
} from '../automigrate/helpers/eslintPlugin';
import { HandledError } from '../HandledError';

const logger = console;

Expand Down Expand Up @@ -171,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 @@ -262,7 +288,9 @@ export async function baseGenerator(
indent: 2,
text: `Getting the correct version of ${packages.length} packages`,
}).start();
const versionedPackages = await packageManager.getVersionedPackages(packages);
const versionedPackages = await stopIfExiting(async () =>
packageManager.getVersionedPackages(packages)
);
versionedPackagesSpinner.succeed();

await fse.ensureDir(`./${storybookConfigFolder}`);
Expand Down Expand Up @@ -346,23 +374,31 @@ export async function baseGenerator(
indent: 2,
text: 'Installing Storybook dependencies',
}).start();
await packageManager.addDependencies({ ...npmOptions, packageJson }, depsToInstall);
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);
}
10 changes: 2 additions & 8 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 @@ -238,12 +238,6 @@ const projectTypeInquirer = async (
};

async function doInitiate(options: CommandOptions, pkg: PackageJson): Promise<void> {
process.on('SIGINT', () => {
logger.log();
logger.log('Storybook init canceled by the user.');
process.exit(0);
});

let { packageManager: pkgMgr } = options;
if (options.useNpm) {
useNpmWarning();
Expand Down Expand Up @@ -292,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
16 changes: 15 additions & 1 deletion code/lib/core-server/src/withTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,30 @@ export async function withTelemetry<T>(
options: TelemetryOptions,
run: () => Promise<T>
): Promise<T> {
// We catch Ctrl+C user interactions to be able to detect a cancel event
process.on('SIGINT', async () => {
if (!options.cliOptions.disableTelemetry) {
await telemetry('canceled', { eventType }, { stripMetadata: true, immediate: true });
process.exit(0);
}

process.exit(0);
});

if (!options.cliOptions.disableTelemetry)
telemetry('boot', { eventType }, { stripMetadata: true });

try {
return await run();
} catch (error) {
if (error?.message === 'Canceled by the user') {
yannbf marked this conversation as resolved.
Show resolved Hide resolved
return undefined;
}

const { printError = logger.error } = options;
printError(error);

await sendTelemetryError(error, eventType, options);

throw error;
}
}
1 change: 1 addition & 0 deletions code/lib/telemetry/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export type EventType =
| 'build'
| 'upgrade'
| 'init'
| 'canceled'
| 'error'
| 'error-metadata'
| 'version-update';
Expand Down