Guard reader view localStorage access

[posthog]

Changes

Problem

• A few posthog.com visitors get a blank page in the reader view. They lose the whole page, not one preference.
• ReaderViewProvider writes its layout preferences to localStorage with no try/catch. localStorage.setItem('fullWidthContent', ...) runs on every mount, so when storage is full or blocked (Safari private mode, partitioned storage) it throws QuotaExceededError.
• The throw escapes through the React commit phase, so it is not contained to the effect. It stops the render.
• The same file has more unguarded calls: the pinned sidebar key, the background image key, and the reads that hydrate them. Blocked storage throws on read too.

Fix

• Add safeLocalStorageGet, safeLocalStorageSet, and safeLocalStorageRemove to src/lib/utils.ts. Each one catches the failure and returns without a value. This is the behavior src/context/App.tsx already has for its own write.
• Use the helpers for every reader view preference. A visitor with blocked storage keeps the page and loses only the saved preference.

Key	File	Before	After
fullWidthContent	ReaderViewContext.tsx	read + write, unguarded	guarded
reader-sidebar-pinned	ReaderViewContext.tsx	read + write, unguarded	guarded
background-image	ReaderViewContext.tsx	read, write, remove, unguarded	guarded
full-width-content	ReaderView/index.tsx	write, unguarded	guarded

Risk

Note

Behavior does not change when storage works. The helpers only add a catch path, and no call site reads a return value that the guard can change: a caught read returns null, which is what an absent key already returns.

Scope

• 27 localStorage.setItem calls in src/ share this missing guard. This PR fixes the reader view, the one site with reported crashes, and adds the shared helper the other sites can adopt later.

Agent context

• The dead-code option was rejected: full-width-content looks unused in ReaderView/index.tsx, but src/components/Layout/context.tsx reads it, so the write stays and is guarded instead.
• node_modules is not installed in this environment, so the dev server and tsc did not run. The change is not visual. I checked the guard logic with a standalone script that makes getItem, setItem, and removeItem throw, and confirmed no exception escapes.

Checklist

• I've read the docs and/or content style guides.
• Words are spelled using American English
• Use relative URLs for internal links
• I've checked the pages added or changed in the Vercel preview build
• If I moved a page, I added a redirect in vercel.json (no pages moved)

---

Created with PostHog Desktop from this inbox report.

[posthog]

🦔 PostHog Review reviewed this pull request

Found 0 must fix, 1 should fix, 0 consider.

Published 1 finding (view the review).

Resolved comments: 1 left for you

[github-actions]

Deploy preview

Status	Details	Updated (UTC)
🟢 Ready	View preview	Sep 05, 2026 06:22PM

[posthog]

PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏

[posthog]

PostHog Review

Found 1 should fix.

[posthog]

Guard the storage read that runs before ReaderView

Why we think it's a valid issue

• Checked: the mount order of the providers around ReaderViewProvider, the call site of getInitialSiteSettings, the presence of an error boundary, and the other unguarded localStorage reads that run on the same page load.
• Found: getInitialSiteSettings reads storage without a guard at src/context/App.tsx:1706, and the Provider calls it from useIsomorphicLayoutEffect at src/context/App.tsx:1785-1790.
• Found: gatsby-ssr.js:25-33 wraps every page in that Provider, so it is a parent of ReaderViewProvider. React runs all layout effects before any passive effect, so this read runs before the safeLocalStorageGet calls that the PR added at src/components/ReaderView/context/ReaderViewContext.tsx:71 and :110.
• Found: grep for componentDidCatch, getDerivedStateFromError, and ErrorBoundary returns no match in src/. An uncaught throw in the commit phase therefore unmounts the whole tree and gives the blank page.
• Impact: confirmed for the blocked-storage mode only. When the browser denies storage access, getItem throws a SecurityError, src/context/App.tsx:1706 throws first, and the page still goes blank. This happens today, for example when posthog.com runs in a third-party iframe and the browser blocks third-party storage.
• Impact: the PR is not defeated in the mode its description names. A quota failure throws on write, not on read. In that mode src/context/App.tsx:1706 succeeds, updateSiteSettings already catches its own write at src/context/App.tsx:2476-2481, and the reader view guards now catch the write that crashed. The PR fixes that path in full.
• Priority: lowered to should_fix. The defect is real and the fix is one line with the helper this PR adds, but it does not block the PR, which already removes the reported crash. The suggested remedy is also incomplete on its own: src/components/CookieBanner/ToastVersion.tsx:14 reads storage without a guard inside a passive effect, and CookieBannerToast mounts on every page through src/components/Wrapper/index.tsx:41. A regression test that only guards getInitialSiteSettings would still see the tree unmount.

Issue description

The new guard runs inside ReaderViewProvider, but Gatsby first mounts AppProvider. Its layout effect calls getInitialSiteSettings(), which still uses unguarded localStorage.getItem('siteSettings') at src/context/App.tsx:1706. A browser that blocks reads throws there before these ReaderView effects can recover. React then removes the full page, so the blocked-storage failure remains.

Suggested fix

Use safeLocalStorageGet in getInitialSiteSettings() and catch invalid JSON. Use {} when either operation fails. Add a regression test that makes localStorage.getItem throw while AppProvider mounts. Confirm that ReaderView stays mounted with default settings.

Prompt to fix with AI (copy-paste)

## Context
@src/components/ReaderView/context/ReaderViewContext.tsx#L31

<issue_description>
The new guard runs inside `ReaderViewProvider`, but Gatsby first mounts `AppProvider`. Its layout effect calls `getInitialSiteSettings()`, which still uses unguarded `localStorage.getItem('siteSettings')` at `src/context/App.tsx:1706`. A browser that blocks reads throws there before these ReaderView effects can recover. React then removes the full page, so the blocked-storage failure remains.
</issue_description>

<issue_validation>
- **Checked:** the mount order of the providers around `ReaderViewProvider`, the call site of `getInitialSiteSettings`, the presence of an error boundary, and the other unguarded `localStorage` reads that run on the same page load.
- **Found:** `getInitialSiteSettings` reads storage without a guard at `src/context/App.tsx:1706`, and the `Provider` calls it from `useIsomorphicLayoutEffect` at `src/context/App.tsx:1785-1790`.
- **Found:** `gatsby-ssr.js:25-33` wraps every page in that `Provider`, so it is a parent of `ReaderViewProvider`. React runs all layout effects before any passive effect, so this read runs before the `safeLocalStorageGet` calls that the PR added at `src/components/ReaderView/context/ReaderViewContext.tsx:71` and `:110`.
- **Found:** `grep` for `componentDidCatch`, `getDerivedStateFromError`, and `ErrorBoundary` returns no match in `src/`. An uncaught throw in the commit phase therefore unmounts the whole tree and gives the blank page.
- **Impact:** confirmed for the blocked-storage mode only. When the browser denies storage access, `getItem` throws a `SecurityError`, `src/context/App.tsx:1706` throws first, and the page still goes blank. This happens today, for example when posthog.com runs in a third-party iframe and the browser blocks third-party storage.
- **Impact:** the PR is not defeated in the mode its description names. A quota failure throws on write, not on read. In that mode `src/context/App.tsx:1706` succeeds, `updateSiteSettings` already catches its own write at `src/context/App.tsx:2476-2481`, and the reader view guards now catch the write that crashed. The PR fixes that path in full.
- **Priority:** lowered to `should_fix`. The defect is real and the fix is one line with the helper this PR adds, but it does not block the PR, which already removes the reported crash. The suggested remedy is also incomplete on its own: `src/components/CookieBanner/ToastVersion.tsx:14` reads storage without a guard inside a passive effect, and `CookieBannerToast` mounts on every page through `src/components/Wrapper/index.tsx:41`. A regression test that only guards `getInitialSiteSettings` would still see the tree unmount.
</issue_validation>

## Task
Investigate the issue and solve it

<potential_solution>
Use `safeLocalStorageGet` in `getInitialSiteSettings()` and catch invalid JSON. Use `{}` when either operation fails. Add a regression test that makes `localStorage.getItem` throw while `AppProvider` mounts. Confirm that ReaderView stays mounted with default settings.
</potential_solution>

[posthog]

Confirmed — this is a real gap, and I have left the code unchanged so a maintainer can decide how to close it.

What I checked at the current head: the site settings read in the app provider genuinely runs unguarded, and it runs in a layout effect, so it fires before the reader view guards this PR added, which sit in passive effects. The provider wraps every page through the Gatsby SSR wrapper, and there is no error boundary anywhere in src/, so a throw there does take the whole page down. Your scoping caveat also holds: the cookie banner toast reads storage unguarded too and mounts on every page, so guarding the app provider on its own would not actually clear the blank page in blocked-storage mode.

I am escalating rather than fixing for four reasons:

1. The repo's own contributor guide lists src/context/App.tsx under "Ask first" as core window management. An automated change to that file is exactly what the rule prevents.
2. This PR deliberately scoped itself to the reader view, the one surface with reported crashes, and left the other storage call sites to adopt the shared helper later. Pulling the site-wide provider in would widen the PR past that decision.
3. A one-line guard here would be partial protection, not a fix, since the cookie banner still throws on the same page load. The complete answer is a sweep of the remaining unguarded reads, which belongs in its own PR.
4. There is no component test setup in this repo — pnpm test is a stub, and the existing tests only cover scripts, navigation, and pure utilities. The regression test you suggested would need new test dependencies, which I am not permitted to add unattended.

What a human needs to decide: whether to approve touching the app provider at all; whether to do the guard as a follow-up sweep covering both the provider and the cookie banner toast rather than expanding this PR; and whether silently falling back to empty settings when the stored value is corrupt is acceptable, since that quietly resets a visitor's saved theme, wallpaper, and cursor instead of surfacing the problem.

[github-actions]

Bundle report

Total JS (gzip)

8.16 MiB (+0.1 KiB / +0.0%)

Eager graph (modules shipped in each entrypoint's initial chunks)

Entrypoint	Eager size	Budget	Modules
✅ app	18.42 MiB (+0.7 KiB / +0.0%)	report-only	2052

Largest modules in the app closure

Module	Size
./src/data/mcp-tools.json	1081.9 KiB
css ./node_modules/.pnpm/css-loader@5.2.7_webpack@5.101.3/node_modules/css-loader/dist/cjs.js??ruleSet[1].rules[8].oneOf[1].use[1]!./node_modules/.pnpm/postcss-loader@4.3.0_postcss@8.5.6_webpack@5.101.3/node_modules/postcss-loader/dist/cjs.js??ruleSet[1].rules[8].oneOf[1].use[2]!./src/styles/global.css	758.7 KiB
./src/components/Stickers/Stickers.tsx	696.4 KiB
./node_modules/.pnpm/@radix-ui+react-icons@1.3.2_react@18.3.1/node_modules/@radix-ui/react-icons/dist/react-icons.esm.js	481.4 KiB
./node_modules/.pnpm/@posthog+brand@0.8.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/x-ray.mjs	480.8 KiB
./node_modules/.pnpm/rehype-raw@7.0.0/node_modules/rehype-raw/lib/index.js + 29 modules	395.1 KiB
./node_modules/.pnpm/@posthog+brand@0.8.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/im-the-driver.mjs	385.7 KiB
./src/hooks/useCustomers.tsx + 55 modules	370.0 KiB
./node_modules/.pnpm/@posthog+icons@0.36.6_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js	354.8 KiB
./node_modules/.pnpm/react-markdown@8.0.7_@types+react@16.14.66_react@18.3.1/node_modules/react-markdown/lib/react-markdown.js + 88 modules	351.4 KiB
./src/components/ProductComparisonTable/index.tsx + 126 modules	301.8 KiB
./node_modules/.pnpm/cloudinary-core@2.14.0_lodash@4.17.21/node_modules/cloudinary-core/cloudinary-core.js	281.9 KiB
./node_modules/.pnpm/@posthog+brand@0.8.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/doll-house.mjs	281.7 KiB
./node_modules/.pnpm/@posthog+brand@0.8.0_react@18.3.1/node_modules/@posthog/brand/dist/generated/hoggies/svg/director.mjs	275.6 KiB
./src/components/SearchUI/index.tsx + 87 modules	273.0 KiB

---

Eager-graph budgets are report-only until a baseline is established. Sizes are gzip of public/**/*.js; eager size is webpack module source bytes for the modules actually shipped in the entrypoint's initial chunks (post-tree-shake).