fix(persistence): serialize autosave generations before queue effects
Track settings input generations so an older asynchronous secret save cannot mark a newer value durable. Commit queue snapshots before cancellation, cleanup, or pause effects so SQLite failures leave runtime processes and files unchanged.
This commit is contained in:
+35
-22
@@ -52,7 +52,7 @@ import { registerTrustedIpcHandler } from './main/domain/privileged-ipc';
|
||||
import { createRendererQueueItem, getMergeGroupCleanupPaths } from './main/domain/renderer-queue-input';
|
||||
import { createAppStateStore, type AppStateStore } from './main/domain/app-state-store';
|
||||
import { createExportableConfig } from './main/domain/config-export';
|
||||
import { persistStateChange } from './main/domain/persistence-commit';
|
||||
import { commitQueueMutation, persistStateChange } from './main/domain/persistence-commit';
|
||||
import { resolveSecretInputUpdate } from './main/domain/secret-input';
|
||||
import { createSecretStore, type SecretStore } from './main/domain/secret-store';
|
||||
import { createElectronSecureStorage } from './main/infra/secure-storage';
|
||||
@@ -7424,25 +7424,29 @@ registerTrustedIpcHandler(ipcMain, 'add-to-queue', isTrustedRendererEvent, () =>
|
||||
registerTrustedIpcHandler(ipcMain, 'remove-from-queue', isTrustedRendererEvent, () => Promise.resolve(downloadQueue), async (_, id: string) => {
|
||||
if (typeof id !== 'string' || !id) return downloadQueue;
|
||||
const wasActiveItem = activeQueueItemId === id || activeDownloads.has(id) || queueProcessRegistry.activeItemIds().includes(id);
|
||||
const removedItem = downloadQueue.find((item) => item.id === id);
|
||||
|
||||
if (wasActiveItem) {
|
||||
cancelledItemIds.add(id);
|
||||
await queueProcessRegistry.cancelItem(id);
|
||||
activeDownloads.delete(id);
|
||||
const nextActiveId = queueProcessRegistry.activeItemIds()[0] || null;
|
||||
activeQueueItemId = nextActiveId;
|
||||
runtimeMetrics.activeItemId = nextActiveId;
|
||||
runtimeMetrics.activeItemTitle = nextActiveId ? downloadQueue.find((item) => item.id === nextActiveId)?.title || null : null;
|
||||
appendDebugLog('queue-item-removed-active-cancelled', { id });
|
||||
}
|
||||
|
||||
// Clean up merge-group temp files (must run for any merge group, not just active)
|
||||
const removedItem = downloadQueue.find(item => item.id === id);
|
||||
for (const cleanupPath of getMergeGroupCleanupPaths(removedItem)) {
|
||||
try { if (fs.existsSync(cleanupPath)) fs.unlinkSync(cleanupPath); } catch { }
|
||||
}
|
||||
|
||||
downloadQueue = persistStateChange(downloadQueue, (current) => current.filter((item) => item.id !== id), saveQueue);
|
||||
await commitQueueMutation(
|
||||
downloadQueue,
|
||||
(current) => current.filter((item) => item.id !== id),
|
||||
saveQueue,
|
||||
(nextQueue) => { downloadQueue = nextQueue; },
|
||||
async () => {
|
||||
if (wasActiveItem) {
|
||||
cancelledItemIds.add(id);
|
||||
await queueProcessRegistry.cancelItem(id);
|
||||
activeDownloads.delete(id);
|
||||
const nextActiveId = queueProcessRegistry.activeItemIds()[0] || null;
|
||||
activeQueueItemId = nextActiveId;
|
||||
runtimeMetrics.activeItemId = nextActiveId;
|
||||
runtimeMetrics.activeItemTitle = nextActiveId ? downloadQueue.find((item) => item.id === nextActiveId)?.title || null : null;
|
||||
appendDebugLog('queue-item-removed-active-cancelled', { id });
|
||||
}
|
||||
for (const cleanupPath of getMergeGroupCleanupPaths(removedItem)) {
|
||||
try { if (fs.existsSync(cleanupPath)) fs.unlinkSync(cleanupPath); } catch { }
|
||||
}
|
||||
},
|
||||
);
|
||||
emitQueueUpdated();
|
||||
return downloadQueue;
|
||||
});
|
||||
@@ -7631,7 +7635,6 @@ ipcMain.handle('pause-download', async (event) => {
|
||||
if (!isTrustedRendererEvent(event)) return false;
|
||||
if (!isDownloading || queuePaused) return false;
|
||||
|
||||
await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.pauseItem(id)));
|
||||
const nextQueue = downloadQueue.map((item) => item.status === 'downloading' ? {
|
||||
...item,
|
||||
status: 'paused' as const,
|
||||
@@ -7639,8 +7642,18 @@ ipcMain.handle('pause-download', async (event) => {
|
||||
eta: '',
|
||||
progressStatus: tBackend('downloadPaused')
|
||||
} : item);
|
||||
downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue);
|
||||
queuePaused = true;
|
||||
await commitQueueMutation(
|
||||
downloadQueue,
|
||||
() => nextQueue,
|
||||
saveQueue,
|
||||
(candidate) => {
|
||||
downloadQueue = candidate;
|
||||
queuePaused = true;
|
||||
},
|
||||
async () => {
|
||||
await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.pauseItem(id)));
|
||||
},
|
||||
);
|
||||
emitQueueUpdated(true);
|
||||
mainWindow?.webContents.send('download-paused');
|
||||
return true;
|
||||
|
||||
@@ -1,10 +1,10 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { openDatabase, type DbHandle } from '../infra/db';
|
||||
import { createAppStateStore } from './app-state-store';
|
||||
import { persistStateChange } from './persistence-commit';
|
||||
import { commitQueueMutation, persistStateChange } from './persistence-commit';
|
||||
|
||||
let directory: string;
|
||||
let db: DbHandle;
|
||||
@@ -59,4 +59,63 @@ describe('persistStateChange', () => {
|
||||
expect(runtime).toEqual(previous);
|
||||
expect(createAppStateStore(db).loadQueue()).toEqual(previous);
|
||||
});
|
||||
|
||||
it('does not cancel a process or delete a file when queue removal cannot persist', async () => {
|
||||
const previous = [{ id: 'q1', status: 'pending' }];
|
||||
const temporaryFile = path.join(directory, 'q1.partial');
|
||||
fs.writeFileSync(temporaryFile, 'keep');
|
||||
createAppStateStore(db).saveQueue(previous);
|
||||
const failingDb: DbHandle = {
|
||||
...db,
|
||||
run(sql, params) {
|
||||
if (sql.includes('DELETE FROM queue_items')) throw new Error('SQLITE_IOERR remove');
|
||||
db.run(sql, params);
|
||||
},
|
||||
};
|
||||
const cancelProcess = vi.fn();
|
||||
let runtime = previous;
|
||||
|
||||
await expect(commitQueueMutation(
|
||||
runtime,
|
||||
() => [],
|
||||
(candidate) => createAppStateStore(failingDb).saveQueue(candidate),
|
||||
(candidate) => { runtime = candidate; },
|
||||
async () => {
|
||||
cancelProcess('q1');
|
||||
fs.rmSync(temporaryFile);
|
||||
},
|
||||
)).rejects.toThrow('SQLITE_IOERR remove');
|
||||
|
||||
expect(cancelProcess).not.toHaveBeenCalled();
|
||||
expect(fs.existsSync(temporaryFile)).toBe(true);
|
||||
expect(runtime).toEqual(previous);
|
||||
expect(createAppStateStore(db).loadQueue()).toEqual(previous);
|
||||
});
|
||||
|
||||
it('does not pause a process when a paused queue snapshot cannot persist', async () => {
|
||||
const previous = [{ id: 'q1', status: 'downloading' }];
|
||||
const paused = [{ id: 'q1', status: 'paused' }];
|
||||
createAppStateStore(db).saveQueue(previous);
|
||||
const failingDb: DbHandle = {
|
||||
...db,
|
||||
run(sql, params) {
|
||||
if (sql.includes('DELETE FROM queue_items')) throw new Error('SQLITE_IOERR pause');
|
||||
db.run(sql, params);
|
||||
},
|
||||
};
|
||||
const pauseProcess = vi.fn();
|
||||
let runtime = previous;
|
||||
|
||||
await expect(commitQueueMutation(
|
||||
runtime,
|
||||
() => paused,
|
||||
(candidate) => createAppStateStore(failingDb).saveQueue(candidate),
|
||||
(candidate) => { runtime = candidate; },
|
||||
async () => { pauseProcess('q1'); },
|
||||
)).rejects.toThrow('SQLITE_IOERR pause');
|
||||
|
||||
expect(pauseProcess).not.toHaveBeenCalled();
|
||||
expect(runtime).toEqual(previous);
|
||||
expect(createAppStateStore(db).loadQueue()).toEqual(previous);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3,3 +3,17 @@ export function persistStateChange<T>(current: T, createNext: (current: T) => T,
|
||||
persist(next);
|
||||
return next;
|
||||
}
|
||||
|
||||
export async function commitQueueMutation<T>(
|
||||
current: T,
|
||||
createNext: (current: T) => T,
|
||||
persist: (next: T) => void,
|
||||
apply: (next: T) => void,
|
||||
effects: () => Promise<void>,
|
||||
): Promise<T> {
|
||||
const next = createNext(current);
|
||||
persist(next);
|
||||
apply(next);
|
||||
await effects();
|
||||
return next;
|
||||
}
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
import * as fs from 'node:fs';
|
||||
import * as path from 'node:path';
|
||||
import * as vm from 'node:vm';
|
||||
import * as ts from 'typescript';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
|
||||
type Input = {
|
||||
value: string;
|
||||
checked: boolean;
|
||||
};
|
||||
|
||||
const inputIds = [
|
||||
'clientId', 'clientSecret', 'sidebarSplitViewToggle', 'downloadMode', 'partMinutes', 'parallelDownloads',
|
||||
'performanceMode', 'smartSchedulerToggle', 'duplicatePreventionToggle', 'persistQueueToggle',
|
||||
'autoResumeQueueToggle', 'notifyEachCompletionToggle', 'streamlinkDisableAdsToggle', 'downloadChatReplayToggle',
|
||||
'captureLiveChatToggle', 'logStreamEventsToggle', 'autoResumeLiveRecordingToggle', 'autoMergeResumedPartsToggle',
|
||||
'deletePartsAfterMergeToggle', 'discordWebhookUrl', 'discordNotifyLiveStartToggle', 'discordNotifyLiveEndToggle',
|
||||
'discordNotifyVodCompleteToggle', 'discordNotifyVodAutoQueuedToggle', 'autoVodPollMinutes', 'autoVodMaxAgeHours',
|
||||
'autoCleanupEnabledToggle', 'autoCleanupDays', 'autoCleanupTarget', 'autoCleanupAction', 'streamlinkQuality',
|
||||
'metadataCacheMinutes', 'vodFilenameTemplate', 'partsFilenameTemplate', 'defaultClipFilenameTemplate'
|
||||
];
|
||||
|
||||
function createInput(value = '', checked = false): Input {
|
||||
return { value, checked };
|
||||
}
|
||||
|
||||
describe('renderer settings autosave orchestration', () => {
|
||||
it('persists a newer secret after an earlier asynchronous save settles', async () => {
|
||||
const inputs = new Map(inputIds.map((id) => [id, createInput()]));
|
||||
inputs.get('clientSecret')!.value = 'A';
|
||||
let resolveFirstSecret: ((status: unknown) => void) | undefined;
|
||||
const setClientSecretCalls: string[] = [];
|
||||
const saveConfigCalls: unknown[] = [];
|
||||
const window = {
|
||||
api: {
|
||||
setClientSecret(value: string) {
|
||||
setClientSecretCalls.push(value);
|
||||
if (value === 'A') {
|
||||
return new Promise((resolve) => { resolveFirstSecret = resolve; });
|
||||
}
|
||||
return Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: true, discordWebhookConfigured: false });
|
||||
},
|
||||
clearClientSecret: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: false }),
|
||||
setDiscordWebhook: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: true }),
|
||||
clearDiscordWebhook: () => Promise.resolve({ encryptionAvailable: true, clientSecretConfigured: false, discordWebhookConfigured: false }),
|
||||
saveConfig(payload: unknown) {
|
||||
saveConfigCalls.push(payload);
|
||||
return Promise.resolve(payload);
|
||||
},
|
||||
},
|
||||
};
|
||||
const sandbox = {
|
||||
window,
|
||||
config: {},
|
||||
UI_TEXT: { status: {}, static: {}, streamers: {} },
|
||||
byId: (id: string) => inputs.get(id) ?? createInput(),
|
||||
collectUnknownTemplatePlaceholders: () => [],
|
||||
document: { hidden: false, querySelector: () => null, getElementById: () => null },
|
||||
setTimeout,
|
||||
clearTimeout,
|
||||
console,
|
||||
};
|
||||
const context = vm.createContext(sandbox);
|
||||
const source = fs.readFileSync(path.join(process.cwd(), 'src', 'renderer-settings.ts'), 'utf8');
|
||||
const compiled = ts.transpileModule(source, {
|
||||
compilerOptions: { target: ts.ScriptTarget.ES2022, module: ts.ModuleKind.None },
|
||||
}).outputText;
|
||||
vm.runInContext(compiled, context);
|
||||
|
||||
const firstSave = vm.runInContext('flushSettingsAutoSave(false)', context) as Promise<void>;
|
||||
expect(setClientSecretCalls).toEqual(['A']);
|
||||
expect(vm.runInContext('settingsAutoSaveInFlight', context)).toBe(true);
|
||||
|
||||
vm.runInContext("byId('clientSecret').value = 'B'; secretInputGenerations.clientSecret += 1; if (typeof settingsInputGeneration === 'number') settingsInputGeneration += 1; void flushSettingsAutoSave(false);", context);
|
||||
expect(vm.runInContext('secretInputGenerations.clientSecret', context)).toBe(1);
|
||||
resolveFirstSecret?.({ encryptionAvailable: true, clientSecretConfigured: true, discordWebhookConfigured: false });
|
||||
await firstSave;
|
||||
await new Promise<void>((resolve) => setImmediate(resolve));
|
||||
|
||||
expect(setClientSecretCalls).toEqual(['A', 'B']);
|
||||
expect(saveConfigCalls).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
@@ -6,6 +6,7 @@ let pendingSettingsAutoSave = false;
|
||||
let settingsAutoSaveTimer: number | null = null;
|
||||
let pendingCredentialsReconnect = false;
|
||||
let lastPersistedSettingsFingerprint = '';
|
||||
let settingsInputGeneration = 0;
|
||||
const SECRET_INPUT_MASK = '••••••••';
|
||||
let secretStatus: SecretStatus = {
|
||||
encryptionAvailable: false,
|
||||
@@ -26,6 +27,10 @@ function canRunSettingsAutoRefresh(): boolean {
|
||||
return document.querySelector('.tab-content.active')?.id === 'settingsTab';
|
||||
}
|
||||
|
||||
function markSettingsInputChanged(): void {
|
||||
settingsInputGeneration += 1;
|
||||
}
|
||||
|
||||
async function connect(): Promise<void> {
|
||||
const hasCredentials = Boolean((config.client_id ?? '').toString().trim() && secretStatus.clientSecretConfigured);
|
||||
if (!hasCredentials) {
|
||||
@@ -99,6 +104,7 @@ function applyTemplatePreset(preset: string): void {
|
||||
byId<HTMLInputElement>('partsFilenameTemplate').value = selected.parts;
|
||||
byId<HTMLInputElement>('defaultClipFilenameTemplate').value = selected.clip;
|
||||
validateFilenameTemplates();
|
||||
markSettingsInputChanged();
|
||||
// Programmatic .value = ... does not trigger the 'input' event the
|
||||
// template inputs listen on for debounced save, so the preset click
|
||||
// would otherwise look applied but never persist until the user
|
||||
@@ -831,6 +837,7 @@ async function flushSettingsAutoSave(reconnectAfterSave = false): Promise<void>
|
||||
|
||||
const payload = collectAutoSavePayload();
|
||||
const fingerprint = getSettingsFingerprint(payload);
|
||||
const inputGeneration = settingsInputGeneration;
|
||||
|
||||
if (fingerprint === lastPersistedSettingsFingerprint) {
|
||||
if (reconnectAfterSave && pendingCredentialsReconnect) {
|
||||
@@ -849,7 +856,11 @@ async function flushSettingsAutoSave(reconnectAfterSave = false): Promise<void>
|
||||
try {
|
||||
await persistSecretInputs();
|
||||
config = await window.api.saveConfig(payload);
|
||||
lastPersistedSettingsFingerprint = getSettingsFingerprint({});
|
||||
if (settingsInputGeneration === inputGeneration) {
|
||||
lastPersistedSettingsFingerprint = getSettingsFingerprint({});
|
||||
} else {
|
||||
pendingSettingsAutoSave = true;
|
||||
}
|
||||
if (reconnectAfterSave && pendingCredentialsReconnect) {
|
||||
pendingCredentialsReconnect = false;
|
||||
await connect();
|
||||
@@ -928,13 +939,17 @@ function initSettingsAutoSave(): void {
|
||||
|
||||
for (const id of immediateSaveIds) {
|
||||
const element = byId<HTMLInputElement | HTMLSelectElement>(id);
|
||||
element.addEventListener('change', triggerImmediateSave);
|
||||
element.addEventListener('change', () => {
|
||||
markSettingsInputChanged();
|
||||
triggerImmediateSave();
|
||||
});
|
||||
element.addEventListener('blur', triggerImmediateSave);
|
||||
}
|
||||
|
||||
for (const id of debouncedSaveIds) {
|
||||
const element = byId<HTMLInputElement>(id);
|
||||
element.addEventListener('input', () => {
|
||||
markSettingsInputChanged();
|
||||
scheduleSettingsAutoSave();
|
||||
});
|
||||
element.addEventListener('blur', () => {
|
||||
@@ -945,6 +960,7 @@ function initSettingsAutoSave(): void {
|
||||
for (const id of credentialIds) {
|
||||
const element = byId<HTMLInputElement>(id);
|
||||
element.addEventListener('input', () => {
|
||||
markSettingsInputChanged();
|
||||
pendingCredentialsReconnect = true;
|
||||
scheduleSettingsAutoSave();
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user