Skip to content

fix(onboarding): make cache.disposables safe to call after unmount - #76409

Draft
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixonboarding-guard-wizard-sync-40f9f5
Draft

fix(onboarding): make cache.disposables safe to call after unmount#76409
posthog[bot] wants to merge 1 commit into
masterfrom
posthog-self-driving/fixonboarding-guard-wizard-sync-40f9f5

Conversation

@posthog

@posthog posthog Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Problem

  • The onboarding wizard's SSE reconnect crashes with a TypeError when an error event fires around unmount, killing that user's reconnect loop mid-onboarding.
  • wizardSessionStreamLogic's eventSource.onerror schedules its backoff reconnect via cache.disposables.add(...), but beforeUnmount in the disposables plugin (frontend/src/kea-disposables.ts) sets cache.disposables = null.
  • eventSource.close() runs as part of the same unmount pass, but a queued error event can still dispatch afterward — it hits the null manager and throws instead of scheduling a reconnect.
  • The same unguarded cache.disposables pattern exists at four other call sites in that file, so any EventSource error near unmount can reproduce it.

Changes

  • frontend/src/kea-disposables.ts: beforeUnmount now swaps in a no-op manager instead of null. add()/dispose() become safe by construction for every consumer of the plugin, rather than requiring each call site to remember to guard for it — this fixes the hazard at all the call sites in wizardSessionStreamLogic in one place instead of scattering optional chaining across five call sites.
  • Added a regression test in kea-disposables.test.ts that reproduces the exact failure: a listener holding a raw cache reference (mirroring how EventSource.onerror closes over cache) calling disposables.add()/dispose() after unmount(). Confirmed it fails with the pre-fix null behavior and passes with the fix.

How did you test this code?

  • Added frontend/src/kea-disposables.test.ts: "a callback holding a raw cache reference can call disposables.add()/dispose() after unmount without throwing" — reproduces the reported TypeError: Cannot read properties of null (reading 'add'). Verified it fails on the pre-fix code and passes after the fix.
  • Ran the full kea-disposables.test.ts suite (13 tests) and wizardSessionStreamLogic.test.ts suite (7 tests) — all pass.
  • Ran hogli ci:preflight --fix — clean.

Docs update

N/A — internal bug fix, no user-facing or documented behavior change.

🤖 Agent context

Autonomy: Fully autonomous

Investigated the crash trace in the report (products/wizard/frontend/wizardSessionStreamLogic.ts:280, frontend/src/kea-disposables.ts), confirmed no open PR or branch was already addressing it (gh pr list --search). Considered guarding each of the five cache.disposables call sites in wizardSessionStreamLogic.ts individually with optional chaining, but hardening the plugin's beforeUnmount to swap in a no-op manager fixes the hazard for every consumer at once and is the smaller, more root-cause change. Skills invoked: /using-kea-disposables, /writing-tests.


Created with PostHog Desktop from this inbox report.

A listener holding a raw `cache` reference (e.g. `EventSource.onerror` in `wizardSessionStreamLogic`) can still fire after `logic.unmount()` — a queued error event dispatching after its own cleanup already ran in the same unmount pass. `beforeUnmount` used to null out `cache.disposables`, so that late call threw a `TypeError` instead of scheduling a reconnect, silently killing the wizard's SSE reconnect loop.

Swap in a no-op manager instead of `null` on unmount so `add()`/`dispose()` are safe by construction for every consumer of the plugin, rather than requiring each call site to guard for it individually.

Generated-By: PostHog Code
Task-Id: 1fa832ed-a607-42df-96ff-2612e1cf55ce
@trunk-io

trunk-io Bot commented Aug 2, 2026

Copy link
Copy Markdown

Merging to master in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

🤖 CI report

⚠️ Bundle size — 🔺 +290 B (+0.0%)

Uncompressed size of every built .js bundle, compared against the base branch.

Total: 65.51 MiB · 🔺 +290 B (+0.0%)

No file changed by more than 1000 B.

Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report

Eager graph — within budget

How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.

Root Eager (shipped) Δ vs base Budget
entry (logged-out pages, app bootstrap)
src/index.tsx
1.25 MiB · 22 files no change ███░░░░░░░ 27.7% of 4.51 MiB
authenticated shell (every logged-in page)
src/scenes/AuthenticatedShell.tsx
8.13 MiB · 3,035 files no change ████████░░ 83.8% of 9.71 MiB

🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx

Largest files eagerly shipped from src/index.tsx
Size File
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
24.6 KiB ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js
6.3 KiB ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js
4.5 KiB ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js
3.9 KiB ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js
1.4 KiB ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js
1.3 KiB src/RootErrorBoundary.tsx
912 B ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js
789 B src/scenes/ChunkLoadErrorBoundary.tsx
762 B src/index.tsx
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
Size File
285.6 KiB ../node_modules/.pnpm/posthog-js@1.409.5/node_modules/posthog-js/dist/rrweb.js
267.7 KiB ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js
235.5 KiB src/taxonomy/core-filter-definitions-by-group.json
231.5 KiB ../node_modules/.pnpm/posthog-js@1.409.5/node_modules/posthog-js/dist/module.js
154.3 KiB ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js
126.8 KiB ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js
105.3 KiB src/lib/api.ts
94.7 KiB ../packages/quill/packages/quill/dist/index.js
93.3 KiB ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js
90.6 KiB ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js

Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479

Toolbar bundle — eager 2.19 MiB within budget

What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.

Metric Size Δ vs base Budget
Eager (shipped)
entry + static imports
2.19 MiB · 17 files 🔺 +72 B (+0.0%) ████░░░░░░ 38.3% of 5.72 MiB
Deferred (lazy) 2.08 MiB · 33 files no change n/a — loads on demand
Loader dist/toolbar.js 1.1 KiB no change █░░░░░░░░░ 5.8% of 19.5 KiB
Largest eagerly-shipped chunks
Size File
718.3 KiB dist/toolbar/toolbar-app-FT4UJ3IP.css
551.4 KiB dist/toolbar/chunk-chunk-AGSGCCBT.js
484.6 KiB dist/toolbar/chunk-chunk-T44C2V5C.js
133.6 KiB dist/toolbar/chunk-chunk-KBLX73CM.js
131.8 KiB dist/toolbar/chunk-chunk-T5KY5WYR.js
71.1 KiB dist/toolbar/toolbar-app-6PECMLVI.js
69.0 KiB dist/toolbar/chunk-chunk-27JL52RE.js
35.6 KiB dist/toolbar/chunk-chunk-3JVI3ZTF.js
20.9 KiB dist/toolbar/chunk-chunk-HJ3ZJMTU.js
12.2 KiB dist/toolbar/chunk-chunk-PIK3PADE.js

Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile

Dist folder size — 🔺 +3.1 KiB (+0.0%)

Total size of the built frontend/dist folder (all assets), compared against the base branch.

Total: 1381.31 MiB · 🔺 +3.1 KiB (+0.0%)

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.

0 participants