Skip to content

Commit

Permalink
Merge pull request #23912 from storybookjs/yann/categorize-errors-mai…
Browse files Browse the repository at this point in the history
…n-config

Maintenance: Categorize server errors
  • Loading branch information
yannbf authored Aug 23, 2023
2 parents 6889787 + af50245 commit a85f652
Show file tree
Hide file tree
Showing 6 changed files with 114 additions and 42 deletions.
28 changes: 8 additions & 20 deletions code/lib/core-common/src/utils/validate-config.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { join } from 'path';
import { dedent } from 'ts-dedent';
import {
CouldNotEvaluateFrameworkError,
MissingFrameworkFieldError,
InvalidFrameworkNameError,
} from '@storybook/core-events/server-errors';
import { frameworkPackages } from './get-storybook-info';

const renderers = ['html', 'preact', 'react', 'server', 'svelte', 'vue', 'vue3', 'web-components'];
Expand All @@ -9,28 +13,15 @@ const rendererNames = [...renderers, ...renderers.map((renderer) => `@storybook/
export function validateFrameworkName(
frameworkName: string | undefined
): asserts frameworkName is string {
const automigrateMessage = `Please run 'npx storybook@next automigrate' to automatically fix your config.
See the migration guide for more information:
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api\n`;

// Throw error if there is no framework field
// TODO: maybe this error should not be thrown if we allow empty Storybooks that only use "refs" for composition
if (!frameworkName) {
throw new Error(dedent`
Could not find a 'framework' field in Storybook config.
${automigrateMessage}
`);
throw new MissingFrameworkFieldError();
}

// Account for legacy scenario where the framework was referring to a renderer
if (rendererNames.includes(frameworkName)) {
throw new Error(dedent`
Invalid value of '${frameworkName}' in the 'framework' field of Storybook config.
${automigrateMessage}
`);
throw new InvalidFrameworkNameError({ frameworkName });
}

// If we know about the framework, we don't need to validate it
Expand All @@ -42,9 +33,6 @@ export function validateFrameworkName(
try {
require.resolve(join(frameworkName, 'preset'));
} catch (err) {
throw new Error(dedent`
Could not evaluate the ${frameworkName} package from the 'framework' field of Storybook config.
Are you sure it's a valid package and is installed?`);
throw new CouldNotEvaluateFrameworkError({ frameworkName });
}
}
75 changes: 75 additions & 0 deletions code/lib/core-events/src/errors/server-errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,78 @@ export class NxProjectDetectedError extends StorybookError {
`;
}
}

export class MissingFrameworkFieldError extends StorybookError {
readonly category = Category.CORE_COMMON;

readonly code = 1;

public readonly documentation =
'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api';

template() {
return dedent`
Could not find a 'framework' field in Storybook config.
Please run 'npx storybook@next automigrate' to automatically fix your config.
`;
}
}

export class InvalidFrameworkNameError extends StorybookError {
readonly category = Category.CORE_COMMON;

readonly code = 2;

public readonly documentation =
'https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#new-framework-api';

constructor(public data: { frameworkName: string }) {
super();
}

template() {
return dedent`
Invalid value of '${this.data.frameworkName}' in the 'framework' field of Storybook config.
Please run 'npx storybook@next automigrate' to automatically fix your config.
`;
}
}

export class CouldNotEvaluateFrameworkError extends StorybookError {
readonly category = Category.CORE_COMMON;

readonly code = 3;

constructor(public data: { frameworkName: string }) {
super();
}

template() {
return dedent`
Could not evaluate the '${this.data.frameworkName}' package from the 'framework' field of Storybook config.
Are you sure it's a valid package and is installed?
`;
}
}

export class ConflictingStaticDirConfigError extends StorybookError {
readonly category = Category.CORE_SERVER;

readonly code = 1;

public readonly documentation =
'https://storybook.js.org/docs/react/configure/images-and-assets#serving-static-files-via-storybook-configuration';

template() {
return dedent`
Storybook encountered a conflict when trying to serve statics. You have configured both:
* Storybook's option in the config file: 'staticDirs'
* Storybook's (deprecated) CLI flag: '--staticDir' or '-s'
Please remove the CLI flag from your storybook script and use only the 'staticDirs' option instead.
`;
}
}
27 changes: 23 additions & 4 deletions code/lib/core-events/src/errors/storybook-error.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,35 @@ describe('StorybookError', () => {
const error = new TestError();
error.documentation = true;
const expectedMessage =
'This is a test error.\n\nMore info: https://storybook.js.org/error/SB_TEST_CATEGORY_0123';
'This is a test error.\n\nMore info: https://storybook.js.org/error/SB_TEST_CATEGORY_0123\n';
expect(error.message).toBe(expectedMessage);
});

it('should generate the correct message with external documentation link', () => {
const error = new TestError();
error.documentation = 'https://example.com/docs/test-error';
const expectedMessage =
'This is a test error.\n\nMore info: https://example.com/docs/test-error';
expect(error.message).toBe(expectedMessage);
expect(error.message).toMatchInlineSnapshot(`
"This is a test error.
More info: https://example.com/docs/test-error
"
`);
});

it('should generate the correct message with multiple external documentation links', () => {
const error = new TestError();
error.documentation = [
'https://example.com/docs/first-error',
'https://example.com/docs/second-error',
];
expect(error.message).toMatchInlineSnapshot(`
"This is a test error.
More info:
- https://example.com/docs/first-error
- https://example.com/docs/second-error
"
`);
});

it('should have default documentation value of false', () => {
Expand Down
6 changes: 4 additions & 2 deletions code/lib/core-events/src/errors/storybook-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export abstract class StorybookError extends Error {
* - If a string, uses the provided URL for documentation (external or FAQ links).
* - If `false` (default), no documentation link is added.
*/
public documentation: boolean | string = false;
public documentation: boolean | string | string[] = false;

/**
* Flag used to easily determine if the error originates from Storybook.
Expand All @@ -51,8 +51,10 @@ export abstract class StorybookError extends Error {
page = `https://storybook.js.org/error/${this.name}`;
} else if (typeof this.documentation === 'string') {
page = this.documentation;
} else if (Array.isArray(this.documentation)) {
page = `\n${this.documentation.map((doc) => `\t- ${doc}`).join('\n')}`;
}

return this.template() + (page != null ? `\n\nMore info: ${page}` : '');
return this.template() + (page != null ? `\n\nMore info: ${page}\n` : '');
}
}
11 changes: 2 additions & 9 deletions code/lib/core-server/src/build-static.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import chalk from 'chalk';
import { copy, emptyDir, ensureDir } from 'fs-extra';
import { dirname, isAbsolute, join, resolve } from 'path';
import { dedent } from 'ts-dedent';
import { global } from '@storybook/global';

import { logger } from '@storybook/node-logger';
import { telemetry, getPrecedingUpgrade } from '@storybook/telemetry';
import type {
Expand All @@ -22,6 +20,7 @@ import {
normalizeStories,
resolveAddonName,
} from '@storybook/core-common';
import { ConflictingStaticDirConfigError } from '@storybook/core-events/server-errors';

import isEqual from 'lodash/isEqual.js';
import { outputStats } from './utils/output-stats';
Expand Down Expand Up @@ -130,13 +129,7 @@ export async function buildStaticStandalone(options: BuildStaticStandaloneOption
};

if (options.staticDir && !isEqual(staticDirs, defaultStaticDirs)) {
throw new Error(dedent`
Conflict when trying to read staticDirs:
* Storybook's configuration option: 'staticDirs'
* Storybook's CLI flag: '--staticDir' or '-s'
Choose one of them, but not both.
`);
throw new ConflictingStaticDirConfigError();
}

const effects: Promise<void>[] = [];
Expand Down
9 changes: 2 additions & 7 deletions code/lib/core-server/src/utils/server-statics.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { logger } from '@storybook/node-logger';
import type { Options, StorybookConfig } from '@storybook/types';
import { getDirectoryFromWorkingDir } from '@storybook/core-common';
import { ConflictingStaticDirConfigError } from '@storybook/core-events/server-errors';
import chalk from 'chalk';
import express from 'express';
import { pathExists } from 'fs-extra';
Expand All @@ -17,13 +18,7 @@ export async function useStatics(router: any, options: Options) {
const faviconPath = await options.presets.apply<string>('favicon');

if (options.staticDir && !isEqual(staticDirs, defaultStaticDirs)) {
throw new Error(dedent`
Conflict when trying to read staticDirs:
* Storybook's configuration option: 'staticDirs'
* Storybook's CLI flag: '--staticDir' or '-s'
Choose one of them, but not both.
`);
throw new ConflictingStaticDirConfigError();
}

const statics = [
Expand Down

0 comments on commit a85f652

Please sign in to comment.