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

Maintenance: Categorize server errors #23912

Merged
merged 2 commits into from
Aug 23, 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
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;
yannbf marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -125,13 +124,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