Skip to content

PT-4179: fix Storybook websocket-timeout crash for renderer stories - #2605

Merged
katherinejensen00 merged 3 commits into
mainfrom
pt-4179-storybook-renderer-story-stubs
Jul 27, 2026
Merged

PT-4179: fix Storybook websocket-timeout crash for renderer stories#2605
katherinejensen00 merged 3 commits into
mainfrom
pt-4179-storybook-renderer-story-stubs

Conversation

@katherinejensen00

@katherinejensen00 katherinejensen00 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Renderer app components (the startup-wizard shell/steps, plus dialogs and overlays) import useLocalizedStrings from @renderer/hooks/papi-hooks and otherwise reach the network service. Storybook has no PAPI backend, so the renderer RpcClient's connection attempt to ws://localhost:<port> never settles and its AsyncVariable('websocket connected') rejects unhandled after ~10s — Timeout reached when waiting for websocket connected to settle — which the dev overlay surfaces as a crash on every startup-wizard story. Storybook previously stubbed only @papi/* imports, never the @renderer/@shared paths these renderer components actually use.

This is split out of the PT-4179 sync-progress work as a standalone fix because it is shared Storybook infra, not step-specific: it also unblocks the sibling wizard PRs (PT-4176 language, PT-4177 registration, PT-4178 sync-consent), which all render FirstRunShell and hit the same crash. Landing it on main lets those PRs inherit it on rebase.

Why review this

Supports the startup-wizard epic: the wizard step stories currently crash in Storybook, blocking visual review of PT-4176/4177/4178/4179. This restores them (and other renderer-component stories) to a working, backend-free state.

Changes

  • .storybook/papi-stubs/renderer-papi-hooks.ts (new) — re-exports the real renderer hooks but overrides only useLocalizedStrings to resolve real English strings synchronously (no network).
  • .storybook/papi-stubs/rpc-handler.factory.ts (new) — inert RPC handler so networkService.initialize() succeeds with no socket and no timer; no connection is ever attempted, so nothing rejects.
  • .storybook/main.ts — wires both via NormalModuleReplacementPlugin.
  • src/renderer/components/notification-display.test.tsx — add afterEach(() => { act(() => { toast.dismiss(); }); }) to cancel Sonner's pending auto-dismiss timers before jsdom tears down. Without this, a 4 s Sonner timer fires after the test environment is torn down, causing window is not defined to surface as an unhandled Vitest error that exits the whole test run with code 1 (all 1191 tests pass, but the process fails). This is a pre-existing race condition introduced in the PR that added Sonner (PT-4193/PT-4193: Add secondary notification action, position, and dismissible #2561); fixing it here unblocks CI on this PR.

Both Storybook stub files use NormalModuleReplacementPlugin, not resolve.alias: the base renderer webpack config resolves @renderer/@shared via TsconfigPathsPlugin, which wins over resolve.alias (that is why the existing @papi/* aliases work, but a @renderer/... alias would be silently ignored). The plugin rewrites the request in beforeResolve, before TsconfigPaths runs.

AI Involvement

AI-assisted. Claude diagnosed the root cause (traced the unhandled rejection from useLocalizedStringsdataProviderService.getnetworkService.initializeRpcClient), wrote the two stubs and the main.ts wiring, and verified the result. Claude also diagnosed the CI test failure (Sonner timer leak in notification-display.test.tsx) and wrote the afterEach fix. The author reviewed the diagnosis, the stub approach, and the diffs.

Testing

  • npm run storybook:build passes (the @papi-free hard gate) with both stubs.
  • Rendered SyncProgressStep (Default, All Done) and FirstRunShell (Language) against the production build: real English strings render and zero unhandled rejections (was: Timeout reached when waiting for websocket connected to settle).
  • Author confirmed the crash is gone in local dev Storybook.
  • notification-display.test.tsx locally: timer leak no longer surfaces as an unhandled error.

Risk Level

Low — Storybook config only (no renderer production code changed); NormalModuleReplacementPlugin rewrites only the two targeted module paths. The notification-display test fix is additive (adds afterEach cleanup only). If either stub is wrong, storybook:build fails cleanly rather than silently.


This change is Reviewable

katherinejensen00 and others added 2 commits July 24, 2026 10:28
Renderer app components (the startup-wizard shell/steps, plus dialogs and
overlays) import `useLocalizedStrings` from `@renderer/hooks/papi-hooks` and
otherwise reach the network service. Storybook has no PAPI backend, so the
renderer `RpcClient`'s connection attempt to `ws://localhost:<port>` never
settles and its `AsyncVariable('websocket connected')` rejects unhandled
after ~10s ("Timeout reached when waiting for websocket connected to
settle"), which the dev overlay surfaces as a crash on every startup-wizard
story. Storybook previously only stubbed `@papi/*` imports, never the
`@renderer`/`@shared` paths these renderer components actually use.

Add two Storybook-only module replacements:
- `papi-stubs/renderer-papi-hooks.ts`: re-exports the real hooks but overrides
  `useLocalizedStrings` to resolve real English strings synchronously (no
  connection).
- `papi-stubs/rpc-handler.factory.ts`: inert RPC handler so
  `networkService.initialize()` succeeds with no socket and no timer, so no
  connection is ever attempted and nothing rejects.

Both are wired via `NormalModuleReplacementPlugin`, not `resolve.alias`: the
base renderer webpack config resolves `@renderer`/`@shared` via
`TsconfigPathsPlugin`, which wins over `resolve.alias` (that is why the
existing `@papi/*` aliases work but a `@renderer/...` alias would be ignored).

Verified with `npm run storybook:build` and by rendering the first-run stories
(real English strings, zero unhandled rejections).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sonner's auto-dismiss timer fires after jsdom tears down, causing
a "window is not defined" unhandled error that fails the test run
even though all 1191 tests pass. Add an afterEach that calls
toast.dismiss() (wrapped in act) to cancel all pending Sonner
timers before the test environment is torn down.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@jolierabideau jolierabideau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me, other than the lint failures!

@jolierabideau reviewed 4 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on katherinejensen00).


.storybook/papi-stubs/renderer-papi-hooks.ts line 2 at r2 (raw file):

/**
 * Storybook stub for the renderer PAPI-hooks barrel (`@renderer/hooks/papi-hooks`).

NIT From Claude

renderer-papi-hooks.ts re-exports the 11 non-localization hooks from src/renderer/hooks/papi-hooks/index.ts by hand. I verified they match exactly today. But since the $-anchored replacement redirects the whole barrel to this stub, a hook added to the real barrel later would be missing here, and any story importing it would fail to resolve. It'd surface as a Storybook build error (not a runtime bug), so it's low-severity — but a one-line comment on the real barrel pointing at this stub as the sync target (or a type-level assertion that the stub covers the barrel) would make the coupling discoverable.

…nused-vars)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@jolierabideau jolierabideau left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

:lgtm:

@jolierabideau reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved.

@katherinejensen00
katherinejensen00 merged commit ef59053 into main Jul 27, 2026
7 checks passed
@katherinejensen00
katherinejensen00 deleted the pt-4179-storybook-renderer-story-stubs branch July 27, 2026 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants