CLI: Prompt for init crash reports#34316
Conversation
|
View your CI Pipeline Execution ↗ for commit 82b7f69
☁️ Nx Cloud last updated this comment at |
|
View your CI Pipeline Execution ↗ for commit 5a63119 ☁️ Nx Cloud last updated this comment at |
| eventType?: EventType; | ||
| }; | ||
|
|
||
| const promptCrashReports = async () => { |
There was a problem hiding this comment.
Are we making sure that we don't prompt if the error is a non-blocking error? blocking: false. Or do we even want to prompt in this scenario?
There was a problem hiding this comment.
Why would we not prompt in a non-blocking scenario? Don't we still benefit from the information?
There was a problem hiding this comment.
We could prompt with a slightly different message to avoid confusion, e.g. "Some errors happened, though Storybook successfully installed. "
There was a problem hiding this comment.
I've disabled prompting for errors in non-blocking scenarios, specifically during init only. I've also updated the manual QA steps in the PR description with this case.
|
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)
📝 WalkthroughWalkthroughAdds an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant withTelemetry
participant sendTelemetryError
participant getErrorLevel
participant Cache
participant Prompt
Caller->>withTelemetry: invoke (error, eventType='init')
withTelemetry->>sendTelemetryError: report error (passes eventType)
sendTelemetryError->>getErrorLevel: determine level (passes eventType, blocking, skipPrompt)
alt presetOptions present
getErrorLevel-->>sendTelemetryError: return preset-determined level
else presetOptions missing & eventType='init'
getErrorLevel->>Cache: read 'enableCrashReports'
alt cache has consent
Cache-->>getErrorLevel: consent value
getErrorLevel-->>sendTelemetryError: return level (full or error)
else no cache
alt skipPrompt true
getErrorLevel-->>sendTelemetryError: return non-prompted level (e.g., error)
else
getErrorLevel->>Prompt: prompt.confirm for consent
Prompt-->>getErrorLevel: accept/reject
getErrorLevel->>Cache: cache.set('enableCrashReports', value)
Cache-->>getErrorLevel: cached
getErrorLevel-->>sendTelemetryError: return level (full or error)
end
end
end
sendTelemetryError-->>withTelemetry: level & payload (blocking/immediate flags)
withTelemetry-->>Caller: continue execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
Comment |
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 50 | 50 | 0 |
| Self size | 20.47 MB | 20.46 MB | 🎉 -11 KB 🎉 |
| Dependency size | 16.55 MB | 16.55 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/nextjs-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 92 | 92 | 0 |
| Self size | 1.12 MB | 1.12 MB | 0 B |
| Dependency size | 22.76 MB | 22.73 MB | 🎉 -33 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-native-web-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 121 | 121 | 0 |
| Self size | 30 KB | 30 KB | 0 B |
| Dependency size | 23.83 MB | 23.80 MB | 🎉 -32 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/react-vite
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 82 | 82 | 0 |
| Self size | 35 KB | 35 KB | 0 B |
| Dependency size | 20.55 MB | 20.51 MB | 🎉 -33 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 184 | 184 | 0 |
| Self size | 782 KB | 780 KB | 🎉 -1 KB 🎉 |
| Dependency size | 67.69 MB | 67.68 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 177 | 177 | 0 |
| Self size | 32 KB | 32 KB | 0 B |
| Dependency size | 66.22 MB | 66.21 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 51 | 51 | 0 |
| Self size | 1.04 MB | 1.04 MB | 🎉 -779 B 🎉 |
| Dependency size | 37.03 MB | 37.02 MB | 🎉 -11 KB 🎉 |
| Bundle Size Analyzer | node | node |
…-init CLI: Prompt for init crash reports (cherry picked from commit 8d12309)
Closes #
What I did
Prompt for crash report consent when
storybook initfails, so init errors can include full stack traces after the user opts in. This reuses the existing consent/cache flow and adds coverage for both accept and reject paths.Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
Crash report prompting
yarn task --task sandbox --start-from auto --template react-vite/default-ts-->
create-storybookwithyarn nx compile create-storybook.cd scripts && yarn jiti ./event-log-collector.ts.tmp="$(mktemp -d)" && cd "$tmp" && printf '{ "name": "sb-init-repro", "private": true }\n' > package.json && HOME="$tmp/.home" env -u STORYBOOK_DISABLE_TELEMETRY STORYBOOK_TELEMETRY_URL="http://127.0.0.1:6007/event-log" STORYBOOK_TELEMETRY_DEBUG=1 node PATH_TO_STORYBOOK_REPO/storybook/code/lib/create-storybook/dist/bin/index.js --no-devcurl -s http://127.0.0.1:6007/event-log | python -m json.tooland verify theerrorevent includespayload.error.errorevent is still sent butpayload.erroris omitted.Not prompting in non-blocking error cases
create-storybookCLI from the Storybook repo:cd /Users/jeppe/dev/work/storybook/storybook yarn nx compile create-storybookcd /Users/jeppe/dev/work/storybook/storybook/scripts yarn jiti ./event-log-collector.tsnpm create vite@latest sb-nonblocking-repro -- --template react-ts cd sb-nonblocking-repro npm installvite.config.tswith this file:storybook initwith the locally built CLI and point telemetry at the event collector:Expected result
storybook initcontinues far enough to hit addon-vitest setupInspect the captured telemetry
curl -s http://127.0.0.1:6007/event-log | python -m json.toolLook for an
errorevent with:"eventType": "init""blocking": falsepayload.errorfield unless crash-report consent was already cached beforehandIf needed, I can also write the matching blocking init error verification steps in the same format.
Documentation
MIGRATION.MD
Checklist for Maintainers
When this PR is ready for testing, make sure to add
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.tsMake sure this PR contains one of the labels below:
Available labels
bug: Internal changes that fixes incorrect behavior.maintenance: User-facing maintenance tasks.dependencies: Upgrading (sometimes downgrading) dependencies.build: Internal-facing build tooling & test updates. Will not show up in release changelog.cleanup: Minor cleanup style change. Will not show up in release changelog.documentation: Documentation only changes. Will not show up in release changelog.feature request: Introducing a new feature.BREAKING CHANGE: Changes that break compatibility in some way with current major version.other: Changes that don't fit in the above categories.🦋 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/coreteam here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook publish.yml --field pr=<PR_NUMBER>Summary by CodeRabbit
Tests
Bug Fixes