mirror of
https://github.com/block/buzz.git
synced 2026-11-02 15:31:27 +00:00
## Summary WebKit throws `SecurityError` from `localStorage.getItem` (not just `setItem`) when storage access is denied for the origin. With no `ErrorBoundary` in `desktop/src`, any such throw inside a provider render (`ThemeProvider`, `CommunitiesProvider`, `App` boot) propagated to the reconciler, unmounted the root, and left a blank window. Measured repro in #5078: a single throwing `getItem` on `buzz-communities` or `buzz-active-community-id` kills the container. Closes #5078. ## What changed **New helper — `desktop/src/shared/lib/safeStorage.ts`** - `getStorageItem(key, fallback?)` — wraps `window.localStorage.getItem`; on a thrown error (SecurityError under denied-storage origin) it warns once per key and returns the fallback. - `setStorageItem(key, value)` and `removeStorageItem(key)` — same fail-closed contract (return `false` on throw). - Unit tests in `safeStorage.test.mjs` cover the happy path and the `SecurityError` path. **Rewired the init-path readers that ran before any UI existed** - `desktop/src/features/communities/communityStorage.ts` — `migrateLegacyCommunityStorage`, `loadCommunities`, `loadActiveCommunityId`, `loadCommunityDiscoveryAfterLeave`, `initFirstCommunity` - `desktop/src/features/communities/legacyCommunityStorage.ts` — `migrateLegacyCommunityStorageBeforeRender` - `desktop/src/shared/theme/ThemeProvider.tsx` — `readStoredTheme`, `applyCachedVars`, the `useState` initialisers for `accentColor` and `followSystem`, and the accent re-read inside `applyTheme` **Root-level fence — `desktop/src/app/RootErrorBoundary.tsx`** - New top-level `ErrorBoundary` wrapping the whole provider tree in `main.tsx`. Any remaining uncaught render error (a future storage read that bypasses the helper, or any other render-time crash) renders a degraded splash with a Reload button instead of a blank window. ## Test plan - `desktop/src/shared/lib/safeStorage.test.mjs` — node `--test` runner, 11 assertions across healthy, absent, and SecurityError-throwing storage. - Full `just ci` runs on the blocker. - Existing `communityStorage.test.mjs` and `legacyCommunityStorage.test.mjs` continue to pass (they exercise the same functions via in-memory Storage doubles; the new code path in `migrateLegacyCommunityStorage` only adds a `try/catch` around the same body). ## Why not an ErrorBoundary-only fix A boundary alone can't help on a *clean* mount — the first throw already unmounted the whole subtree before any state or fallback data was loaded, so retrying would hit the same throw on the very next render. The storage accessor has to fail closed *and* the boundary has to exist for whatever bypasses it. Both are needed; neither is sufficient alone. --------- Signed-off-by: iroiro147 <sarthak.singh@mastersunion.org> Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
114 lines
2.9 KiB
JavaScript
114 lines
2.9 KiB
JavaScript
import assert from "node:assert/strict";
|
|
import test from "node:test";
|
|
|
|
import {
|
|
__resetSafeStorageWarningsForTests,
|
|
getStorageItem,
|
|
removeStorageItem,
|
|
setStorageItem,
|
|
} from "./safeStorage.ts";
|
|
|
|
function createThrowingStorage(initial = {}) {
|
|
const values = new Map(Object.entries(initial));
|
|
return {
|
|
values,
|
|
getItem: (key) => {
|
|
if (key === "__THROW__") throw new Error("SecurityError");
|
|
return values.get(key) ?? null;
|
|
},
|
|
setItem: (key, value) => {
|
|
if (key === "__THROW__") throw new Error("SecurityError");
|
|
values.set(key, String(value));
|
|
},
|
|
removeItem: (key) => {
|
|
if (key === "__THROW__") throw new Error("SecurityError");
|
|
values.delete(key);
|
|
},
|
|
};
|
|
}
|
|
|
|
function patchLocalStorage(storage) {
|
|
globalThis.window ??= {};
|
|
const original = globalThis.window.localStorage;
|
|
Object.defineProperty(globalThis.window, "localStorage", {
|
|
configurable: true,
|
|
writable: true,
|
|
value: storage,
|
|
});
|
|
return () => {
|
|
if (original === undefined) {
|
|
delete globalThis.window.localStorage;
|
|
return;
|
|
}
|
|
Object.defineProperty(globalThis.window, "localStorage", {
|
|
configurable: true,
|
|
writable: true,
|
|
value: original,
|
|
});
|
|
};
|
|
}
|
|
|
|
test.beforeEach(() => {
|
|
__resetSafeStorageWarningsForTests();
|
|
});
|
|
|
|
test("getStorageItem returns the stored value when storage is healthy", () => {
|
|
const restore = patchLocalStorage(
|
|
createThrowingStorage({ "buzz-theme": "buzz" }),
|
|
);
|
|
try {
|
|
assert.equal(getStorageItem("buzz-theme"), "buzz");
|
|
assert.equal(getStorageItem("missing"), null);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
test("getStorageItem returns null when the key is absent", () => {
|
|
const restore = patchLocalStorage(createThrowingStorage());
|
|
try {
|
|
assert.equal(getStorageItem("buzz-theme"), null);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
test("getStorageItem swallows SecurityError and returns the fallback", () => {
|
|
const restore = patchLocalStorage(createThrowingStorage());
|
|
try {
|
|
assert.equal(getStorageItem("__THROW__"), null);
|
|
assert.equal(getStorageItem("__THROW__", "fallback"), "fallback");
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
test("setStorageItem returns true on a healthy write", () => {
|
|
const storage = createThrowingStorage();
|
|
const restore = patchLocalStorage(storage);
|
|
try {
|
|
assert.equal(setStorageItem("buzz-theme", "buzz-dark"), true);
|
|
assert.equal(storage.values.get("buzz-theme"), "buzz-dark");
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
test("setStorageItem returns false instead of throwing on denied storage", () => {
|
|
const restore = patchLocalStorage(createThrowingStorage());
|
|
try {
|
|
assert.equal(setStorageItem("__THROW__", "x"), false);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
test("removeStorageItem returns false instead of throwing on denied storage", () => {
|
|
const restore = patchLocalStorage(createThrowingStorage());
|
|
try {
|
|
assert.equal(removeStorageItem("__THROW__"), false);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|