Telemetry: Fix race condition in telemetry cache causing malformed JSON #33323

valentinpalkovic · 2025-12-10T10:49:45Z

Closes #

What I did

We observed that the lastEvents cache file occasionally becomes malformed (e.g., ending in sequences like ...}89}}}), leading to parsing errors.

The root cause was identified in event-cache.ts. The previous implementation of the set function attempted to queue write operations using a shared operation promise. However, because the operation variable was updated after the await statement, rapid concurrent calls failed to chain correctly.

The Scenario (Old Behavior): If three write events ($A$, $B$, $C$) are triggered in quick succession:

Event A starts running; operation points to A.
Event B is called. It awaits operation (A).
Event C is called. Since B is currently suspended at the await and hasn't updated the variable yet, C also awaits operation (A).
When A completes, both B and C resume simultaneously.
Both read the file, modify it, and attempt to write back at the same time, resulting in a race condition and file corruption.

The Fix (New Behavior): This PR refactors set to chain promises synchronously using .then(). This ensures that the queue is constructed immediately when the function is called, regardless of when the execution actually occurs.

Flow with this fix:

Event A is called. processingPromise becomes $A$.
Event B is called. It immediately chains onto $A$ ($A \rightarrow B$). processingPromise is updated to $B$.
Event C is called. It immediately chains onto $B$ ($A \rightarrow B \rightarrow C$). processingPromise is updated to $C$.

This guarantees strictly sequential file I/O: A must finish before B starts, and B must finish before C starts.

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 pull request has been released as version 0.0.0-pr-33323-sha-bb01d69f. Try it out in a new sandbox by running npx [email protected] sandbox or in an existing project with npx [email protected] upgrade.

More information


Published version	`0.0.0-pr-33323-sha-bb01d69f`
Triggered by	@valentinpalkovic
Repository	storybookjs/storybook
Branch	`valentin/fix-telemetry-last-event`
Commit	`bb01d69f`
Datetime	Thu Dec 11 08:11:46 UTC 2025 (`1765440706`)
Workflow run	20126317577

To request a new release of this pull request, mention the @storybookjs/core team.

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

Summary by CodeRabbit

Chores
- Centralized telemetry imports and expanded public telemetry API (including session ID access).
Chores
- Adjusted telemetry payloads to include session identifiers for more accurate event context.
Chores
- Introduced an asynchronous queuing mechanism for telemetry updates to ensure sequential processing and resilience.
Bug Fixes
- Reads now wait for pending telemetry writes, preventing race conditions and stale state.
Tests
- Strengthened telemetry tests to validate queued operations, error handling, and session-aware events.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…ons and error handling - Updated `set` function to queue multiple operations and process them sequentially. - Enhanced error handling to allow subsequent operations to succeed even if a previous one fails. - Modified `getLastEvents` to wait for the completion of queued operations before returning data. - Adjusted tests to cover new queuing behavior and error scenarios.

nx-cloud · 2025-12-10T10:50:22Z

View your CI Pipeline Execution ↗ for commit bb01d69

Command	Status	Duration	Result
`nx run-many -t compile,check,knip,test,pretty-d...`	✅ Succeeded	9m 6s	View ↗

☁️ Nx Cloud last updated this comment at 2025-12-11 08:03:31 UTC

coderabbitai · 2025-12-10T10:53:21Z

📝 Walkthrough

Walkthrough

Centralizes telemetry imports, exposes getLastEvents/CacheEntry/getSessionId, converts event-cache to a queued async model (processingPromise) with typed set/getPrecedingUpgrade, and extends preview-initialized payload to accept and use a sessionId.

Changes

Cohort / File(s)	Summary
Telemetry public API `code/core/src/telemetry/index.ts`	Added exports: `getLastEvents`, `CacheEntry` type, and `getSessionId`.
Event-cache implementation `code/core/src/telemetry/event-cache.ts`	Replaced single-operation model with a queued Promise chain (`processingPromise`); `set(eventType, body)` now accepts `TelemetryEvent` and returns `Promise<void>` by enqueuing work; `getLastEvents()` awaits queue completion; signatures tightened to use `CacheEntry`/`Partial<Record<...>>`.
Event-cache tests `code/core/src/telemetry/event-cache.test.ts`	Tests updated to use `CacheEntry`/`TelemetryEvent` types, introduced `createTelemetryEvent` helper, mocked immediate resolutions for cache ops, added sequential-set and error-handling queue tests, updated snapshots to include `sessionId`.
Preview initialization telemetry `code/core/src/core-server/server-channel/preview-initialized-channel.ts`	Switched to centralized `storybook/internal/telemetry` imports (`CacheEntry`, `getLastEvents`, `getSessionId`); extended `makePayload` signature to accept `sessionId` and use it when computing `timeSinceInit` and `isNewUser`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Pay attention to: correct promise-chaining and catch behavior in event-cache.ts (ensure queue isn't broken by rejections).
Verify getLastEvents() properly awaits in-flight sets and that type narrowing for CacheEntry is correct.
Confirm tests in event-cache.test.ts accurately simulate queue timing and error cases.
Ensure preview-initialized-channel.ts receives the sessionId from callers and imports match the new telemetry public API.

Possibly related PRs

Telemetry: Fix preview-first-load event #32859 — Directly modifies makePayload/sessionId handling and telemetry exports; strong overlap with this change.
Core: Add preview-first-load telemetry #32770 — Adjusts preview-initialized telemetry wiring to use centralized telemetry utilities (getLastEvents, getSessionId).
Telemetry: Await pending operations in getLastEvents to prevent race conditions #33285 — Changes getLastEvents to await pending set operations (queued async pattern) — closely related to event-cache queue behavior.

✨ Finishing touches

📝 Generate docstrings

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

code/core/src/telemetry/event-cache.ts (1)
97-103: getLastEvents may throw if the last queued set operation failed.

Since currentOperation can be a rejected promise (when a set operation fails), awaiting it without a try-catch will propagate that rejection to callers of getLastEvents. This may be unexpected behavior for a "read" operation.

Consider catching the rejection to ensure getLastEvents always returns data:
 export const getLastEvents = async (): Promise<Record<EventType, CacheEntry>> => {
   // Wait for any pending set operations to complete before reading
   // This prevents race conditions where getLastEvents() reads stale data
   // while a set() operation is still in progress
-  await currentOperation;
+  try {
+    await currentOperation;
+  } catch {
+    // Previous operation failed, but we can still read
+  }
   return (await cache.get('lastEvents')) || {};
 };

🧹 Nitpick comments (2)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (1)
3-5: Consolidate duplicate imports from the same module.

Lines 3, 4, and 5 all import from 'storybook/internal/telemetry'. These should be merged into a single import statement.
-import { type InitPayload, telemetry } from 'storybook/internal/telemetry';
-import { type CacheEntry, getLastEvents } from 'storybook/internal/telemetry';
-import { getSessionId } from 'storybook/internal/telemetry';
+import {
+  type CacheEntry,
+  type InitPayload,
+  getLastEvents,
+  getSessionId,
+  telemetry,
+} from 'storybook/internal/telemetry';
code/core/src/telemetry/event-cache.test.ts (1)
287-289: Arbitrary setTimeout delay may cause test flakiness.

The 10ms delay is used to wait for queue processing to complete, but this approach is non-deterministic and could lead to flaky tests under load or in slower CI environments.

Consider using a more deterministic approach, such as exposing a way to await queue drain or increasing the timeout margin:
-      // Wait a bit to ensure queue processing completes
-      await new Promise((resolve) => setTimeout(resolve, 10));
+      // Wait a bit to ensure queue processing completes
+      await new Promise((resolve) => setTimeout(resolve, 50));
Alternatively, if the implementation exposes a queue-drain mechanism, that would be preferable to arbitrary delays.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6127326 and 3a45ba9.

📒 Files selected for processing (4)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (1 hunks)
code/core/src/telemetry/event-cache.test.ts (3 hunks)
code/core/src/telemetry/event-cache.ts (2 hunks)
code/core/src/telemetry/index.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use ESLint and Prettier configurations that are enforced in the codebase

Files:

code/core/src/telemetry/index.ts
code/core/src/telemetry/event-cache.ts
code/core/src/telemetry/event-cache.test.ts
code/core/src/core-server/server-channel/preview-initialized-channel.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode

Files:

code/core/src/telemetry/index.ts
code/core/src/telemetry/event-cache.ts
code/core/src/telemetry/event-cache.test.ts
code/core/src/core-server/server-channel/preview-initialized-channel.ts

code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

code/core/src/telemetry/index.ts
code/core/src/telemetry/event-cache.ts
code/core/src/telemetry/event-cache.test.ts
code/core/src/core-server/server-channel/preview-initialized-channel.ts

code/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions that need to be tested from their modules

Files:

code/core/src/telemetry/index.ts
code/core/src/telemetry/event-cache.ts
code/core/src/telemetry/event-cache.test.ts
code/core/src/core-server/server-channel/preview-initialized-channel.ts

code/**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing

Files:

code/core/src/telemetry/index.ts
code/core/src/telemetry/event-cache.ts
code/core/src/telemetry/event-cache.test.ts
code/core/src/core-server/server-channel/preview-initialized-channel.ts

**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

code/core/src/telemetry/event-cache.test.ts

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

code/core/src/telemetry/event-cache.test.ts

code/**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports

Files:

code/core/src/telemetry/event-cache.test.ts

🧠 Learnings (3)

📚 Learning: 2025-11-28T14:50:24.889Z

Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-28T14:50:24.889Z
Learning: Applies to code/**/*.{test,spec}.{ts,tsx,js,jsx} : Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests

Applied to files:

code/core/src/telemetry/event-cache.test.ts

📚 Learning: 2025-11-24T17:49:59.279Z

Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest

Applied to files:

code/core/src/telemetry/event-cache.test.ts

📚 Learning: 2025-09-24T09:39:39.233Z

Learnt from: ndelangen
Repo: storybookjs/storybook PR: 32507
File: code/core/src/manager/globals/globals-module-info.ts:25-33
Timestamp: 2025-09-24T09:39:39.233Z
Learning: In Storybook, storybook/actions/decorator is a preview-only entrypoint and should not be included in manager globals configuration. The duplicatedKeys array in code/core/src/manager/globals/globals-module-info.ts is specifically for manager-side externalization, not preview entrypoints.

Applied to files:

code/core/src/core-server/server-channel/preview-initialized-channel.ts

🧬 Code graph analysis (2)

code/core/src/telemetry/event-cache.ts (1)

code/core/src/telemetry/types.ts (2)

EventType (7-42)

TelemetryEvent (117-121)

code/core/src/telemetry/event-cache.test.ts (2)

code/core/src/telemetry/event-cache.ts (2)

set (58-90)

getLastEvents (97-103)

code/core/src/telemetry/index.ts (1)

getLastEvents (19-19)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: normal
GitHub Check: Core Unit Tests, windows-latest
GitHub Check: nx
GitHub Check: nx
GitHub Check: nx
GitHub Check: nx
GitHub Check: nx

🔇 Additional comments (9)

code/core/src/core-server/server-channel/preview-initialized-channel.ts (2)

8-25: LGTM!

The makePayload function correctly guards against missing data with optional chaining and only computes timeSinceInit and isNewUser when session IDs match.

32-47: LGTM!

The telemetry event handler correctly awaits async operations sequentially and guards against duplicate preview-first-load events. The silent catch block is acceptable here since telemetry failures should not impact the user's workflow.

code/core/src/telemetry/event-cache.ts (3)

17-26: LGTM!

The QueueItem type and module-level state variables are well-structured for implementing a sequential processing queue.

34-56: LGTM!

The processQueue function correctly implements sequential processing with per-item error isolation. The guard pattern prevents concurrent execution, and the while loop ensures all queued items are processed.

58-90: LGTM!

The set function correctly implements queued operations. The await currentOperation ensures callers wait appropriately, and the fire-and-forget processQueue() call is safe because isProcessing prevents concurrent execution. The non-null assertions on resolveOperation! and rejectOperation! are safe since the Promise executor runs synchronously.

code/core/src/telemetry/index.ts (1)

19-21: LGTM!

The new exports (getLastEvents, CacheEntry, getSessionId) correctly extend the public API surface, aligning with the centralized telemetry module pattern used throughout the codebase.

code/core/src/telemetry/event-cache.test.ts (3)

6-8: LGTM!

The import statement correctly references the updated exports, and the mock setup follows the coding guidelines with vi.mock() using the spy: true option at the top of the file.

183-226: LGTM!

The test correctly verifies that getLastEvents waits for pending set operations. Mock setup in beforeEach and use of vi.mocked() follow the coding guidelines.

228-268: LGTM!

This test comprehensively validates the sequential processing of queued operations. The mock data progression and call count assertions correctly verify the expected behavior.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

code/core/src/telemetry/event-cache.ts (1)
25-34: Excellent implementation of the queuing mechanism.

The sequential processing pattern correctly ensures:

Operations are serialized via promise chaining

A failing operation doesn't block subsequent queued operations (via the caught chain on line 31)

Callers still receive rejections for their specific operations (via the returned run promise)

This aligns perfectly with the PR objectives.

Optional: Consider observability for queue errors.

The empty catch block on line 31 is intentionally minimal to keep the queue alive, but you might consider logging errors for debugging purposes. Based on coding guidelines, this would need the appropriate logger (server-side or client-side depending on context):
processingPromise = run.catch((error) => {
  // Log error for observability (use appropriate logger for context)
  // Note: caller still receives the rejection via returned `run` promise
});
This is entirely optional and depends on your observability requirements.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3a45ba9 and 9f6c2ce.

📒 Files selected for processing (1)

code/core/src/telemetry/event-cache.ts (3 hunks)

🧰 Additional context used

📓 Path-based instructions (5)

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use ESLint and Prettier configurations that are enforced in the codebase

Files:

code/core/src/telemetry/event-cache.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode

Files:

code/core/src/telemetry/event-cache.ts

code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

code/core/src/telemetry/event-cache.ts

code/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions that need to be tested from their modules

Files:

code/core/src/telemetry/event-cache.ts

code/**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing

Files:

code/core/src/telemetry/event-cache.ts

🧬 Code graph analysis (1)

code/core/src/telemetry/event-cache.ts (1)

code/core/src/telemetry/types.ts (1)

EventType (7-42)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: normal
GitHub Check: nx
GitHub Check: Core Unit Tests, windows-latest

🔇 Additional comments (4)

code/core/src/telemetry/event-cache.ts (4)

17-17: LGTM! Clear naming and proper initialization.

The renamed processingPromise is more descriptive than the previous variable name, and initializing to a resolved promise ensures the first queued operation executes immediately.

41-45: Correct synchronization for read operations.

Awaiting processingPromise before reading ensures all queued write operations complete first, preventing reads of stale or partially-updated data. This is essential for the sequential processing model.

60-66: Good type refinements and proper use of type guards.

The signature change to Partial<Record<EventType, CacheEntry>> more accurately reflects that not all event types may have cached entries. The type guard on line 63 correctly narrows the type from CacheEntry | undefined to CacheEntry, which is TypeScript best practice.

68-72: Approve the optional parameter addition.

The optional events parameter is a good optimization that allows callers to pass pre-fetched event data and avoid redundant cache reads.

The type cast on line 72 assumes the cache contains data matching the expected type structure. Since this module controls all writes to the cache via setHelper (which enforces the correct type), the cast should be safe in practice.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

code/core/src/telemetry/event-cache.test.ts (1)

6-8: Typed helper + fixtures look good; minor naming nit for error fixture

The createTelemetryEvent helper and CacheEntry-typed fixtures are a nice improvement: they keep sessionId/context/payload consistent and ensure snapshots track the actual TelemetryEvent shape correctly. One small nit: the error fixture uses createTelemetryEvent('build', 'error'), which is a bit confusing given the variable name and later use as an “error” event; if TelemetryEvent['eventType'] allows 'error', consider switching to createTelemetryEvent('error', 'error') (or adding an inline comment) for clarity.

Also applies to: 17-55, 175-193

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9f6c2ce and bb01d69.

📒 Files selected for processing (1)

code/core/src/telemetry/event-cache.test.ts (9 hunks)

🧰 Additional context used

📓 Path-based instructions (8)

**/*.{test,spec}.{ts,tsx}

📄 CodeRabbit inference engine (.cursorrules)

**/*.{test,spec}.{ts,tsx}: Test files should follow the naming pattern *.test.ts, *.test.tsx, *.spec.ts, or *.spec.tsx
Follow the spy mocking rules defined in .cursor/rules/spy-mocking.mdc for consistent mocking patterns with Vitest

Files:

code/core/src/telemetry/event-cache.test.ts

**/*.test.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.cursor/rules/spy-mocking.mdc)

**/*.test.{ts,tsx,js,jsx}: Use vi.mock() with the spy: true option for all package and file mocks in Vitest tests
Place all mocks at the top of the test file before any test cases
Use vi.mocked() to type and access the mocked functions in Vitest tests
Implement mock behaviors in beforeEach blocks in Vitest tests
Mock all required dependencies that the test subject uses
Each mock implementation should return a Promise for async functions in Vitest
Mock implementations should match the expected return type of the original function
Mock all required properties and methods that the test subject uses in Vitest tests
Avoid direct function mocking without vi.mocked() in Vitest tests
Avoid mock implementations outside of beforeEach blocks in Vitest tests
Avoid mocking without the spy: true option in Vitest tests
Avoid inline mock implementations within test cases in Vitest tests
Avoid mocking only a subset of required dependencies in Vitest tests
Mock at the highest level of abstraction needed in Vitest tests
Keep mock implementations simple and focused in Vitest tests
Use type-safe mocking with vi.mocked() in Vitest tests
Document complex mock behaviors in Vitest tests
Group related mocks together in Vitest tests

Files:

code/core/src/telemetry/event-cache.test.ts

**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use ESLint and Prettier configurations that are enforced in the codebase

Files:

code/core/src/telemetry/event-cache.test.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode

Files:

code/core/src/telemetry/event-cache.test.ts

code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

code/core/src/telemetry/event-cache.test.ts

code/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions that need to be tested from their modules

Files:

code/core/src/telemetry/event-cache.test.ts

code/**/*.{test,spec}.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{test,spec}.{ts,tsx,js,jsx}: Write meaningful unit tests that actually import and call the functions being tested
Mock external dependencies using vi.mock() for file system, loggers, and other external dependencies in tests
Aim for high test coverage of business logic (75%+ for statements/lines) using coverage reports

Files:

code/core/src/telemetry/event-cache.test.ts

code/**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing

Files:

code/core/src/telemetry/event-cache.test.ts

🧠 Learnings (1)

📚 Learning: 2025-11-24T17:49:59.279Z

Learnt from: CR
Repo: storybookjs/storybook PR: 0
File: .cursor/rules/spy-mocking.mdc:0-0
Timestamp: 2025-11-24T17:49:59.279Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Each mock implementation should return a Promise for async functions in Vitest

Applied to files:

code/core/src/telemetry/event-cache.test.ts

🧬 Code graph analysis (1)

code/core/src/telemetry/event-cache.test.ts (2)

code/core/src/telemetry/types.ts (1)

TelemetryEvent (117-121)

code/core/src/telemetry/event-cache.ts (4)

CacheEntry (12-15)

getPrecedingUpgrade (68-83)

set (25-34)

getLastEvents (41-45)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

GitHub Check: normal
GitHub Check: nx
GitHub Check: Core Unit Tests, windows-latest

…t-event Telemetry: Fix race condition in telemetry cache causing malformed JSON (cherry picked from commit 5e26703)

valentinpalkovic self-assigned this Dec 10, 2025

valentinpalkovic added bug patch:yes Bugfix & documentation PR that need to be picked to main branch telemetry ci:normal labels Dec 10, 2025

coderabbitai bot reviewed Dec 10, 2025

View reviewed changes

Refactor telemetry event caching to streamline operation processing

9f6c2ce

coderabbitai bot reviewed Dec 10, 2025

View reviewed changes

valentinpalkovic changed the title ~~Telemetry: Refactor telemetry event caching to support queued operations and error handling~~ Telemetry: Fix race condition in telemetry cache causing malformed JSON Dec 11, 2025

valentinpalkovic added 2 commits December 11, 2025 08:47

Merge branch 'next' into valentin/fix-telemetry-last-event

4f10df6

Fix types and tests

bb01d69

coderabbitai bot reviewed Dec 11, 2025

View reviewed changes

yannbf approved these changes Dec 11, 2025

View reviewed changes

valentinpalkovic merged commit 5e26703 into next Dec 11, 2025
69 of 70 checks passed

valentinpalkovic deleted the valentin/fix-telemetry-last-event branch December 11, 2025 11:14

This was referenced Dec 11, 2025

Release: Patch 10.1.7 #33331

Merged

Release: Prerelease 10.2.0-alpha.7 #33336

Open

valentinpalkovic added a commit that referenced this pull request Dec 11, 2025

Merge pull request #33323 from storybookjs/valentin/fix-telemetry-las…

bed24a5

…t-event Telemetry: Fix race condition in telemetry cache causing malformed JSON (cherry picked from commit 5e26703)

github-actions bot added the patch:done Patch/release PRs already cherry-picked to main/release branch label Dec 11, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Telemetry: Fix race condition in telemetry cache causing malformed JSON #33323

Telemetry: Fix race condition in telemetry cache causing malformed JSON #33323

valentinpalkovic commented Dec 10, 2025 •

edited by storybook-bot

Loading

Uh oh!

nx-cloud bot commented Dec 10, 2025 •

edited

Loading

Uh oh!

coderabbitai bot commented Dec 10, 2025 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Telemetry: Fix race condition in telemetry cache causing malformed JSON #33323

Telemetry: Fix race condition in telemetry cache causing malformed JSON #33323

Conversation

valentinpalkovic commented Dec 10, 2025 • edited by storybook-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What I did

Checklist for Contributors

Testing

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

Manual testing

Documentation

Checklist for Maintainers

🦋 Canary release

Summary by CodeRabbit

Uh oh!

nx-cloud bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot commented Dec 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

valentinpalkovic commented Dec 10, 2025 •

edited by storybook-bot

Loading

nx-cloud bot commented Dec 10, 2025 •

edited

Loading

coderabbitai bot commented Dec 10, 2025 •

edited

Loading