Skip to content

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented Jul 22, 2025

Closes #

What I did

This PR attaches (sanitized, anonymized errors) to automigrations in the telemetry data, so we can detect patterns and use them to improve the Storybook upgrade experience.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

This section is mandatory for all contributions. If you believe no manual test is necessary, please state so explicitly. Thanks!

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for 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-storybook/src/sandbox-templates.ts

  • Make 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/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>

Greptile Summary

This PR enhances Storybook's telemetry system by adding detailed error tracking for automigrations. The changes modify how error data is collected, stored, and transmitted during the upgrade process, specifically:

  1. Introduces a new AutomigrationResult type that tracks both success statuses and error messages
  2. Implements error sanitization using sanitizeError to ensure privacy when collecting error data
  3. Updates the telemetry payload structure to include detailed error information
  4. Modifies how failed automigrations are counted (now based on errors object length)

Confidence score: 5/5

  1. This PR is extremely safe to merge as it only adds telemetry data collection and doesn't affect core functionality
  2. The changes are well-structured, properly typed, and include appropriate error sanitization
  3. The modified files (upgrade.ts and multi-project.ts) need careful review to ensure proper error handling

…results

- Updated the upgrade.ts and multi-project.ts files to replace FixId and FixStatus with the new AutomigrationResult type.
- Enhanced the structure of project results to include automigration statuses and errors.
- Adjusted related functions to accommodate the new data structure for improved clarity and maintainability.
@yannbf yannbf added maintenance User-facing maintenance tasks telemetry ci:normal labels Jul 22, 2025
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 files reviewed, no comments

Edit Code Review Bot Settings | Greptile

Copy link

nx-cloud bot commented Jul 22, 2025

View your CI Pipeline Execution ↗ for commit 4345344

Command Status Duration Result
nx run-many -t build --parallel=3 ✅ Succeeded 1m 18s View ↗

☁️ Nx Cloud last updated this comment at 2025-07-22 13:46:26 UTC

@valentinpalkovic valentinpalkovic merged commit 63d0170 into next Jul 23, 2025
56 of 59 checks passed
@valentinpalkovic valentinpalkovic deleted the yann/automigration-failure-telemetry branch July 23, 2025 13:53
@github-actions github-actions bot mentioned this pull request Jul 23, 2025
11 tasks
const { dryRun, skipInstall, automigrations } = options;
const projectResults: Record<ConfigDir, Record<FixId, FixStatus>> = {};
const projectResults: Record<ConfigDir, AutomigrationResult> = {};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we do it differently so that we have projectResults and projectErrors as peers? That way it's not a breaking change to the telemetry. Otherwise the new data won't be compatible with the old data and will be a pain in the neck.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:normal maintenance User-facing maintenance tasks telemetry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants