Core: Categorize UniversalStore follower timeout error#34592
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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)
📝 WalkthroughWalkthroughAdds a new ChangesUniversal store follower timeout integration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager-errors.ts`:
- Line 21: The enum member MANAGER_UNIVERSAL_STORE in manager-errors.ts uses an
underscore in its string value; update its value to use a hyphen to match the
existing multi-word MANAGER_* convention and telemetry format (change the value
from 'MANAGER_UNIVERSAL_STORE' to 'MANAGER_UNIVERSAL-STORE') so generated codes
become SB_MANAGER_UNIVERSAL-STORE_0001 and remain consistent with other members
like MANAGER_CLIENT-LOGGER and MANAGER_CORE-EVENTS.
🪄 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: e4375fe2-03f6-4485-af0b-635e0a70dd57
📒 Files selected for processing (3)
code/core/src/manager-errors.tscode/core/src/shared/universal-store/index.test.tscode/core/src/shared/universal-store/index.ts
bec139d to
41cd7e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@code/core/src/manager-errors.ts`:
- Around line 71-80: The error message in UniversalStoreFollowerTimeoutError is
misleading: change the constructor's message to reflect the actual observed
condition (a sync timeout) rather than asserting "No existing state"; update the
message in UniversalStoreFollowerTimeoutError (class name) passed to super(...)
so it states that the follower with followerId failed to sync with its leader
within the 1s timeout and include a helpful hint to verify the leader exists and
network/connectivity, while preserving name 'UniversalStoreFollowerTimeoutError'
and Category.MANAGER_UNIVERSAL_STORE and the existing code structure.
🪄 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: e4f0c053-ddea-4780-9a18-251b852cc13f
📒 Files selected for processing (3)
code/core/src/manager-errors.tscode/core/src/shared/universal-store/index.test.tscode/core/src/shared/universal-store/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- code/core/src/shared/universal-store/index.ts
Package BenchmarksCommit: The following packages have significant changes to their size or dependencies:
|
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 72 | 72 | 0 |
| Self size | 20.25 MB | 20.31 MB | 🚨 +56 KB 🚨 |
| Dependency size | 36.17 MB | 36.17 MB | 0 B |
| Bundle Size Analyzer | Link | Link |
@storybook/cli
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 203 | 203 | 0 |
| Self size | 908 KB | 908 KB | 🎉 -78 B 🎉 |
| Dependency size | 87.55 MB | 87.61 MB | 🚨 +56 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
@storybook/codemod
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 196 | 196 | 0 |
| Self size | 32 KB | 32 KB | 🚨 +36 B 🚨 |
| Dependency size | 86.04 MB | 86.10 MB | 🚨 +56 KB 🚨 |
| Bundle Size Analyzer | Link | Link |
create-storybook
| Before | After | Difference | |
|---|---|---|---|
| Dependency count | 73 | 73 | 0 |
| Self size | 1.08 MB | 1.08 MB | 🎉 -222 B 🎉 |
| Dependency size | 56.42 MB | 56.48 MB | 🚨 +56 KB 🚨 |
| Bundle Size Analyzer | node | node |
Closes #34566
What I did
Categorized the UniversalStore follower timeout failure with a dedicated
StorybookErrorinstead of letting it fall back to the generic uncaught manager error bucket.Specifically:
MANAGER_UNIVERSAL_STOREerror categoryUniversalStoreFollowerTimeoutErrorTypeErrorthrown in the UniversalStore follower timeout pathWhy
Previously, when a follower timed out waiting for a leader, it threw a plain
TypeError. Since that error was not aStorybookError, telemetry grouped it under the generic uncaught manager bucket instead of reporting it as a distinct failure.With this change, the timeout is reported as its own categorized error, making it easier to track and triage in telemetry.
Checklist for Contributors
Testing
The changes in this PR are covered in the following automated tests:
Manual testing
No manual testing was required for this change because it updates error classification and the related unit test coverage.
Documentation
MIGRATION.MD
Summary by CodeRabbit
New Features
Bug Fixes
Tests