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

Core: Add error categorization framework #23653

Merged
merged 28 commits into from
Aug 21, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 14 additions & 0 deletions code/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ module.exports = {
tsconfigRootDir: __dirname,
project: ['./tsconfig.json'],
},
plugins: ['local-rules'],
rules: {
'eslint-comments/disable-enable-pair': ['error', { allowWholeFile: true }],
'eslint-comments/no-unused-disable': 'error',
Expand Down Expand Up @@ -166,5 +167,18 @@ module.exports = {
'import/no-unresolved': 'off',
},
},
{
files: ['**/*.ts', '!**/*.test.*', '!**/*.spec.*'],
rules: {
'local-rules/no-uncategorized-errors': 'warn',
},
},
{
files: ['**/core-events/src/**/*'],
excludedFiles: ['**/*.test.*'],
rules: {
'local-rules/no-duplicated-error-codes': 'error',
},
},
],
};
2 changes: 1 addition & 1 deletion code/addons/actions/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@
"polished": "^4.2.2",
"prop-types": "^15.7.2",
"react-inspector": "^6.0.0",
"telejson": "^7.0.3",
"telejson": "^7.2.0",
"ts-dedent": "^2.0.0",
"uuid": "^9.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion code/frameworks/angular/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@
"find-up": "^5.0.0",
"read-pkg-up": "^7.0.1",
"semver": "^7.3.7",
"telejson": "^7.0.3",
"telejson": "^7.2.0",
"ts-dedent": "^2.0.0",
"tsconfig-paths-webpack-plugin": "^4.0.1",
"util-deprecate": "^1.0.2",
Expand Down
2 changes: 1 addition & 1 deletion code/lib/channels/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
"@storybook/core-events": "workspace:*",
"@storybook/global": "^5.0.0",
"qs": "^6.10.0",
"telejson": "^7.0.3",
"telejson": "^7.2.0",
"tiny-invariant": "^1.3.1"
},
"devDependencies": {
Expand Down
2 changes: 2 additions & 0 deletions code/lib/channels/src/postmessage/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export class PostMessageTransport implements ChannelTransport {
allowFunction,
allowSymbol,
allowDate,
allowError,
allowUndefined,
allowClass,
maxDepth,
Expand All @@ -85,6 +86,7 @@ export class PostMessageTransport implements ChannelTransport {
allowFunction,
allowSymbol,
allowDate,
allowError,
allowUndefined,
allowClass,
maxDepth,
Expand Down
7 changes: 2 additions & 5 deletions code/lib/cli/src/initiate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import chalk from 'chalk';
import prompts from 'prompts';
import { telemetry } from '@storybook/telemetry';
import { withTelemetry } from '@storybook/core-server';
import { NxProjectDetectedError } from '@storybook/core-events/server-errors';

import dedent from 'ts-dedent';
import boxen from 'boxen';
Expand Down Expand Up @@ -156,11 +157,7 @@ const installStorybook = async <Project extends ProjectType>(
);

case ProjectType.NX:
throw new Error(dedent`
We have detected Nx in your project. Please use "nx g @nrwl/storybook:configuration" to add Storybook to your project.

For more information, please see https://nx.dev/packages/storybook
`);
throw new NxProjectDetectedError();

case ProjectType.SOLID:
return solidGenerator(packageManager, npmOptions, generatorOptions).then(
Expand Down
42 changes: 41 additions & 1 deletion code/lib/core-events/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,45 @@
"require": "./dist/index.js",
"import": "./dist/index.mjs"
},
"./preview-errors": {
"types": "./dist/errors/preview-errors.d.ts",
"node": "./dist/errors/preview-errors.js",
"require": "./dist/errors/preview-errors.js",
"import": "./dist/errors/preview-errors.mjs"
},
"./manager-errors": {
"types": "./dist/errors/manager-errors.d.ts",
"node": "./dist/errors/manager-errors.js",
"require": "./dist/errors/manager-errors.js",
"import": "./dist/errors/manager-errors.mjs"
},
"./server-errors": {
"types": "./dist/errors/server-errors.d.ts",
"node": "./dist/errors/server-errors.js",
"require": "./dist/errors/server-errors.js",
"import": "./dist/errors/server-errors.mjs"
},
"./package.json": "./package.json"
},
"main": "./dist/index.js",
"module": "./dist/index.mjs",
"types": "./dist/index.d.ts",
"typesVersions": {
"*": {
"*": [
"dist/index.d.ts"
],
"preview-errors": [
"dist/errors/preview-errors.d.ts"
],
"manager-errors": [
"dist/errors/manager-errors.d.ts"
],
"server-errors": [
"dist/errors/server-errors.d.ts"
]
}
},
"files": [
"dist/**/*",
"README.md",
Expand All @@ -43,6 +77,9 @@
"check": "../../../scripts/prepare/check.ts",
"prep": "../../../scripts/prepare/bundle.ts"
},
"dependencies": {
"ts-dedent": "^2.0.0"
},
"devDependencies": {
"typescript": "~4.9.3"
},
Expand All @@ -51,7 +88,10 @@
},
"bundler": {
"entries": [
"./src/index.ts"
"./src/index.ts",
"./src/errors/preview-errors.ts",
"./src/errors/manager-errors.ts",
"./src/errors/server-errors.ts"
]
},
"gitHead": "e6a7fd8a655c69780bc20b9749c2699e44beae17"
Expand Down
160 changes: 160 additions & 0 deletions code/lib/core-events/src/errors/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
# Storybook Errors

Storybook provides a utility to manage errors thrown from it. Each error is categorized and coded, and there is an ESLint plugin which enforces their usage, instead of throwing generic errors like `throw new Error()`.

Storybook errors reside in this package and are categorized into:

1. **[Preview errors](./preview-errors.ts)**
- Errors which occur in the preview part of Storybook
- e.g. Rendering issues, etc.
- available in `@storybook/core-events/preview-errors`
2. **[Manager errors](./manager-errors.ts)**
- Errors which occur
yannbf marked this conversation as resolved.
Show resolved Hide resolved
- e.g. Sidebar, addons, Storybook UI, Storybook router, etc.
- available in `@storybook/core-events/server-errors`
3. **[Server errors](./server-errors.ts)**
- Errorr which occur in node
yannbf marked this conversation as resolved.
Show resolved Hide resolved
- e.g. Storybook init command, dev command, builder errors (Webpack, Vite), etc.
- available in `@storybook/core-events/server-errors`

## How to create errors

First, **find which file your error should be part of**, based on the criteria above.
Second use the `StorybookError` class to define custom errors with specific codes and categories for use within the Storybook codebase. Below is a detailed documentation for the error properties:

### Class Structure

```typescript
import { StorybookError } from './storybook-error';
export class YourCustomError extends StorybookError {
readonly category: Category; // The category to which the error belongs. Check the source in client-errors.ts or server-errors.ts for reference.
readonly code: number; // The numeric code for the error.
readonly telemetry?: boolean; // Optional. If set to `true`, telemetry will be used to send errors. Only for client-based errors.

template(): string {
// A function that returns the error message.
}
}
```

### Properties

| Name | Type | Description |
| ------------- | --------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------- |
| category | `Category` | The category to which the error belongs. |
| code | `number` | The numeric code for the error. |
| template | `() => string` | Function that returns a properly written error message. |
| data | `Object` | Optional. Data associated with the error. Used to provide additional information in the error message or to be passed to telemetry. |
| documentation | `boolean` or `string` | Optional. Should be set to `true` **if the error is documented on the Storybook website**. If defined as string, it should be a custom documentation link. |
| telemetry | `boolean` | Optional. If set to `true`, telemetry will be used to send errors. |

## Usage Example

```typescript
// Define a custom error with a numeric code and a static error message template.
export class StorybookIndexGenerationError extends StorybookError {
category = Category.Generic;
code = 1;
telemetry = true;

template(): string {
return `Storybook failed when generating an index for your stories. Check the stories field in your main.js`;
}
}

// Define a custom error with a numeric code and a dynamic error message template based on properties from the constructor.
export class InvalidFileExtensionError extends StorybookError {
category = Category.Validation;
code = 2;
telemetry = true;
documentation = 'https://some-custom-documentation.com/validation-errors';

// extra properties are defined in the constructor via a data property, which is available in any class method
// always use this data Object notation!
constructor(public data: { extension: string }) {
super();
}

template(): string {
return `Invalid file extension found: ${this.data.extension}.`;
}
}

// import the errors where you need them, i.e.
import {
StorybookIndexGenerationError,
InvalidFileExtensionError,
} from '@storybook/core-events/server-errors';

throw StorybookIndexGenerationError();
// "SB_Generic_0001: Storybook failed when generating an index for your stories. Check the stories field in your main.js.

throw InvalidFileExtensionError({ extension: 'mtsx' });
// "SB_Validation_0002: Invalid file extension found: mtsx. More info: https://some-custom-documentation.com/validation-errors"
```

## How to write a proper error message
yannbf marked this conversation as resolved.
Show resolved Hide resolved

Writing clear and informative error messages is crucial for effective debugging and troubleshooting. A well-crafted error message can save developers and users valuable time. Consider the following guidelines:

- **Be clear and specific:** Provide straightforward error messages that precisely describe the issue.
- **Include relevant context:** Add details about the error's origin and relevant context to aid troubleshooting.
yannbf marked this conversation as resolved.
Show resolved Hide resolved
- **Provide guidance for resolution:** Offer actionable steps to resolve the error or suggest potential fixes.
- **Provide documentation links:** Whenever applicable, provide links for users to get guidance or more context to fix their issues.

<img src="./message-reference.png" width="800px" />

✅ Here are a few recommended examples:

Long:

```
Couldn't find story matching id 'component--button-primary' after HMR.
- Did you just rename a story?
- Did you remove it from your CSF file?
- Are you sure a story with the id 'component--button-primary' exists?
- Please check the values in the stories field of your main.js config and see if they would match your CSF File.
- Also check the browser console and terminal for potential error messages.
```

Medium:

```
Addon-docs no longer uses configureJsx or mdxBabelOptions in 7.0.

To update your configuration, please see migration instructions here:

https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#dropped-addon-docs-manual-babel-configuration
```

Short:

```
Failed to start Storybook.

Do you have an error in your \`preview.js\`? Check your Storybook's browser console for errors.
```

❌ Here are a few unrecommended examples:
yannbf marked this conversation as resolved.
Show resolved Hide resolved

```
outputDir is required
```

```
Cannot render story
```

```
no builder configured!
```

## What's the motivation for this errors framework?

Centralizing and categorizing errors offers several advantages:

Better understanding of what is actually failing: By defining categories, error origins become more evident, easing the debugging process for developers and providing users with actionable insights.

Improved Telemetry: Aggregating and filtering errors allows better assessment of their impact, which helps in prioritization and tackling the issues.

Improved Documentation: Categorized errors lead to the creation of a helpful errors page on the Storybook website, benefiting users with better guidance and improving overall accessibility and user experience.
32 changes: 32 additions & 0 deletions code/lib/core-events/src/errors/manager-errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { StorybookError } from './storybook-error';

/**
* If you can't find a suitable category for your error, create one
* based on the package name/file path of which the error is thrown.
* For instance:
* If it's from @storybook/client-logger, then MANAGER_CLIENT-LOGGER
*
* Categories are prefixed by a logical grouping, e.g. MANAGER_
* to prevent manager and preview errors from having the same category and error code.
*/
export enum Category {
MANAGER_UI = 'MANAGER_UI',
MANAGER_API = 'MANAGER_API',
MANAGER_CLIENT_LOGGER = 'MANAGER_CLIENT-LOGGER',
MANAGER_CHANNELS = 'MANAGER_CHANNELS',
MANAGER_CORE_EVENTS = 'MANAGER_CORE-EVENTS',
MANAGER_ROUTER = 'MANAGER_ROUTER',
MANAGER_THEMING = 'MANAGER_THEMING',
}

export class ProviderDoesNotExtendBaseProviderError extends StorybookError {
readonly category = Category.MANAGER_UI;

readonly code = 1;

readonly telemetry = true;

template() {
return `The Provider passed into Storybook's UI is not extended from the base Provider. Please check your Provider implementation.`;
}
}
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.