diff --git a/code/.eslintrc.js b/code/.eslintrc.js index c764d4b28d83..e967113bb8eb 100644 --- a/code/.eslintrc.js +++ b/code/.eslintrc.js @@ -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', @@ -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', + }, + }, ], }; diff --git a/code/addons/actions/package.json b/code/addons/actions/package.json index fb921db912e5..bff07d8c6329 100644 --- a/code/addons/actions/package.json +++ b/code/addons/actions/package.json @@ -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" }, diff --git a/code/frameworks/angular/package.json b/code/frameworks/angular/package.json index 9d2d2944f043..575f53bdb903 100644 --- a/code/frameworks/angular/package.json +++ b/code/frameworks/angular/package.json @@ -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", diff --git a/code/lib/channels/package.json b/code/lib/channels/package.json index d1f02d7e0093..6b0ffae6aba5 100644 --- a/code/lib/channels/package.json +++ b/code/lib/channels/package.json @@ -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": { diff --git a/code/lib/channels/src/postmessage/index.ts b/code/lib/channels/src/postmessage/index.ts index 2de550dbce38..312a8d756295 100644 --- a/code/lib/channels/src/postmessage/index.ts +++ b/code/lib/channels/src/postmessage/index.ts @@ -72,6 +72,7 @@ export class PostMessageTransport implements ChannelTransport { allowFunction, allowSymbol, allowDate, + allowError, allowUndefined, allowClass, maxDepth, @@ -85,6 +86,7 @@ export class PostMessageTransport implements ChannelTransport { allowFunction, allowSymbol, allowDate, + allowError, allowUndefined, allowClass, maxDepth, diff --git a/code/lib/cli/src/initiate.ts b/code/lib/cli/src/initiate.ts index 0b2a75a4a488..5f84963bfc05 100644 --- a/code/lib/cli/src/initiate.ts +++ b/code/lib/cli/src/initiate.ts @@ -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'; @@ -156,11 +157,7 @@ const installStorybook = async ( ); 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( diff --git a/code/lib/core-events/package.json b/code/lib/core-events/package.json index 711937e16a78..b98966eda27a 100644 --- a/code/lib/core-events/package.json +++ b/code/lib/core-events/package.json @@ -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", @@ -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" }, @@ -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" diff --git a/code/lib/core-events/src/errors/README.md b/code/lib/core-events/src/errors/README.md new file mode 100644 index 000000000000..291fb5abe779 --- /dev/null +++ b/code/lib/core-events/src/errors/README.md @@ -0,0 +1,156 @@ +# 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 (where user code executes) + - e.g. Rendering issues, etc. + - available in `@storybook/core-events/preview-errors` +2. **[Manager errors](./manager-errors.ts)** + - Errors which occur in the manager part of Storybook (manager UI) + - e.g. Sidebar, addons, Storybook UI, Storybook router, etc. + - available in `@storybook/core-events/server-errors` +3. **[Server errors](./server-errors.ts)** + - Errors which occur in node + - 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. + + 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. | + +## 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; + + 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; + 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 + +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. +- **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. + + + +✅ 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: + +``` +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. diff --git a/code/lib/core-events/src/errors/manager-errors.ts b/code/lib/core-events/src/errors/manager-errors.ts new file mode 100644 index 000000000000..a97c5f8d7035 --- /dev/null +++ b/code/lib/core-events/src/errors/manager-errors.ts @@ -0,0 +1,46 @@ +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_UNCAUGHT = 'MANAGER_UNCAUGHT', + 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; + + template() { + return `The Provider passed into Storybook's UI is not extended from the base Provider. Please check your Provider implementation.`; + } +} + +export class UncaughtManagerError extends StorybookError { + readonly category = Category.MANAGER_UNCAUGHT; + + readonly code = 1; + + constructor(public error: Error) { + super(error.message); + this.stack = error.stack; + } + + template() { + return this.message; + } +} diff --git a/code/lib/core-events/src/errors/message-reference.png b/code/lib/core-events/src/errors/message-reference.png new file mode 100644 index 000000000000..b829f93689ea Binary files /dev/null and b/code/lib/core-events/src/errors/message-reference.png differ diff --git a/code/lib/core-events/src/errors/preview-errors.ts b/code/lib/core-events/src/errors/preview-errors.ts new file mode 100644 index 000000000000..3a2fe93aecf5 --- /dev/null +++ b/code/lib/core-events/src/errors/preview-errors.ts @@ -0,0 +1,69 @@ +import dedent from 'ts-dedent'; +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 CLIENT-LOGGER + * + * Categories are prefixed by a logical grouping, e.g. PREVIEW_ or FRAMEWORK_ + * to prevent manager and preview errors from having the same category and error code. + */ +export enum Category { + PREVIEW_CLIENT_LOGGER = 'PREVIEW_CLIENT-LOGGER', + PREVIEW_CHANNELS = 'PREVIEW_CHANNELS', + PREVIEW_CORE_EVENTS = 'PREVIEW_CORE-EVENTS', + PREVIEW_INSTRUMENTER = 'PREVIEW_INSTRUMENTER', + PREVIEW_API = 'PREVIEW_API', + PREVIEW_REACT_DOM_SHIM = 'PREVIEW_REACT-DOM-SHIM', + PREVIEW_ROUTER = 'PREVIEW_ROUTER', + PREVIEW_THEMING = 'PREVIEW_THEMING', + FRAMEWORK_ANGULAR = 'FRAMEWORK_ANGULAR', + FRAMEWORK_EMBER = 'FRAMEWORK_EMBER', + FRAMEWORK_HTML_VITE = 'FRAMEWORK_HTML-VITE', + FRAMEWORK_HTML_WEBPACK5 = 'FRAMEWORK_HTML-WEBPACK5', + FRAMEWORK_NEXTJS = 'FRAMEWORK_NEXTJS', + FRAMEWORK_PREACT_VITE = 'FRAMEWORK_PREACT-VITE', + FRAMEWORK_PREACT_WEBPACK5 = 'FRAMEWORK_PREACT-WEBPACK5', + FRAMEWORK_REACT_VITE = 'FRAMEWORK_REACT-VITE', + FRAMEWORK_REACT_WEBPACK5 = 'FRAMEWORK_REACT-WEBPACK5', + FRAMEWORK_SERVER_WEBPACK5 = 'FRAMEWORK_SERVER-WEBPACK5', + FRAMEWORK_SVELTE_VITE = 'FRAMEWORK_SVELTE-VITE', + FRAMEWORK_SVELTE_WEBPACK5 = 'FRAMEWORK_SVELTE-WEBPACK5', + FRAMEWORK_SVELTEKIT = 'FRAMEWORK_SVELTEKIT', + FRAMEWORK_VUE_VITE = 'FRAMEWORK_VUE-VITE', + FRAMEWORK_VUE_WEBPACK5 = 'FRAMEWORK_VUE-WEBPACK5', + FRAMEWORK_VUE3_VITE = 'FRAMEWORK_VUE3-VITE', + FRAMEWORK_VUE3_WEBPACK5 = 'FRAMEWORK_VUE3-WEBPACK5', + FRAMEWORK_WEB_COMPONENTS_VITE = 'FRAMEWORK_WEB-COMPONENTS-VITE', + FRAMEWORK_WEB_COMPONENTS_WEBPACK5 = 'FRAMEWORK_WEB-COMPONENTS-WEBPACK5', + RENDERER_HTML = 'RENDERER_HTML', + RENDERER_PREACT = 'RENDERER_PREACT', + RENDERER_REACT = 'RENDERER_REACT', + RENDERER_SERVER = 'RENDERER_SERVER', + RENDERER_SVELTE = 'RENDERER_SVELTE', + RENDERER_VUE = 'RENDERER_VUE', + RENDERER_VUE3 = 'RENDERER_VUE3', + RENDERER_WEB_COMPONENTS = 'RENDERER_WEB-COMPONENTS', +} + +export class MissingStoryAfterHmrError extends StorybookError { + readonly category = Category.PREVIEW_API; + + readonly code = 1; + + constructor(public data: { storyId: string }) { + super(); + } + + template() { + return dedent` + Couldn't find story matching id '${this.data.storyId}' after HMR. + - Did you just rename a story? + - Did you remove it from your CSF file? + - Are you sure a story with the id '${this.data.storyId}' 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.`; + } +} diff --git a/code/lib/core-events/src/errors/server-errors.ts b/code/lib/core-events/src/errors/server-errors.ts new file mode 100644 index 000000000000..695b4cb09306 --- /dev/null +++ b/code/lib/core-events/src/errors/server-errors.ts @@ -0,0 +1,46 @@ +import dedent from 'ts-dedent'; +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/node-logger, then NODE-LOGGER + * If it's from a package that is too broad, e.g. @storybook/cli in the init command, then use a combination like CLI_INIT + */ +export enum Category { + CLI = 'CLI', + CLI_INIT = 'CLI_INIT', + CLI_AUTOMIGRATE = 'CLI_AUTOMIGRATE', + CLI_UPGRADE = 'CLI_UPGRADE', + CLI_ADD = 'CLI_ADD', + CODEMOD = 'CODEMOD', + CORE_SERVER = 'CORE-SERVER', + CSF_PLUGIN = 'CSF-PLUGIN', + CSF_TOOLS = 'CSF-TOOLS', + CORE_COMMON = 'CORE-COMMON', + NODE_LOGGER = 'NODE-LOGGER', + TELEMETRY = 'TELEMETRY', + BUILDER_MANAGER = 'BUILDER-MANAGER', + BUILDER_VITE = 'BUILDER-VITE', + BUILDER_WEBPACK5 = 'BUILDER-WEBPACK5', + SOURCE_LOADER = 'SOURCE-LOADER', + POSTINSTALL = 'POSTINSTALL', + DOCS_TOOLS = 'DOCS-TOOLS', + CORE_WEBPACK = 'CORE-WEBPACK', +} + +export class NxProjectDetectedError extends StorybookError { + readonly category = Category.CLI_INIT; + + readonly code = 1; + + public readonly documentation = 'https://nx.dev/packages/storybook'; + + template() { + return dedent` + We have detected Nx in your project. Nx has its own Storybook initializer, so please use it instead. + Run "nx g @nx/storybook:configuration" to add Storybook to your project. + `; + } +} diff --git a/code/lib/core-events/src/errors/storybook-error.test.ts b/code/lib/core-events/src/errors/storybook-error.test.ts new file mode 100644 index 000000000000..328c27a827e4 --- /dev/null +++ b/code/lib/core-events/src/errors/storybook-error.test.ts @@ -0,0 +1,45 @@ +import { StorybookError } from './storybook-error'; + +describe('StorybookError', () => { + class TestError extends StorybookError { + category = 'TEST_CATEGORY'; + + code = 123; + + template() { + return 'This is a test error.'; + } + } + + it('should generate the correct error name', () => { + const error = new TestError(); + expect(error.name).toBe('SB_TEST_CATEGORY_0123'); + }); + + it('should generate the correct message without documentation link', () => { + const error = new TestError(); + const expectedMessage = 'This is a test error.'; + expect(error.message).toBe(expectedMessage); + }); + + it('should generate the correct message with internal documentation link', () => { + 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'; + 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); + }); + + it('should have default documentation value of false', () => { + const error = new TestError(); + expect(error.documentation).toBe(false); + }); +}); diff --git a/code/lib/core-events/src/errors/storybook-error.ts b/code/lib/core-events/src/errors/storybook-error.ts new file mode 100644 index 000000000000..3a0abbf463f5 --- /dev/null +++ b/code/lib/core-events/src/errors/storybook-error.ts @@ -0,0 +1,58 @@ +export abstract class StorybookError extends Error { + /** + * Category of the error. Used to classify the type of error, e.g., 'PREVIEW_API'. + */ + abstract readonly category: string; + + /** + * Code representing the error. Used to uniquely identify the error, e.g., 1. + */ + abstract readonly code: number; + + /** + * A properly written error message template for this error. + * @see https://github.com/storybookjs/storybook/blob/next/code/lib/core-events/src/errors/README.md#how-to-write-a-proper-error-message + */ + abstract template(): string; + + /** + * Data associated with the error. Used to provide additional information in the error message or to be passed to telemetry. + */ + public readonly data = {}; + + /** + * Specifies the documentation for the error. + * - If `true`, links to a documentation page on the Storybook website (make sure it exists before enabling). + * - 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; + + /** + * Flag used to easily determine if the error originates from Storybook. + */ + readonly fromStorybook: true = true as const; + + /** + * Overrides the default `Error.name` property in the format: SB__. + */ + get name() { + const paddedCode = String(this.code).padStart(4, '0'); + return `SB_${this.category}_${paddedCode}` as `SB_${this['category']}_${string}`; + } + + /** + * Generates the error message along with additional documentation link (if applicable). + */ + get message() { + let page: string | undefined; + + if (this.documentation === true) { + page = `https://storybook.js.org/error/${this.name}`; + } else if (typeof this.documentation === 'string') { + page = this.documentation; + } + + return this.template() + (page != null ? `\n\nMore info: ${page}` : ''); + } +} diff --git a/code/lib/core-events/src/index.ts b/code/lib/core-events/src/index.ts index beec768961de..d8d6c41c01d3 100644 --- a/code/lib/core-events/src/index.ts +++ b/code/lib/core-events/src/index.ts @@ -69,6 +69,7 @@ enum events { RESULT_WHATS_NEW_DATA = 'resultWhatsNewData', SET_WHATS_NEW_CACHE = 'setWhatsNewCache', TOGGLE_WHATS_NEW_NOTIFICATIONS = 'toggleWhatsNewNotifications', + TELEMETRY_ERROR = 'telemetryError', } // Enables: `import Events from ...` @@ -120,9 +121,11 @@ export const { RESULT_WHATS_NEW_DATA, SET_WHATS_NEW_CACHE, TOGGLE_WHATS_NEW_NOTIFICATIONS, + TELEMETRY_ERROR, } = events; // Used to break out of the current render without showing a redbox +// eslint-disable-next-line local-rules/no-uncategorized-errors export const IGNORED_EXCEPTION = new Error('ignoredException'); export interface WhatsNewCache { diff --git a/code/lib/core-server/package.json b/code/lib/core-server/package.json index eaaf76ab05e9..305d010bb28f 100644 --- a/code/lib/core-server/package.json +++ b/code/lib/core-server/package.json @@ -90,7 +90,7 @@ "read-pkg-up": "^7.0.1", "semver": "^7.3.7", "serve-favicon": "^2.5.0", - "telejson": "^7.0.3", + "telejson": "^7.2.0", "tiny-invariant": "^1.3.1", "ts-dedent": "^2.0.0", "util": "^0.12.4", diff --git a/code/lib/core-server/src/presets/common-preset.ts b/code/lib/core-server/src/presets/common-preset.ts index 7e13ade9ddf0..36816316ccbf 100644 --- a/code/lib/core-server/src/presets/common-preset.ts +++ b/code/lib/core-server/src/presets/common-preset.ts @@ -25,6 +25,7 @@ import type { WhatsNewCache, WhatsNewData } from '@storybook/core-events'; import { REQUEST_WHATS_NEW_DATA, RESULT_WHATS_NEW_DATA, + TELEMETRY_ERROR, SET_WHATS_NEW_CACHE, TOGGLE_WHATS_NEW_NOTIFICATIONS, } from '@storybook/core-events'; @@ -329,5 +330,17 @@ export const experimental_serverChannel = async ( } ); + channel.on(TELEMETRY_ERROR, async (error) => { + const isTelemetryEnabled = coreOptions.disableTelemetry !== true; + + if (isTelemetryEnabled) { + await sendTelemetryError(error, 'browser', { + cliOptions: options, + presetOptions: { ...options, corePresets: [], overridePresets: [] }, + skipPrompt: true, + }); + } + }); + return channel; }; diff --git a/code/lib/core-server/src/withTelemetry.ts b/code/lib/core-server/src/withTelemetry.ts index 706439b4bfd7..49b8c6c79699 100644 --- a/code/lib/core-server/src/withTelemetry.ts +++ b/code/lib/core-server/src/withTelemetry.ts @@ -85,6 +85,25 @@ export async function sendTelemetryError( error instanceof Error, 'The error passed to sendTelemetryError was not an Error, please only send Errors' ); + + let storybookErrorProperties = {}; + // if it's an UNCATEGORIZED error, it won't have a coded name, so we just pass the category and source + if ((error as any).category) { + const { category } = error as any; + storybookErrorProperties = { + category, + }; + } + + if ((error as any).fromStorybook) { + const { code, name } = error as any; + storybookErrorProperties = { + ...storybookErrorProperties, + code, + name, + }; + } + await telemetry( 'error', { @@ -92,6 +111,7 @@ export async function sendTelemetryError( precedingUpgrade, error: errorLevel === 'full' ? error : undefined, errorHash: oneWayHash(error.message), + ...storybookErrorProperties, }, { immediate: true, diff --git a/code/lib/manager-api/package.json b/code/lib/manager-api/package.json index 72a4ef70fe38..95ac20b8a0bb 100644 --- a/code/lib/manager-api/package.json +++ b/code/lib/manager-api/package.json @@ -56,7 +56,7 @@ "memoizerific": "^1.11.3", "semver": "^7.3.7", "store2": "^2.14.2", - "telejson": "^7.0.3", + "telejson": "^7.2.0", "ts-dedent": "^2.0.0" }, "devDependencies": { diff --git a/code/lib/preview-api/src/modules/store/StoryIndexStore.test.ts b/code/lib/preview-api/src/modules/store/StoryIndexStore.test.ts index a898ecd1b06c..c906d7a21a75 100644 --- a/code/lib/preview-api/src/modules/store/StoryIndexStore.test.ts +++ b/code/lib/preview-api/src/modules/store/StoryIndexStore.test.ts @@ -154,7 +154,7 @@ describe('StoryIndexStore', () => { const store = new StoryIndexStore(storyIndex); expect(() => store.storyIdToEntry('random')).toThrow( - /Couldn't find story matching 'random'/ + /Couldn't find story matching id 'random'/ ); }); }); diff --git a/code/lib/preview-api/src/modules/store/StoryIndexStore.ts b/code/lib/preview-api/src/modules/store/StoryIndexStore.ts index 8731f3d495e1..03e8a129b9bf 100644 --- a/code/lib/preview-api/src/modules/store/StoryIndexStore.ts +++ b/code/lib/preview-api/src/modules/store/StoryIndexStore.ts @@ -1,4 +1,3 @@ -import { dedent } from 'ts-dedent'; import type { IndexEntry, Path, @@ -8,6 +7,7 @@ import type { ComponentTitle, } from '@storybook/types'; import memoize from 'memoizerific'; +import { MissingStoryAfterHmrError } from '@storybook/core-events/preview-errors'; export type StorySpecifier = StoryId | { name: StoryName; title: ComponentTitle } | '*'; @@ -49,11 +49,7 @@ export class StoryIndexStore { storyIdToEntry(storyId: StoryId): IndexEntry { const storyEntry = this.entries[storyId]; if (!storyEntry) { - throw new Error(dedent`Couldn't find story matching '${storyId}' after HMR. - - Did you remove it from your CSF file? - - Are you sure a story with that id exists? - - Please check your entries field of your main.js config. - - Also check the browser console and terminal for error messages.`); + throw new MissingStoryAfterHmrError({ storyId }); } return storyEntry; diff --git a/code/lib/preview-api/src/typings.d.ts b/code/lib/preview-api/src/typings.d.ts index fb9194834b96..bedefed4b9a8 100644 --- a/code/lib/preview-api/src/typings.d.ts +++ b/code/lib/preview-api/src/typings.d.ts @@ -24,6 +24,7 @@ declare var __STORYBOOK_PREVIEW__: import('./modules/preview-web/PreviewWeb').Pr declare var __STORYBOOK_STORY_STORE__: any; declare var STORYBOOK_HOOKS_CONTEXT: any; declare var LOGLEVEL: 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'silent' | undefined; +declare var sendTelemetryError: (error: any) => void; declare module 'ansi-to-html'; declare class AnsiToHtml { diff --git a/code/lib/preview/package.json b/code/lib/preview/package.json index 700252c07181..7d386ea80aae 100644 --- a/code/lib/preview/package.json +++ b/code/lib/preview/package.json @@ -59,6 +59,7 @@ "@storybook/channels": "workspace:*", "@storybook/client-logger": "workspace:*", "@storybook/core-events": "workspace:*", + "@storybook/global": "^5.0.0", "@storybook/preview-api": "workspace:*", "typescript": "~4.9.3" }, diff --git a/code/lib/preview/src/runtime.ts b/code/lib/preview/src/runtime.ts index 5e890a133a55..7785e42df8c9 100644 --- a/code/lib/preview/src/runtime.ts +++ b/code/lib/preview/src/runtime.ts @@ -1,3 +1,6 @@ +import { TELEMETRY_ERROR } from '@storybook/core-events'; +import { global } from '@storybook/global'; + import { values } from './globals/runtime'; import { globals } from './globals/types'; @@ -5,5 +8,23 @@ const getKeys = Object.keys as (obj: T) => Array; // Apply all the globals getKeys(globals).forEach((key) => { - (globalThis as any)[globals[key]] = values[key]; + (global as any)[globals[key]] = values[key]; +}); + +global.sendTelemetryError = (error: any) => { + const channel = global.__STORYBOOK_ADDONS_CHANNEL__; + channel.emit(TELEMETRY_ERROR, error); +}; + +// handle all uncaught StorybookError at the root of the application and log to telemetry if applicable +global.addEventListener('error', (args: any) => { + const error = args.error || args; + if (error.fromStorybook) { + global.sendTelemetryError(error); + } +}); +global.addEventListener('unhandledrejection', ({ reason }: any) => { + if (reason.fromStorybook) { + global.sendTelemetryError(reason); + } }); diff --git a/code/lib/preview/src/typings.d.ts b/code/lib/preview/src/typings.d.ts index bfd9e55123ff..a816c261fa72 100644 --- a/code/lib/preview/src/typings.d.ts +++ b/code/lib/preview/src/typings.d.ts @@ -1 +1,5 @@ +/* eslint-disable @typescript-eslint/naming-convention */ declare var LOGLEVEL: 'trace' | 'debug' | 'info' | 'warn' | 'error' | 'silent' | undefined; + +declare var __STORYBOOK_ADDONS_CHANNEL__: any; +declare var sendTelemetryError: (error: any) => void; diff --git a/code/lib/telemetry/src/types.ts b/code/lib/telemetry/src/types.ts index ba3af37719c0..35266814dff7 100644 --- a/code/lib/telemetry/src/types.ts +++ b/code/lib/telemetry/src/types.ts @@ -9,6 +9,7 @@ export type EventType = | 'build' | 'upgrade' | 'init' + | 'browser' | 'canceled' | 'error' | 'error-metadata' diff --git a/code/package.json b/code/package.json index fea83a390a7d..3ae4a217a371 100644 --- a/code/package.json +++ b/code/package.json @@ -225,6 +225,7 @@ "eslint": "^8.28.0", "eslint-import-resolver-typescript": "^3.5.2", "eslint-plugin-import": "^2.26.0", + "eslint-plugin-local-rules": "portal:../scripts/eslint-plugin-local-rules", "eslint-plugin-react": "^7.31.10", "eslint-plugin-storybook": "^0.6.6", "fs-extra": "^11.1.0", diff --git a/code/ui/blocks/package.json b/code/ui/blocks/package.json index 9e9ffa3f697c..e077d15ec27c 100644 --- a/code/ui/blocks/package.json +++ b/code/ui/blocks/package.json @@ -63,7 +63,7 @@ "memoizerific": "^1.11.3", "polished": "^4.2.2", "react-colorful": "^5.1.2", - "telejson": "^7.0.3", + "telejson": "^7.2.0", "tocbot": "^4.20.1", "ts-dedent": "^2.0.0", "util-deprecate": "^1.0.2" diff --git a/code/ui/manager/src/globals/exports.ts b/code/ui/manager/src/globals/exports.ts index 793165aaf103..8eb900860c6e 100644 --- a/code/ui/manager/src/globals/exports.ts +++ b/code/ui/manager/src/globals/exports.ts @@ -172,6 +172,7 @@ export default { 'STORY_SPECIFIED', 'STORY_THREW_EXCEPTION', 'STORY_UNCHANGED', + 'TELEMETRY_ERROR', 'TOGGLE_WHATS_NEW_NOTIFICATIONS', 'UPDATE_GLOBALS', 'UPDATE_QUERY_PARAMS', diff --git a/code/ui/manager/src/index.tsx b/code/ui/manager/src/index.tsx index 8bf921c005da..2836846be5b1 100644 --- a/code/ui/manager/src/index.tsx +++ b/code/ui/manager/src/index.tsx @@ -7,6 +7,7 @@ import { Location, LocationProvider, useNavigate } from '@storybook/router'; import { Provider as ManagerProvider, types } from '@storybook/manager-api'; import type { Combo } from '@storybook/manager-api'; import { ThemeProvider, ensure as ensureTheme } from '@storybook/theming'; +import { ProviderDoesNotExtendBaseProviderError } from '@storybook/core-events/manager-errors'; import { HelmetProvider } from 'react-helmet-async'; @@ -83,7 +84,7 @@ const Main: FC<{ provider: Provider }> = ({ provider }) => { export function renderStorybookUI(domNode: HTMLElement, provider: Provider) { if (!(provider instanceof Provider)) { - throw new Error('provider is not extended from the base Provider'); + throw new ProviderDoesNotExtendBaseProviderError(); } ReactDOM.render(, domNode); diff --git a/code/ui/manager/src/runtime.ts b/code/ui/manager/src/runtime.ts index bb1691be334c..dd29b9a45223 100644 --- a/code/ui/manager/src/runtime.ts +++ b/code/ui/manager/src/runtime.ts @@ -1,3 +1,5 @@ +/* eslint-disable local-rules/no-uncategorized-errors */ + import { global } from '@storybook/global'; import type { Channel } from '@storybook/channels'; @@ -5,7 +7,8 @@ import type { AddonStore } from '@storybook/manager-api'; import { addons } from '@storybook/manager-api'; import type { Addon_Types, Addon_Config } from '@storybook/types'; import { createBrowserChannel } from '@storybook/channels'; -import { CHANNEL_CREATED } from '@storybook/core-events'; +import { CHANNEL_CREATED, TELEMETRY_ERROR } from '@storybook/core-events'; +import { UncaughtManagerError } from '@storybook/core-events/manager-errors'; import Provider from './provider'; import { renderStorybookUI } from './index'; @@ -35,6 +38,7 @@ class ReactProvider extends Provider { this.addons = addons; this.channel = channel; + global.__STORYBOOK_ADDONS_CHANNEL__ = channel; if (FEATURES?.storyStoreV7 && CONFIG_TYPE === 'DEVELOPMENT') { this.serverChannel = this.channel; @@ -55,12 +59,51 @@ class ReactProvider extends Provider { } } -const { document } = global; - -const rootEl = document.getElementById('root'); -renderStorybookUI(rootEl, new ReactProvider()); - // Apply all the globals Object.keys(Keys).forEach((key: keyof typeof Keys) => { global[Keys[key]] = values[key]; }); + +function preprocessError( + originalError: Error & { + fromStorybook?: boolean; + category?: string; + target?: any; + currentTarget?: any; + srcElement?: any; + } +) { + let error = originalError; + + if (!originalError.fromStorybook) { + error = new UncaughtManagerError(originalError); + } + + // DOM manipulation errors and other similar errors are not serializable as they contain + // circular references to the window object. If that's the case, we make a simplified copy + if (error.target === window || error.currentTarget === window || error.srcElement === window) { + error = new Error(originalError.message); + error.name = originalError.name || error.name; + error.category = originalError.category; + } + + return error; +} + +global.sendTelemetryError = (error) => { + const channel = global.__STORYBOOK_ADDONS_CHANNEL__; + channel.emit(TELEMETRY_ERROR, preprocessError(error)); +}; + +// handle all uncaught errors at the root of the application and log to telemetry +global.addEventListener('error', (args) => { + const error = args.error || args; + global.sendTelemetryError(error); +}); +global.addEventListener('unhandledrejection', ({ reason }) => { + global.sendTelemetryError(reason); +}); + +const { document } = global; +const rootEl = document.getElementById('root'); +renderStorybookUI(rootEl, new ReactProvider()); diff --git a/code/ui/manager/src/typings.d.ts b/code/ui/manager/src/typings.d.ts index f46c49b91852..2ff3df07e63e 100644 --- a/code/ui/manager/src/typings.d.ts +++ b/code/ui/manager/src/typings.d.ts @@ -25,3 +25,5 @@ declare var __STORYBOOKTHEMING__: any; declare var __STORYBOOKAPI__: any; declare var __STORYBOOKADDONS__: any; declare var __STORYBOOKCLIENTLOGGER__: any; +declare var __STORYBOOK_ADDONS_CHANNEL__: any; +declare var sendTelemetryError: (error: any) => void; diff --git a/code/yarn.lock b/code/yarn.lock index a576cddaf7a5..799a4a106cbe 100644 --- a/code/yarn.lock +++ b/code/yarn.lock @@ -5704,7 +5704,7 @@ __metadata: 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 typescript: ~4.9.3 uuid: ^9.0.0 @@ -6277,7 +6277,7 @@ __metadata: jest-specific-snapshot: ^8.0.0 read-pkg-up: ^7.0.1 semver: ^7.3.7 - telejson: ^7.0.3 + telejson: ^7.2.0 tmp: ^0.2.1 ts-dedent: ^2.0.0 tsconfig-paths-webpack-plugin: ^4.0.1 @@ -6383,7 +6383,7 @@ __metadata: memoizerific: ^1.11.3 polished: ^4.2.2 react-colorful: ^5.1.2 - telejson: ^7.0.3 + telejson: ^7.2.0 tocbot: ^4.20.1 ts-dedent: ^2.0.0 util-deprecate: ^1.0.2 @@ -6554,7 +6554,7 @@ __metadata: "@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 typescript: ~4.9.3 languageName: unknown @@ -6754,6 +6754,7 @@ __metadata: version: 0.0.0-use.local resolution: "@storybook/core-events@workspace:lib/core-events" dependencies: + ts-dedent: ^2.0.0 typescript: ~4.9.3 languageName: unknown linkType: soft @@ -6808,7 +6809,7 @@ __metadata: semver: ^7.3.7 serve-favicon: ^2.5.0 slash: ^5.0.0 - telejson: ^7.0.3 + telejson: ^7.2.0 tiny-invariant: ^1.3.1 ts-dedent: ^2.0.0 typescript: ~4.9.3 @@ -7088,7 +7089,7 @@ __metadata: qs: ^6.10.0 semver: ^7.3.7 store2: ^2.14.2 - telejson: ^7.0.3 + telejson: ^7.2.0 ts-dedent: ^2.0.0 typescript: ~4.9.3 peerDependencies: @@ -7514,6 +7515,7 @@ __metadata: "@storybook/channels": "workspace:*" "@storybook/client-logger": "workspace:*" "@storybook/core-events": "workspace:*" + "@storybook/global": ^5.0.0 "@storybook/preview-api": "workspace:*" typescript: ~4.9.3 languageName: unknown @@ -7771,6 +7773,7 @@ __metadata: eslint: ^8.28.0 eslint-import-resolver-typescript: ^3.5.2 eslint-plugin-import: ^2.26.0 + eslint-plugin-local-rules: "portal:../scripts/eslint-plugin-local-rules" eslint-plugin-react: ^7.31.10 eslint-plugin-storybook: ^0.6.6 fs-extra: ^11.1.0 @@ -15751,6 +15754,12 @@ __metadata: languageName: node linkType: hard +"eslint-plugin-local-rules@portal:../scripts/eslint-plugin-local-rules::locator=%40storybook%2Froot%40workspace%3A.": + version: 0.0.0-use.local + resolution: "eslint-plugin-local-rules@portal:../scripts/eslint-plugin-local-rules::locator=%40storybook%2Froot%40workspace%3A." + languageName: node + linkType: soft + "eslint-plugin-prettier@npm:^3.4.0": version: 3.4.1 resolution: "eslint-plugin-prettier@npm:3.4.1" @@ -29440,12 +29449,12 @@ __metadata: languageName: node linkType: hard -"telejson@npm:^7.0.3": - version: 7.1.0 - resolution: "telejson@npm:7.1.0" +"telejson@npm:^7.2.0": + version: 7.2.0 + resolution: "telejson@npm:7.2.0" dependencies: memoizerific: ^1.11.3 - checksum: dc9a185d0e00d947c0eaa229bfb993aab61a3ba79282ae409768fc8ae66d236e89a64ebe291f9ea6ed5e05396e0be52a7542ea32b6c1321b20440f28c7828edc + checksum: d26e6cc93e54bfdcdb207b49905508c5db45862e811a2e2193a735409e47b14530e1c19351618a3e03ad2fd4ffc3759364fcd72851aba2df0300fab574b6151c languageName: node linkType: hard diff --git a/scripts/eslint-plugin-local-rules/README.md b/scripts/eslint-plugin-local-rules/README.md new file mode 100644 index 000000000000..86ca125d559d --- /dev/null +++ b/scripts/eslint-plugin-local-rules/README.md @@ -0,0 +1,12 @@ +## ESLint plugin local rules + +This package serves as a local ESLint plugin to be used in the monorepo and help maintainers keep certain code standards. + +### Development + +If you're fixing a rule or creating a new one, make sure to: + +1. Make your code changes +2. Rerun yarn install in the `code` directory. It's necessary to update the module reference +3. Update the necessary `.eslintrc.js` files (if you are adding a new rule) +4. Restart the ESLint server in your IDE diff --git a/scripts/eslint-plugin-local-rules/index.js b/scripts/eslint-plugin-local-rules/index.js new file mode 100644 index 000000000000..3e3e116b7527 --- /dev/null +++ b/scripts/eslint-plugin-local-rules/index.js @@ -0,0 +1,7 @@ +/* eslint-disable global-require */ +module.exports = { + rules: { + 'no-uncategorized-errors': require('./no-uncategorized-errors'), + 'no-duplicated-error-codes': require('./no-duplicated-error-codes'), + }, +}; diff --git a/scripts/eslint-plugin-local-rules/no-duplicated-error-codes.js b/scripts/eslint-plugin-local-rules/no-duplicated-error-codes.js new file mode 100644 index 000000000000..53968edb572c --- /dev/null +++ b/scripts/eslint-plugin-local-rules/no-duplicated-error-codes.js @@ -0,0 +1,49 @@ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Ensure unique error codes per category in the same file', + category: 'Best Practices', + recommended: true, + }, + fixable: null, + }, + create(context) { + const errorClasses = {}; + + return { + ClassDeclaration(node) { + if (node.superClass.name === 'StorybookError') { + const categoryProperty = node.body.body.find((prop) => { + return prop.type === 'PropertyDefinition' && prop.key.name === 'category'; + }); + + const codeProperty = node.body.body.find((prop) => { + return prop.type === 'PropertyDefinition' && prop.key.name === 'code'; + }); + + if (categoryProperty && categoryProperty.value.type === 'MemberExpression') { + const categoryName = categoryProperty.value.property.name; + + if (codeProperty && codeProperty.value.type === 'Literal') { + const errorCode = codeProperty.value.value; + + if (!errorClasses[categoryName]) { + errorClasses[categoryName] = new Set(); + } + + if (errorClasses[categoryName].has(errorCode)) { + context.report({ + node: codeProperty, + message: `Duplicate error code '${errorCode}' in category '${categoryName}'.`, + }); + } else { + errorClasses[categoryName].add(errorCode); + } + } + } + } + }, + }; + }, +}; diff --git a/scripts/eslint-plugin-local-rules/no-uncategorized-errors.js b/scripts/eslint-plugin-local-rules/no-uncategorized-errors.js new file mode 100644 index 000000000000..ce91cccf1040 --- /dev/null +++ b/scripts/eslint-plugin-local-rules/no-uncategorized-errors.js @@ -0,0 +1,24 @@ +module.exports = { + meta: { + type: 'problem', + docs: { + description: 'Disallow usage of the Error JavaScript class.', + category: 'Best Practices', + recommended: true, + url: 'https://github.com/storybookjs/storybook/blob/next/code/lib/core-events/src/errors/README.md', + }, + }, + create(context) { + return { + NewExpression(node) { + if (node.callee.name === 'Error') { + context.report({ + node, + message: + 'Avoid using a generic Error class. Use a categorized StorybookError class instead. See the docs 👉', + }); + } + }, + }; + }, +}; diff --git a/scripts/eslint-plugin-local-rules/package.json b/scripts/eslint-plugin-local-rules/package.json new file mode 100644 index 000000000000..d61a7e099db9 --- /dev/null +++ b/scripts/eslint-plugin-local-rules/package.json @@ -0,0 +1,8 @@ +{ + "name": "eslint-plugin-local-rules", + "version": "1.0.0", + "description": "", + "license": "ISC", + "author": "", + "main": "index.js" +}