fix(settings): stop settings silently reverting on restart (#1272)

Settings live in the renderer's IndexedDB, and two failure modes made them look
saved while nothing reached disk.

A second app instance sharing the same userData directory cannot take the
Chromium storage lock, so its renderer reads defaults and every write is
dropped. The app now holds a single-instance lock and focuses the running window
instead of starting a rival copy. The lock is requested after the userData
override so E2E runs with their own data dir keep independent locks, and after
Squirrel event handling. IPTVNATOR_ALLOW_MULTIPLE_INSTANCES=1 opts out for local
CDP debugging.

updateSettings() patches in-memory state before persisting and the submit path
had no rejection handler, so a failed write produced an unhandled rejection and
no user-visible feedback. SettingsStore now records which half of the round trip
failed, and the settings page surfaces it through a dismissible error snackbar;
the dialog stays open on failure so the save can be retried.

Two follow-ups from review, both wider than the report:

- a second launch now re-creates the main window when the lock owner has none
  left, so closing the last window on macOS no longer leaves a second launch
  quitting silently with nothing on screen
- App.onMainWindowCreated() re-runs window-owned bindings for every rebuilt
  window, so the downloads broadcaster stops holding a destroyed window. This
  also fixes the same bug on the pre-existing dock `activate` path.

Closes #1156
Closes #102
This commit is contained in:
4gray authored and GitHub committed 2026-07-27 23:14:30 +02:00
1 parent 0334296f15
commit bfad82c26c
37 files changed
+956 -23

No files matched your search

@@ -287,3 +287,110 @@ describe('SettingsStore dashboard rail settings', () => {
);
});
});
describe('SettingsStore storage failure reporting', () => {
let injector: Injector;
let storage: {
get: jest.Mock;
set: jest.Mock;
};
let consoleError: jest.SpyInstance;
beforeEach(() => {
storage = {
get: jest.fn(() => of(null)),
set: jest.fn(() => of(undefined)),
};
consoleError = jest
.spyOn(console, 'error')
.mockImplementation(() => undefined);
injector = Injector.create({
providers: [
SettingsStore,
{
provide: StorageMap,
useValue: storage,
},
],
});
});
afterEach(() => {
consoleError.mockRestore();
});
it('reports no failure while storage works', async () => {
const store = injector.get(SettingsStore);
await store.loadSettings();
await store.updateSettings({ language: Language.FRENCH });
expect(store.storageFailure()).toBeNull();
});
it('flags a failed initial load so defaults are not mistaken for saved values', async () => {
const failingSettings = new Subject<Partial<Settings> | null>();
storage.get.mockReturnValueOnce(failingSettings.asObservable());
const store = injector.get(SettingsStore);
const initialLoad = store.loadSettings();
failingSettings.error(new Error('storage unavailable'));
await initialLoad;
expect(store.storageFailure()).toBe('load');
expect(store.getSettings().language).toBe(Language.ENGLISH);
});
it('flags a failed save and rethrows instead of silently keeping the in-memory change', async () => {
storage.set.mockImplementationOnce(() => {
throw new Error('quota exceeded');
});
const store = injector.get(SettingsStore);
await store.loadSettings();
await expect(
store.updateSettings({ language: Language.FRENCH })
).rejects.toThrow('quota exceeded');
expect(store.storageFailure()).toBe('save');
// The in-memory patch still applied — that is exactly why the flag
// matters: the UI shows French but nothing reached disk.
expect(store.getSettings().language).toBe(Language.FRENCH);
});
it('clears the failure once a later save succeeds', async () => {
storage.set.mockImplementationOnce(() => {
throw new Error('quota exceeded');
});
const store = injector.get(SettingsStore);
await store.loadSettings();
await expect(
store.updateSettings({ language: Language.FRENCH })
).rejects.toThrow('quota exceeded');
expect(store.storageFailure()).toBe('save');
await store.updateSettings({ language: Language.GERMAN });
expect(store.storageFailure()).toBeNull();
});
it('clears a load failure once the retried load succeeds', async () => {
const failingSettings = new Subject<Partial<Settings> | null>();
storage.get
.mockReturnValueOnce(failingSettings.asObservable())
.mockReturnValueOnce(of({ language: Language.FRENCH }));
const store = injector.get(SettingsStore);
const initialLoad = store.loadSettings();
failingSettings.error(new Error('storage unavailable'));
await initialLoad;
expect(store.storageFailure()).toBe('load');
await store.loadSettings();
expect(store.storageFailure()).toBeNull();
expect(store.getSettings().language).toBe(Language.FRENCH);
});
});
@@ -59,6 +59,23 @@ const DEFAULT_SETTINGS: Settings = {
tmdb: DEFAULT_TMDB_SETTINGS,
};
/**
* Which half of the settings persistence round-trip failed, if any.
*
* Settings live in the renderer's IndexedDB, which can be unavailable for
* reasons the app cannot control (a second instance holding the Chromium
* storage lock, a corrupted profile, storage blocked by security software).
* Both failures used to be swallowed: `updateSettings` patches the in-memory
* state before persisting, so a failed write still looked applied until the
* next restart (issue #1156). Recording the failure lets the settings UI say
* so instead of pretending the change stuck.
*/
export type SettingsStorageFailure = 'load' | 'save';
interface SettingsStorageState {
storageFailure: SettingsStorageFailure | null;
}
let embeddedMpvPrepareScheduled = false;
function scheduleEmbeddedMpvPrepare(): void {
@@ -101,6 +118,7 @@ function scheduleEmbeddedMpvPrepare(): void {
export const SettingsStore = signalStore(
{ providedIn: 'root' },
withState<Settings>(DEFAULT_SETTINGS),
withState<SettingsStorageState>({ storageFailure: null }),
withComputed((store) => ({
/**
* Live EPG panel layout with the `'timeline'` default applied — the
@@ -124,6 +142,7 @@ export const SettingsStore = signalStore(
const stored = await firstValueFrom(
storage.get(STORE_KEY.Settings)
);
patchState(store, { storageFailure: null });
if (stored) {
const storedSettings = stored as Partial<Settings>;
patchState(store, {
@@ -147,7 +166,10 @@ export const SettingsStore = signalStore(
})().catch((error) => {
settingsLoadPromise = undefined;
console.error('Failed to load settings:', error);
// Keep default settings if loading fails
// Keep default settings if loading fails, but remember
// that they are defaults-by-failure rather than by choice
// so the settings UI can warn about it.
patchState(store, { storageFailure: 'load' });
});
return settingsLoadPromise;
@@ -176,11 +198,15 @@ export const SettingsStore = signalStore(
await firstValueFrom(
storage.set(STORE_KEY.Settings, completeSettings)
);
patchState(store, { storageFailure: null });
if (completeSettings.player === VideoPlayer.EmbeddedMpv) {
scheduleEmbeddedMpvPrepare();
}
} catch (error) {
console.error('Failed to save settings:', error);
// The in-memory patch above already applied, so without
// this flag the change looks saved until the next restart.
patchState(store, { storageFailure: 'save' });
throw error;
}
},