Repository navigation
CLI: Categorize Execa failures for init telemetry - #35720
Merged
Merged
Conversation
Wrap subprocess errors from executeCommand in StorybookError subclasses so telemetry can slice init failures by SB_* code instead of opaque message hashes, including stable messages when Execa output is empty. Closes #34301 Co-authored-by: Cursor <cursoragent@cursor.com>
ndelangen
added
bug
ci:normal
Contributor
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 21.42 MB | 21.43 MB | 🚨 +13 KB 🚨 |
| Dependency size | 31.21 MB | 31.21 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 205 | 205 | 0 |
| Self size | 829 KB | 829 KB | 0 B |
| Dependency size | 86.10 MB | 86.12 MB | 🚨 +13 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 198 | 198 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 84.58 MB | 84.59 MB | 🚨 +13 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 74 | 74 | 0 |
| Self size | 1.09 MB | 1.09 MB | 0 B |
| Dependency size | 52.63 MB | 52.65 MB | 🚨 +13 KB 🚨 |
| Bundle Size Analyzer | node | node |
Keep only coverage for empty-output messaging, install classification, and the generic fallback — matching how other categorized errors are tested in the repo. Co-authored-by: Cursor <cursoragent@cursor.com>
command.test.ts already covers categorized failures at the executeCommand boundary, consistent with how other Storybook errors are tested in this repo. Co-authored-by: Cursor <cursoragent@cursor.com>
Returning execaProcess.catch(...) stripped ResultPromise methods like on('exit'), which broke storybook init in CI. Categorize failures via patched then/catch handlers instead.
Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves CLI/init telemetry by converting raw Execa failures into categorized StorybookError subclasses with stable SB_* error codes and non-empty messages derived from Execa output (stdout/stderr/shortMessage/exit code), addressing large opaque errorHash buckets (e.g. EMPTY_MESSAGE) reported in #34301.
Changes:
- Added new CLI/init/automigrate
StorybookErrorsubclasses to represent common subprocess failure modes (install conflicts, missing manifest, pnpm-specific failures, missing binary, Playwright install, Nuxt module add, automigrate). - Introduced
categorizeExecaError()to map Execa-like errors into these stable error classes based on command/args/log signatures. - Updated
executeCommand/executeCommandSyncto throw categorizedStorybookErrors (and updated tests accordingly); ensuredJsPackageManagerrethrows Storybook-originated errors to preserve categorization.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| code/core/src/server-errors.ts | Adds new StorybookError subclasses and shared data shape for categorized Execa failures. |
| code/core/src/common/utils/command.ts | Wraps async/sync command execution failures with categorizeExecaError for stable error codes/messages. |
| code/core/src/common/utils/command.test.ts | Updates expectations to assert categorized error types and preserves subprocess methods. |
| code/core/src/common/utils/categorize-execa-error.ts | New categorization logic extracting logs/codes and mapping into the new error classes. |
| code/core/src/common/js-package-manager/JsPackageManager.ts | Preserves categorized Storybook errors by rethrowing them instead of wrapping in HandledError. |
Suppressed comments (1)
code/core/src/common/utils/command.ts:53
- Similarly, the patched
.catchpassesonRejecteddirectly into a new.catch(...). If a non-function is provided, native Promise behavior would ignore it, but this code will forward it and may throw at runtime. Guard it with atypeof === 'function'check.
execaProcess.catch = ((onRejected) =>
originalCatch((error) => {
throw categorize(error);
}).catch(onRejected)) as ResultPromise['catch'];
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughChangesExeca error categorization
Sequence Diagram(s)sequenceDiagram
participant command.ts
participant categorizeExecaError
participant StorybookError
command.ts->>categorizeExecaError: pass Execa failure metadata
categorizeExecaError->>StorybookError: create specialized error with cause
StorybookError-->>command.ts: return categorized failure
Possibly related issues
✨ Finishing Touches📝 Generate docstrings
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
code/core/src/common/utils/command.ts (1)
32-54: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPatch
.finallytogether with.thenand.catch. Execa 9.6.1 binds all three methods to its internal promise, soexecaProcess.finally(...)bypasses the patched handlers and can reject with the uncategorized error. No currentexecuteCommand(...).finally(...)call sites exist, butResultPromiseexposes this API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/common/utils/command.ts` around lines 32 - 54, Update attachCategorizedExecaFailureHandler to also wrap execaProcess.finally, preserving the existing categorization behavior when the underlying ResultPromise rejects. Bind and delegate through the original finally implementation, ensuring finally callbacks remain unchanged while errors reaching the returned promise are categorized consistently with the patched then and catch methods.code/core/src/common/js-package-manager/JsPackageManager.ts (1)
380-399: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAwait the
runAddDeps()result inside thistry.All implementations return
executeCommand()’sResultPromise, whilecategorizeExecaError()runs only on rejection. ReturningresultletsaddDependencies()adopt that rejection after thiscatchexits, so thefromStorybookbranch cannot handle categorized failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@code/core/src/common/js-package-manager/JsPackageManager.ts` around lines 380 - 399, Update the addDependencies try block around runAddDeps so it awaits the returned ResultPromise before assigning or returning the result. Keep the await inside the existing try, ensuring rejected executeCommand failures reach the catch and preserve the fromStorybook handling and HandledError conversion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/common/utils/categorize-execa-error.ts`:
- Around line 113-121: Update isPackageInstall to treat the add argument as a
package-install command alongside install, preserving the existing command
normalization and package-manager checks.
In `@code/core/src/server-errors.ts`:
- Around line 943-979: Remove the duplicate ExecaCommandErrorData type and
formatExecaCommand/formatExecaFailureDetails helpers from
code/core/src/server-errors.ts:943-979, importing and reusing the single-source
definitions from code/core/src/common/utils/categorize-execa-error.ts instead.
Keep code/core/src/common/utils/categorize-execa-error.ts:21-60 as the canonical
implementation and make no direct changes there unless needed to support the
shared imports.
---
Nitpick comments:
In `@code/core/src/common/js-package-manager/JsPackageManager.ts`:
- Around line 380-399: Update the addDependencies try block around runAddDeps so
it awaits the returned ResultPromise before assigning or returning the result.
Keep the await inside the existing try, ensuring rejected executeCommand
failures reach the catch and preserve the fromStorybook handling and
HandledError conversion.
In `@code/core/src/common/utils/command.ts`:
- Around line 32-54: Update attachCategorizedExecaFailureHandler to also wrap
execaProcess.finally, preserving the existing categorization behavior when the
underlying ResultPromise rejects. Bind and delegate through the original finally
implementation, ensuring finally callbacks remain unchanged while errors
reaching the returned promise are categorized consistently with the patched then
and catch methods.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81b1af85-7533-4831-ac07-f17198f5fb81
📒 Files selected for processing (5)
code/core/src/common/js-package-manager/JsPackageManager.tscode/core/src/common/utils/categorize-execa-error.tscode/core/src/common/utils/command.test.tscode/core/src/common/utils/command.tscode/core/src/server-errors.ts
Check command-specific failures before binary-not-found, treat package-manager add/ci as install failures, dedupe ExecaCommandErrorData helpers via server-errors exports, and only invoke promise rejection handlers when they are functions. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@code/core/src/common/utils/categorize-execa-error.ts`:
- Around line 151-152: Update the binary-not-found branch in
categorizeExecaError so it returns PackageManagerBinaryNotFoundError only when
isBinaryNotFound(data) and data.command identifies a package-manager command;
allow missing non-package-manager executables to follow the existing alternative
classification path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: edbf4938-b16e-4c35-afea-cd0e89e1b7a5
📒 Files selected for processing (3)
code/core/src/common/utils/categorize-execa-error.tscode/core/src/common/utils/command.tscode/core/src/server-errors.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- code/core/src/common/utils/command.ts
- code/core/src/server-errors.ts
shilman
approved these changes
Aug 4, 2026
Member
Author
|
@shilman SB_CLI_INIT_0011 (PackageInstallDependencyConflictError) — forced via storybook init on a project with conflicting react/react-dom versions An agent set up the reproduction, but I saw the telemetry in the SB CLI firsthand. |
Only classify missing npm/pnpm/yarn/bun binaries as PackageManagerBinaryNotFoundError so other missing executables fall through to ExecaCommandFailedError. Co-authored-by: Cursor <cursoragent@cursor.com>
2 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related: #34301
What I did
Init and addon setup failures were reaching telemetry as raw Execa errors, which collapsed into opaque
errorHashbuckets (including a largeEMPTY_MESSAGEgroup). This wraps everyexecuteCommand/executeCommandSyncfailure in categorizedStorybookErrorsubclasses so telemetry receives stableSB_*codes.Added error classes for the high-volume failure modes seen in production data:
SB_CLI_0003)SB_CLI_INIT_0010–0014) — dependency conflicts, missing manifest, pnpm ignored builds, pnpm no-TTYSB_CLI_INIT_0015)SB_CLI_INIT_0016)nuxi module add(SB_CLI_INIT_0017)addon-a11y-addon-test(SB_CLI_AUTOMIGRATE_0003)Errors always include a message rebuilt from Execa output (
stderr/stdout/shortMessage) or, when those are empty, from the exit code — so telemetry no longer hashes blank strings.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
No manual testing is required. This change only affects error classification and messaging for failed subprocess calls; behavior on success is unchanged. Maintainers can verify by running
yarn test categorize-execa-errorand inspecting that a failingstorybook initreports anSB_*error code in telemetry.Documentation
MIGRATION.MD
Checklist for Maintainers
ci:normal,ci:mergedorci:dailyGH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli-storybook/src/sandbox-templates.tsqa:neededorqa:skipMade with Cursor