From 553789e83af83c57179659c26223d87f043f8460 Mon Sep 17 00:00:00 2001 From: Yann Braga Date: Fri, 1 Sep 2023 18:48:17 +0200 Subject: [PATCH] standardize errors thrown in webpack builder - also bring back error logging on dev mode, which seemed to be lost in Storybook 7.0.0 --- code/builders/builder-webpack5/src/index.ts | 131 +++++++++--------- .../core-events/src/errors/server-errors.ts | 85 ++++++++---- code/lib/core-server/src/build-static.ts | 12 +- code/lib/core-server/src/dev-server.ts | 5 + 4 files changed, 140 insertions(+), 93 deletions(-) diff --git a/code/builders/builder-webpack5/src/index.ts b/code/builders/builder-webpack5/src/index.ts index 45775dc7d04f..582c3156c71b 100644 --- a/code/builders/builder-webpack5/src/index.ts +++ b/code/builders/builder-webpack5/src/index.ts @@ -9,7 +9,11 @@ import { dirname, join, parse } from 'path'; import express from 'express'; import fs from 'fs-extra'; import { PREVIEW_BUILDER_PROGRESS } from '@storybook/core-events'; -import { WebpackCompilationError } from '@storybook/core-events/server-errors'; +import { + WebpackCompilationError, + WebpackInvocationError, + WebpackMissingStatsError, +} from '@storybook/core-events/server-errors'; import prettyTime from 'pretty-hrtime'; @@ -118,21 +122,19 @@ const starter: StarterFunction = async function* starterGeneratorFn({ yield; const config = await getConfig(options); + + if (config.stats === 'none' || config.stats === 'summary') { + throw new WebpackMissingStatsError(); + } yield; + const compiler = webpackInstance(config); if (!compiler) { - const err = `${config.name}: missing webpack compiler at runtime!`; - logger.error(err); - return { - bail, - totalTime: process.hrtime(startTime), - stats: { - hasErrors: () => true, - hasWarnings: () => false, - toJson: () => ({ warnings: [] as any[], errors: [err] }), - } as any as Stats, - }; + throw new WebpackInvocationError({ + // eslint-disable-next-line local-rules/no-uncategorized-errors + error: new Error(`Missing Webpack compiler at runtime!`), + }); } yield; @@ -173,6 +175,7 @@ const starter: StarterFunction = async function* starterGeneratorFn({ const middlewareOptions: Parameters[1] = { publicPath: config.output?.publicPath as string, writeToDisk: true, + stats: 'errors-only', }; compilation = webpackDevMiddleware(compiler, middlewareOptions); @@ -185,18 +188,24 @@ const starter: StarterFunction = async function* starterGeneratorFn({ router.use(compilation); router.use(webpackHotMiddleware(compiler, { log: false })); - const stats = await new Promise((ready, stop) => { - compilation?.waitUntilValid(ready as any); - reject = stop; + const stats = await new Promise((res, rej) => { + compilation?.waitUntilValid(res as any); + reject = rej; }); yield; if (!stats) { - throw new Error('no stats after building preview'); + throw new WebpackMissingStatsError(); } - if (stats.hasErrors()) { - throw new WebpackCompilationError({ error: stats }); + const { warnings, errors } = getWebpackStats({ config, stats }); + + if (warnings.length > 0) { + warnings?.forEach((e) => logger.error(e.message)); + } + + if (errors.length > 0) { + throw new WebpackCompilationError({ errors }); } return { @@ -206,6 +215,22 @@ const starter: StarterFunction = async function* starterGeneratorFn({ }; }; +function getWebpackStats({ config, stats }: { config: Configuration; stats: Stats }) { + const statsOptions = + typeof config.stats === 'string' + ? config.stats + : { + ...(config.stats as StatsOptions), + warnings: true, + errors: true, + }; + const { warnings = [], errors = [] } = stats?.toJson(statsOptions) || {}; + return { + warnings, + errors, + }; +} + /** * This function is a generator so that we can abort it mid process * in case of failure coming from other processes e.g. manager builder @@ -215,73 +240,47 @@ const starter: StarterFunction = async function* starterGeneratorFn({ const builder: BuilderFunction = async function* builderGeneratorFn({ startTime, options }) { const webpackInstance = await executor.get(options); yield; - logger.info('=> Compiling preview..'); const config = await getConfig(options); + + if (config.stats === 'none' || config.stats === 'summary') { + throw new WebpackMissingStatsError(); + } yield; const compiler = webpackInstance(config); if (!compiler) { - const err = `${config.name}: missing webpack compiler at runtime!`; - logger.error(err); - return { - hasErrors: () => true, - hasWarnings: () => false, - toJson: () => ({ warnings: [] as any[], errors: [err] }), - } as any as Stats; + throw new WebpackInvocationError({ + // eslint-disable-next-line local-rules/no-uncategorized-errors + error: new Error(`Missing Webpack compiler at runtime!`), + }); } const webpackCompilation = new Promise((succeed, fail) => { compiler.run((error, stats) => { - if (error || !stats || stats.hasErrors()) { - logger.error('=> Failed to build the preview'); - process.exitCode = 1; - - if (error) { - logger.error(error.message); + if (error) { + compiler.close(() => fail(new WebpackInvocationError({ error }))); + return; + } - compiler.close(() => fail(error)); + if (!stats) { + throw new WebpackMissingStatsError(); + } - return; - } + const { warnings, errors } = getWebpackStats({ config, stats }); - if (stats && (stats.hasErrors() || stats.hasWarnings())) { - const { warnings = [], errors = [] } = stats.toJson( - typeof config.stats === 'string' - ? config.stats - : { - warnings: true, - errors: true, - ...(config.stats as StatsOptions), - } - ); - - errors.forEach((e) => logger.error(e.message)); - warnings.forEach((e) => logger.error(e.message)); - - compiler.close(() => - options.debugWebpack - ? fail(stats) - : fail(new Error('=> Webpack failed, learn more with --debug-webpack')) - ); - - return; - } + if (warnings.length > 0) { + warnings?.forEach((e) => logger.error(e.message)); } - logger.trace({ message: '=> Preview built', time: process.hrtime(startTime) }); - if (stats && stats.hasWarnings()) { - // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- we know it has warnings because of hasWarnings() - stats - .toJson({ warnings: true } as StatsOptions) - .warnings!.forEach((e) => logger.warn(e.message)); + if (errors.length > 0) { + compiler.close(() => fail(new WebpackCompilationError({ errors }))); + return; } - // https://webpack.js.org/api/node/#run - // #15227 compiler.close((closeErr) => { if (closeErr) { - return fail(closeErr); + return fail(new WebpackInvocationError({ error: closeErr })); } return succeed(stats as Stats); diff --git a/code/lib/core-events/src/errors/server-errors.ts b/code/lib/core-events/src/errors/server-errors.ts index 5c1eb801ee57..4c3e10abf8e4 100644 --- a/code/lib/core-events/src/errors/server-errors.ts +++ b/code/lib/core-events/src/errors/server-errors.ts @@ -138,47 +138,80 @@ export class InvalidStoriesEntryError extends StorybookError { } } -export class WebpackCompilationError extends StorybookError { +export class WebpackMissingStatsError extends StorybookError { readonly category = Category.BUILDER_WEBPACK5; readonly code = 1; + public documentation = [ + 'https://webpack.js.org/configuration/stats/', + 'https://storybook.js.org/docs/react/builders/webpack#configure', + ]; + + template() { + return dedent` + No Webpack stats found. Did you turn off stats reporting in your webpack config? + Storybook needs Webpack stats (including errors) in order to build correctly. + `; + } +} + +export class WebpackInvocationError extends StorybookError { + readonly category = Category.BUILDER_WEBPACK5; + + readonly code = 2; + private errorMessage = ''; constructor( public data: { - error: - | (Error & { - error?: Error; - stats?: { compilation: { errors: Error[] } }; - compilation?: { errors: Error[] }; - }) - | { - compilation?: { errors: Error[] }; - }; + error: Error; } ) { super(); + this.errorMessage = data.error.message; + } + + template() { + return this.errorMessage.trim(); + } +} +function removeAnsiEscapeCodes(input = '') { + // eslint-disable-next-line no-control-regex + return input.replace(/\u001B\[[0-9;]*m/g, ''); +} + +export class WebpackCompilationError extends StorybookError { + readonly category = Category.BUILDER_WEBPACK5; + + readonly code = 3; - if (data.error instanceof Error) { - if (data.error.error) { - this.errorMessage = data.error.error.message; - this.stack = data.error.error.stack; - } else if (data.error.stats && data.error.stats.compilation.errors) { - data.error.stats.compilation.errors.forEach((e: Error) => { - this.errorMessage += `${e.name}: ${e.message}\n\n`; - }); - } else { - this.errorMessage = data.error.message; - } - } else if (data.error.compilation?.errors) { - data.error.compilation.errors.forEach((e: Error) => { - this.errorMessage += `${e.name}: ${e.message}\n\n`; - }); + constructor( + public data: { + errors: { + message: string; + stack?: string; + name?: string; + }[]; } + ) { + super(); + + this.data.errors = data.errors.map((e, i) => { + return { + ...e, + message: removeAnsiEscapeCodes(e.message), + stack: removeAnsiEscapeCodes(e.stack), + name: e.name, + }; + }); } template() { - return this.errorMessage.trim(); + // This error message is a followup of errors logged by Webpack to the user + return dedent` + There were problems when compiling your code with Webpack. + Run Storybook with --debug-webpack for more information. + `; } } diff --git a/code/lib/core-server/src/build-static.ts b/code/lib/core-server/src/build-static.ts index 810c871724fd..b8ca4a95fa6c 100644 --- a/code/lib/core-server/src/build-static.ts +++ b/code/lib/core-server/src/build-static.ts @@ -199,23 +199,33 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption if (options.ignorePreview) { logger.info(`=> Not building preview`); + } else { + logger.info('=> Building preview..'); } + const startTime = process.hrtime(); await Promise.all([ ...(options.ignorePreview ? [] : [ previewBuilder .build({ - startTime: process.hrtime(), + startTime, options: fullOptions, }) .then(async (previewStats) => { + logger.trace({ message: '=> Preview built', time: process.hrtime(startTime) }); + if (options.webpackStatsJson) { const target = options.webpackStatsJson === true ? options.outputDir : options.webpackStatsJson; await outputStats(target, previewStats); } + }) + .catch((error) => { + logger.error('=> Failed to build the preview'); + process.exitCode = 1; + throw error; }), ]), ...effects, diff --git a/code/lib/core-server/src/dev-server.ts b/code/lib/core-server/src/dev-server.ts index 2eb1ea420cda..50d4c8dd4cdc 100644 --- a/code/lib/core-server/src/dev-server.ts +++ b/code/lib/core-server/src/dev-server.ts @@ -6,6 +6,7 @@ import type { CoreConfig, Options, StorybookConfig } from '@storybook/types'; import { logConfig } from '@storybook/core-common'; +import { logger } from '@storybook/node-logger'; import { getMiddleware } from './utils/middleware'; import { getServerAddresses } from './utils/server-address'; import { getServer } from './utils/server-init'; @@ -90,6 +91,7 @@ export async function storybookDevServer(options: Options) { let previewStarted: Promise = Promise.resolve(); if (!options.ignorePreview) { + logger.info('=> Starting preview..'); previewStarted = previewBuilder .start({ startTime: process.hrtime(), @@ -99,6 +101,9 @@ export async function storybookDevServer(options: Options) { channel: serverChannel, }) .catch(async (e: any) => { + logger.error('=> Failed to build the preview'); + process.exitCode = 1; + await managerBuilder?.bail().catch(); // For some reason, even when Webpack fails e.g. wrong main.js config, // the preview may continue to print to stdout, which can affect output