Skip to content

Addon-Vitest: Prevent manager "Server timed out" during large test runs#35400

Closed
ndelangen wants to merge 2 commits into
nextfrom
norbert/fix-timeout-vitest-addon
Closed

Addon-Vitest: Prevent manager "Server timed out" during large test runs#35400
ndelangen wants to merge 2 commits into
nextfrom
norbert/fix-timeout-vitest-addon

Conversation

@ndelangen

@ndelangen ndelangen commented Jul 7, 2026

Copy link
Copy Markdown
Member

Closes #

What I did

Problem: On large storybooks (thousands of stories), running the full component test suite via @storybook/addon-vitest makes the manager show a scary "Server timed out — Please restart your Storybook server and reload the page" toast, even though the dev server is healthy and Vitest keeps running in the background. Reloading the page reconnects.

Root cause: The toast maps to the manager WebSocket closing with code 3008 (reason: "timeout"). During a run, addon-vitest flushes batched test-status updates onto the dev-server → manager WebSocket every ~500ms. On large projects this keeps the channel saturated, and the manager's onmessage handler processes messages one at a time, synchronously running the full channel handler before resetting the heartbeat. With a big enough backlog, a heartbeat ping isn't processed within the 20s window (HEARTBEAT_INTERVAL 15s + HEARTBEAT_MAX_LATENCY 5s), so the client-side watchdog closes a perfectly healthy connection.

This failure mode was already anticipated in a comment in test-manager.ts, but the fixed 500ms throttle isn't enough at scale.

The fix has two parts:

Area Change Why
Core channels (WebsocketTransport) Handle heartbeat ping before invoking the channel handler and return early Decouples heartbeat reset from channel-handler latency, so a busy channel can no longer starve pings. Fixes the whole class of "busy channel" disconnects, not just addon-vitest.
Addon-Vitest (TestManager) Make the status-flush interval configurable via the STORYBOOK_TEST_STATUS_FLUSH_INTERVAL env var (default unchanged at 500) Lets very large projects reduce WebSocket pressure without a per-repo patch.

Notes for reviewers:

  • The default flush interval is unchanged (500ms) — the core ping prioritization is the actual root-cause fix, so there's no UX regression (sidebar/status update frequency) for normal projects.
  • Pings have no channel listeners, so no longer forwarding them to the handler is a no-op behaviorally (verified) besides the intended heartbeat decoupling.
  • The env var crosses into the Vitest child process automatically via the existing extendEnv inheritance, so no extra plumbing (or main.ts option) was needed.

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

Caution

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

Reproducing the original bug requires a large storybook, so the most reliable manual check uses one with 1000+ stories (the original report was ~3579).

  1. Check out this branch and build core + the addon: yarn && yarn task compile.
  2. Point Storybook at a large project (1000+ test-tagged stories) using @storybook/nextjs-vite (or any Vite framework) with @storybook/addon-vitest enabled, and run storybook dev.
  3. Open the manager, go to the Test widget, and Run all component tests (enable interactions + coverage for max channel traffic).
  4. Let the run proceed well past ~60–70% progress (where the disconnect previously occurred).
  5. Expected: No "Server timed out" toast appears, and the Test widget progress keeps updating live until completion. In DevTools → Network → WS, the manager socket (.../storybook-server-channel) should not close with code 3008.
  6. Escape hatch check (optional): Start with STORYBOOK_TEST_STATUS_FLUSH_INTERVAL=2000 storybook dev and confirm tests still run and statuses still update (just less frequently).

Areas most worth extra scrutiny: the WebsocketTransport.onmessage change is in shared core channel code used by both manager and preview, so verify normal (small) storybooks still connect, receive live HMR/story updates, and reconnect after a server restart.

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

  • Declare whether manual QA will be needed for this PR during the next release, through qa:needed or qa:skip

  • 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 publish.yml --field pr=<PR_NUMBER>

Made with Cursor

Summary by CodeRabbit

  • New Features

    • Added configurable batching for Vitest test status updates, with a default interval and an environment variable override.
  • Bug Fixes

    • Improved WebSocket heartbeat handling so keep-alive pings are responded to immediately and no longer get delayed by other message processing.
    • Reduced the chance of test runs timing out by spacing out status update traffic more effectively.
  • Documentation

    • Added guidance for resolving “Server timed out” during large Vitest runs.

On large storybooks, a running test suite keeps the dev-server WebSocket
saturated with status updates, delaying heartbeat processing in the manager
until it disconnects with code 3008 ("Server timed out") even though the dev
server is healthy and tests keep running.

- Core channels: process heartbeat pings before the (potentially slow) channel
  handler in WebsocketTransport, so a busy channel can no longer starve pings.
- Addon-Vitest: make the test-case result flush interval configurable via the
  STORYBOOK_TEST_STATUS_FLUSH_INTERVAL env var (default 500ms) to reduce channel
  pressure on very large projects.

Co-authored-by: Cursor <cursoragent@cursor.com>
@ndelangen ndelangen added bug ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. labels Jul 7, 2026
@ndelangen ndelangen self-assigned this Jul 7, 2026
@ndelangen ndelangen marked this pull request as ready for review July 8, 2026 15:09
Copilot AI review requested due to automatic review settings July 8, 2026 15:09
@ndelangen ndelangen requested review from JReinhold and Sidnioulz July 8, 2026 15:10
@coderabbitai

coderabbitai Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a configurable flush interval for batched Vitest test-case results, controllable via an environment variable, with corresponding constants, TestManager wiring, and tests. Separately, reorders WebSocket onmessage handling so ping events trigger heartbeat reset and pong reply before channel dispatch, with new tests.

Changes

Configurable Test-Status Flush Interval

Layer / File(s) Summary
Flush interval constants
code/addons/vitest/src/constants.ts
Adds DEFAULT_TEST_STATUS_FLUSH_INTERVAL and TEST_STATUS_FLUSH_INTERVAL_ENV_VAR exported constants with JSDoc.
TestManager throttled flush
code/addons/vitest/src/node/test-manager.ts, code/addons/vitest/src/node/test-manager.test.ts
Adds flushTestCaseResultsInterval option, refactors flush logic into a flushTestCaseResults method wrapped by throttle using the configurable interval, and adds tests for leading/batched/trailing-edge flush behavior.
Environment override and docs
code/addons/vitest/src/node/vitest.ts, docs/writing-tests/integrations/vitest-addon/index.mdx
Parses TEST_STATUS_FLUSH_INTERVAL_ENV_VAR into a validated numeric interval passed to TestManager, and documents STORYBOOK_TEST_STATUS_FLUSH_INTERVAL in FAQ docs.

Estimated code review effort: 2 (Simple) | ~12 minutes

WebSocket Heartbeat Ping Handling

Layer / File(s) Summary
Ping-first heartbeat handling
code/core/src/channels/websocket/index.ts, code/core/src/channels/websocket/index.test.ts
Reorders onmessage so ping events trigger heartbeat reset and pong reply with early return, no longer forwarding pings to the channel handler; adds tests for timeout, reset, suppression, forwarding, and handler-throw resilience.

Estimated code review effort: 2 (Simple) | ~10 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Vitest
  participant TestManager
  participant ComponentTestStatusStore

  Vitest->>TestManager: onTestCaseResult(result)
  TestManager->>TestManager: throttledFlushTestCaseResults()
  alt leading edge or interval elapsed
    TestManager->>ComponentTestStatusStore: set(results)
  else within interval
    TestManager-->>TestManager: batch pending result
  end
Loading
sequenceDiagram
  participant Server
  participant WebsocketTransport
  participant ChannelHandler

  Server->>WebsocketTransport: onmessage(event)
  alt event is ping
    WebsocketTransport->>WebsocketTransport: reset heartbeat timer
    WebsocketTransport->>Server: send pong
  else other event
    WebsocketTransport->>ChannelHandler: handler(event)
  end
Loading
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
code/core/src/channels/websocket/index.test.ts (1)

51-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Missing vi.unstubAllGlobals() cleanup for the stubbed WebSocket global.

vi.stubGlobal('WebSocket', MockWebSocket) is set once at module scope but never unstubbed. Per Vitest docs, stubbed globals persist across tests/files unless vi.unstubAllGlobals() is called (or the unstubGlobals config option is set). This risks leaking the mocked WebSocket constructor into other test files sharing the same worker.

🧹 Proposed fix
   afterEach(() => {
     vi.useRealTimers();
     vi.clearAllMocks();
+    vi.unstubAllGlobals();
   });

As per coding guidelines, "In tests, never assign ambient globals directly (for example globalThis.*, global.fetch, or globalThis.window); use vi.stubGlobal and vi.unstubAllGlobals() instead."

Also applies to: 74-77

🤖 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/channels/websocket/index.test.ts` at line 51, The WebSocket
global stub in the websocket tests is never cleaned up, so it can leak into
other tests in the same worker. Update the test setup around the module-scope
`vi.stubGlobal('WebSocket', MockWebSocket)` in `index.test.ts` and add
`vi.unstubAllGlobals()` in the appropriate teardown path (for example, a
matching `afterEach`/`afterAll` near the `MockWebSocket` test setup). Make sure
any other related global stubs in the same test block are also restored so the
`MockWebSocket` setup does not persist beyond this file.

Source: Coding guidelines

code/addons/vitest/src/node/vitest.ts (1)

43-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider warning on invalid STORYBOOK_TEST_STATUS_FLUSH_INTERVAL values.

Invalid/negative/non-numeric values silently fall back to the default with no indication to the user. Since this variable exists specifically to help diagnose large-run WebSocket saturation, a silent fallback could make misconfiguration hard to notice.

As per coding guidelines, "Use Storybook loggers (storybook/internal/node-logger on the server ...) instead of raw console.* in normal code paths."

♻️ Proposed diagnostic
+import { logger } from 'storybook/internal/node-logger';
+
 const parsedFlushInterval = Number(process.env[TEST_STATUS_FLUSH_INTERVAL_ENV_VAR]);
 const flushTestCaseResultsInterval =
-  Number.isFinite(parsedFlushInterval) && parsedFlushInterval > 0 ? parsedFlushInterval : undefined;
+  Number.isFinite(parsedFlushInterval) && parsedFlushInterval > 0 ? parsedFlushInterval : undefined;
+
+if (process.env[TEST_STATUS_FLUSH_INTERVAL_ENV_VAR] && flushTestCaseResultsInterval === undefined) {
+  logger.warn(
+    `Ignoring invalid ${TEST_STATUS_FLUSH_INTERVAL_ENV_VAR} value; expected a positive number.`
+  );
+}
🤖 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/addons/vitest/src/node/vitest.ts` around lines 43 - 46, The flush
interval parsing in vitest.ts silently ignores invalid
STORYBOOK_TEST_STATUS_FLUSH_INTERVAL values, which makes misconfiguration hard
to spot. Update the logic around parsedFlushInterval and
flushTestCaseResultsInterval to detect non-numeric, zero, or negative inputs and
emit a warning using the Storybook node logger instead of falling back quietly.
Keep the existing defaulting behavior for valid positive numbers, but add a
clear diagnostic that includes the TEST_STATUS_FLUSH_INTERVAL_ENV_VAR name and
the bad value.

Source: Coding guidelines

🤖 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.

Nitpick comments:
In `@code/addons/vitest/src/node/vitest.ts`:
- Around line 43-46: The flush interval parsing in vitest.ts silently ignores
invalid STORYBOOK_TEST_STATUS_FLUSH_INTERVAL values, which makes
misconfiguration hard to spot. Update the logic around parsedFlushInterval and
flushTestCaseResultsInterval to detect non-numeric, zero, or negative inputs and
emit a warning using the Storybook node logger instead of falling back quietly.
Keep the existing defaulting behavior for valid positive numbers, but add a
clear diagnostic that includes the TEST_STATUS_FLUSH_INTERVAL_ENV_VAR name and
the bad value.

In `@code/core/src/channels/websocket/index.test.ts`:
- Line 51: The WebSocket global stub in the websocket tests is never cleaned up,
so it can leak into other tests in the same worker. Update the test setup around
the module-scope `vi.stubGlobal('WebSocket', MockWebSocket)` in `index.test.ts`
and add `vi.unstubAllGlobals()` in the appropriate teardown path (for example, a
matching `afterEach`/`afterAll` near the `MockWebSocket` test setup). Make sure
any other related global stubs in the same test block are also restored so the
`MockWebSocket` setup does not persist beyond this file.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 10a13e55-2cc5-403b-9d13-d0a67304a761

📥 Commits

Reviewing files that changed from the base of the PR and between b897c04 and f52b788.

📒 Files selected for processing (7)
  • code/addons/vitest/src/constants.ts
  • code/addons/vitest/src/node/test-manager.test.ts
  • code/addons/vitest/src/node/test-manager.ts
  • code/addons/vitest/src/node/vitest.ts
  • code/core/src/channels/websocket/index.test.ts
  • code/core/src/channels/websocket/index.ts
  • docs/writing-tests/integrations/vitest-addon/index.mdx

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses false “Server timed out” disconnects in the Storybook manager during large @storybook/addon-vitest runs by ensuring WebSocket heartbeat ping messages are handled promptly (and not delayed by heavy channel handlers), and by allowing large projects to reduce channel pressure via a configurable status-flush interval.

Changes:

  • Prioritize heartbeat ping handling in WebsocketTransport (reset heartbeat + reply pong, and don’t forward ping to the channel handler).
  • Add configurable test-status flush throttling to addon-vitest via STORYBOOK_TEST_STATUS_FLUSH_INTERVAL (default remains 500ms).
  • Add unit tests for the heartbeat-under-load behavior and the configurable flush interval; document the new troubleshooting/override behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
docs/writing-tests/integrations/vitest-addon/index.mdx Adds FAQ-style documentation for the timeout symptom and the flush-interval env var.
code/core/src/channels/websocket/index.ts Handles ping before running channel handlers to avoid heartbeat starvation under load.
code/core/src/channels/websocket/index.test.ts Adds regression coverage for timeout/heartbeat/ping-forwarding behavior.
code/addons/vitest/src/node/vitest.ts Reads STORYBOOK_TEST_STATUS_FLUSH_INTERVAL and passes it into TestManager.
code/addons/vitest/src/node/test-manager.ts Makes the status-flush throttling interval configurable (defaults unchanged).
code/addons/vitest/src/node/test-manager.test.ts Adds coverage verifying throttling respects the configured interval.
code/addons/vitest/src/constants.ts Introduces constants for the default flush interval and env var name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.


On very large projects (thousands of stories), a running test suite streams a high volume of status updates from the dev server to the Storybook manager over a WebSocket. If that channel stays saturated, the manager's heartbeat can be delayed enough to disconnect and show a "Server timed out" notification, even though the dev server is healthy and the tests keep running in the background. Reloading the page reconnects the manager.

To reduce the pressure on the channel, you can increase how often test-case results are batched before being flushed to the UI by setting the `STORYBOOK_TEST_STATUS_FLUSH_INTERVAL` environment variable (in milliseconds) before starting Storybook. The default is `500`; larger values (for example `2000`) reduce WebSocket traffic at the cost of less frequent sidebar and status updates during a run:
@Sidnioulz

Copy link
Copy Markdown
Contributor

Superseded by #35422

@Sidnioulz Sidnioulz closed this Jul 9, 2026
@Sidnioulz Sidnioulz added the upgrade:10.5 Issues/PRs found during 10.5 upgrade QA and post-release regressions label Jul 9, 2026
@storybook-app-bot

Copy link
Copy Markdown

Package Benchmarks

Commit: 5d87183, ran on 9 July 2026 at 10:44:37 UTC

The following packages have significant changes to their size or dependencies:

storybook

Before After Difference
Dependency count 73 72 🎉 -1 🎉
Self size 21.95 MB 21.95 MB 🎉 -8 KB 🎉
Dependency size 36.65 MB 36.44 MB 🎉 -213 KB 🎉
Bundle Size Analyzer Link Link

@storybook/cli

Before After Difference
Dependency count 205 204 🎉 -1 🎉
Self size 825 KB 821 KB 🎉 -4 KB 🎉
Dependency size 92.16 MB 91.94 MB 🎉 -221 KB 🎉
Bundle Size Analyzer Link Link

@storybook/codemod

Before After Difference
Dependency count 198 197 🎉 -1 🎉
Self size 32 KB 32 KB 0 B
Dependency size 90.64 MB 90.42 MB 🎉 -221 KB 🎉
Bundle Size Analyzer Link Link

create-storybook

Before After Difference
Dependency count 74 73 🎉 -1 🎉
Self size 1.09 MB 1.09 MB 🎉 -225 B 🎉
Dependency size 58.61 MB 58.39 MB 🎉 -221 KB 🎉
Bundle Size Analyzer node node

pull Bot pushed a commit to SimenB/storybook that referenced this pull request Jul 9, 2026
Fixes the "Server timed out" manager disconnect on large addon-vitest
runs (see storybookjs#35400) at its root, in the shared channel transport rather
than only reordering ping handling: any incoming message proves the
connection is alive, so the heartbeat now resets before the (possibly
slow) channel handler runs for every message, not only for a literal
ping. This also closes a gap the narrower ping-only fix leaves open -
if a backlog is deep enough that the next ping itself is stuck behind
many other queued messages, resetting only on ping doesn't help, since
nothing about processing those other messages pushes the deadline out.
kylegach pushed a commit that referenced this pull request Jul 10, 2026
Fixes the "Server timed out" manager disconnect on large addon-vitest
runs (see #35400) at its root, in the shared channel transport rather
than only reordering ping handling: any incoming message proves the
connection is alive, so the heartbeat now resets before the (possibly
slow) channel handler runs for every message, not only for a literal
ping. This also closes a gap the narrower ping-only fix leaves open -
if a backlog is deep enough that the next ping itself is stuck behind
many other queued messages, resetting only on ping doesn't help, since
nothing about processing those other messages pushes the deadline out.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug ci:normal Run our default set of CI jobs (choose this for most PRs). qa:skip Pull Requests that do not need any QA. upgrade:10.5 Issues/PRs found during 10.5 upgrade QA and post-release regressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants