Core: Add error categorization framework #23653

yannbf · 2023-07-28T16:36:15Z

Closes #23511

What I did

This PR proposes a new way to write and throw errors in Storybook.

For more details, please check the README included in this PR.

Checklist

Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
Make sure to add/update documentation regarding your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Maintainers

When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli/src/sandbox-templates.ts
Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

code/lib/core-server/src/presets/common-preset.ts

code/lib/core-events/src/errors/preview-errors.ts

code/lib/core-events/src/errors/README.md

kylegach

I only reviewed the README, but I'm loving this work! Great job, Yann! 👏

scripts/eslint-plugin-local-rules/no-duplicated-error-codes.js

code/lib/core-server/src/presets/common-preset.ts

kasperpeulen · 2023-08-14T07:45:48Z

code/lib/core-events/src/errors/server-errors.ts

+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',
+  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',
+}


I'm thinking that this category should not be a field of StorybookError, but should be should be passed to telemetry at the point where the error is caught.

For example, a function such as readConfig from @storybook/csf-tools will throw all kind of errors, but we likely want to know in telemetry more about the context where this function is called. As it can be called in codemods, the eslint-plugin, build, init, dev etc.

Also, I'm kind of thinking that the boolean telemetry should not be part of the StorybookError class, but is something that is also context dependent. As for example, I don't think we might not want to send telemetry for the eslint plugin, as it can be used independent of storybook (for other integrations)?

Maybe we should just maintain a list of code's that we want telemetry for?

I guess you could say that telemetry is just a different concern all together than throwing errors. For example, when you use composeStories outside of storybook, or when used by other integrators, it is kind of weird that the Error that we throw contains a telemetry boolean.

code/lib/core-events/src/errors/storybook-error.ts

code/ui/manager/src/runtime.ts

code/lib/core-server/src/withTelemetry.ts

socket-security · 2023-08-21T08:16:13Z

Removed dependencies detected. Learn more about Socket for GitHub ↗︎

🚮 Removed packages: telejson@7.1.0

code/lib/core-events/src/errors/README.md

code/ui/manager/src/runtime.ts

yannbf added feature request core ci:normal patch:no labels Jul 28, 2023

yannbf commented Jul 31, 2023

View reviewed changes

code/lib/core-server/src/presets/common-preset.ts Outdated Show resolved Hide resolved

yannbf commented Jul 31, 2023

View reviewed changes

code/lib/core-events/src/errors/preview-errors.ts Outdated Show resolved Hide resolved