From e17cb092690acad2c0406fa5d1c5ce1d535c5c44 Mon Sep 17 00:00:00 2001
From: Jared Vititoe <jjvititoe1@gmail.com>
Date: Tue, 30 Jun 2026 13:46:51 -0400
Subject: [PATCH] fix(settings): don't crash on load when localStorage is
 blocked + tests (+6)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Prevention work found a real bug: getSettings() runs at module load, and its
catch block called localStorage.removeItem() — but we often reach that catch
*because* localStorage access threw (blocked storage / private mode / sandboxed
context). The removeItem then re-threw, producing an uncaught error that crashed
the whole app at startup. Guarded the cleanup in its own try/catch.

New state/settings suite (6) covers the legacy-boolean callNoiseSuppression
migration, denoise-model/ringtone-id coercion of unknown values, default merge,
malformed JSON, and the blocked-storage regression.

Full suite now 129 tests, all passing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 src/app/state/settings.test.ts | 79 ++++++++++++++++++++++++++++++++++
 src/app/state/settings.ts      | 10 ++++-
 2 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 src/app/state/settings.test.ts

diff --git a/src/app/state/settings.test.ts b/src/app/state/settings.test.ts
new file mode 100644
index 000000000..aa26890ab
--- /dev/null
+++ b/src/app/state/settings.test.ts
@@ -0,0 +1,79 @@
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { getSettings } from './settings';
+
+// getSettings() reads localStorage; node has none, so install a controllable
+// mock per case. (The module already loaded safely with no localStorage thanks
+// to the guarded catch — that path is exercised by the "throws" test below.)
+const setStored = (value: string | null): void => {
+  (globalThis as { localStorage?: unknown }).localStorage = {
+    getItem: () => value,
+    setItem: () => undefined,
+    removeItem: () => undefined,
+  };
+};
+const setThrowingStorage = (): void => {
+  (globalThis as { localStorage?: unknown }).localStorage = {
+    getItem: () => {
+      throw new Error('storage blocked');
+    },
+    setItem: () => {
+      throw new Error('storage blocked');
+    },
+    removeItem: () => {
+      throw new Error('storage blocked');
+    },
+  };
+};
+
+test('returns defaults when nothing is stored', () => {
+  setStored(null);
+  assert.equal(getSettings().callNoiseSuppression, 'browser');
+});
+
+test('migrates the legacy boolean callNoiseSuppression to the 3-way mode', () => {
+  setStored(JSON.stringify({ callNoiseSuppression: true }));
+  assert.equal(getSettings().callNoiseSuppression, 'browser');
+  setStored(JSON.stringify({ callNoiseSuppression: false }));
+  assert.equal(getSettings().callNoiseSuppression, 'off');
+  // a new string value passes through untouched
+  setStored(JSON.stringify({ callNoiseSuppression: 'ml' }));
+  assert.equal(getSettings().callNoiseSuppression, 'ml');
+});
+
+test('coerces unknown persisted denoise model / ringtone id back to defaults', () => {
+  setStored(null);
+  const defaults = getSettings();
+
+  setStored(JSON.stringify({ callDenoiseModel: 'retired-model', ringtoneId: 'bogus' }));
+  const coerced = getSettings();
+  assert.equal(coerced.callDenoiseModel, defaults.callDenoiseModel);
+  assert.equal(coerced.ringtoneId, defaults.ringtoneId);
+
+  setStored(JSON.stringify({ callDenoiseModel: 'rnnoise', ringtoneId: 'chime' }));
+  const valid = getSettings();
+  assert.equal(valid.callDenoiseModel, 'rnnoise');
+  assert.equal(valid.ringtoneId, 'chime');
+});
+
+test('merges stored values over defaults', () => {
+  setStored(JSON.stringify({ callNoiseSuppression: 'off', someUnknownKey: 1 }));
+  const s = getSettings();
+  assert.equal(s.callNoiseSuppression, 'off');
+  // a default field not present in storage is still populated
+  assert.notEqual(s.callDenoiseModel, undefined);
+});
+
+test('returns defaults without throwing when localStorage access throws', () => {
+  setThrowingStorage();
+  // regression: the catch used to call localStorage.removeItem(), which re-threw
+  // and crashed the app at module load when storage was blocked.
+  assert.doesNotThrow(() => getSettings());
+  assert.equal(getSettings().callNoiseSuppression, 'browser');
+});
+
+test('returns defaults on malformed JSON', () => {
+  setStored('{ not valid json');
+  assert.doesNotThrow(() => getSettings());
+  assert.equal(getSettings().callNoiseSuppression, 'browser');
+});
diff --git a/src/app/state/settings.ts b/src/app/state/settings.ts
index 5db347673..6ceaab69b 100644
--- a/src/app/state/settings.ts
+++ b/src/app/state/settings.ts
@@ -296,7 +296,15 @@ export const getSettings = (): Settings => {
       },
     };
   } catch {
-    localStorage.removeItem(STORAGE_KEY);
+    // We may be here precisely because localStorage access throws (blocked
+    // storage / private mode / sandboxed context). Removing the key must not be
+    // allowed to re-throw — getSettings() runs at module load, so an uncaught
+    // error here would crash the whole app on startup.
+    try {
+      localStorage.removeItem(STORAGE_KEY);
+    } catch {
+      /* localStorage unavailable — nothing to clean up */
+    }
     return defaultSettings;
   }
 };