From 8d03ca124ff73fd32585dda54726a15c314a98e8 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe Date: Tue, 2 Jun 2026 05:19:20 +0200 Subject: [PATCH] Update-Neustart: laufende Downloads als queued parken statt als "Gestoppt" haengenzubleiben Beim Update parkte installUpdate() aktive Downloads via stop() -> deren Abbruch- Continuation markierte die Items "cancelled"/"Gestoppt". autoResumeOnStart nimmt nach dem Neustart aber nur "queued"/"reconnect_wait" auf, also liefen die gerade ladenden Downloads nach dem Update nicht weiter (timing-abhaengig: "manchmal"). Jetzt: stop({parkForRestart:true}) bricht aktive Tasks mit Grund "shutdown" ab, sodass sie als "queued" re-queued werden (wie bei normalem App-Shutdown). Das schliesst zugleich den einzigen plausiblen Loesch-Pfad (all-cancelled-Pakete sind ueber applyRetroactiveCleanupPolicy entfernbar). Stop-Button-Verhalten unveraendert. Zusaetzliche Robustheit in storage.ts (enge Blast-Radien, nicht die Hauptursache): - async-Save-Clobber: eine gequeuete, veraltete Payload konnte einen neueren Sync-Save (persistNowSync/prepareForShutdown) ueberschreiben; Generation wird jetzt zum Snapshot-Zeitpunkt erfasst und durch die Queue getragen. - loadSession gab leer zurueck (und ignorierte ein gefuelltes .bak), wenn die Primaerdatei fehlte; faellt jetzt auf die Backup/Temp-Recovery zurueck. Regressionstests: tests/update-restart-resume.test.ts (echter Live-Download -> Park -> Reload = queued, plus Charakterisierung plain stop() -> cancelled) und tests/session-restart-loss.test.ts (Clobber + Backup-Fallback). Volle Suite gruen. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/main/app-controller.ts | 9 +- src/main/download-manager.ts | 14 ++- src/main/storage.ts | 43 +++++-- tests/session-restart-loss.test.ts | 141 +++++++++++++++++++++ tests/update-restart-resume.test.ts | 188 ++++++++++++++++++++++++++++ 5 files changed, 379 insertions(+), 16 deletions(-) create mode 100644 tests/session-restart-loss.test.ts create mode 100644 tests/update-restart-resume.test.ts diff --git a/src/main/app-controller.ts b/src/main/app-controller.ts index 5c4b152..d792749 100644 --- a/src/main/app-controller.ts +++ b/src/main/app-controller.ts @@ -440,8 +440,15 @@ public async checkDebridAccounts(): Promise { public async installUpdate(onProgress?: (progress: UpdateInstallProgress) => void): Promise { // Stop active downloads before installing. Extractions may continue briefly // until prepareForShutdown() is called during app quit. + // parkForRestart MUST stay true here: it keeps in-flight items as "queued" + // (not "cancelled") so the updated app auto-resumes them after the silent + // install relaunch. A plain stop() marks them "cancelled"/"Gestoppt", which + // autoResumeOnStart does NOT pick up — the downloads then silently fail to + // continue after the update (the reported "packages gone after update" bug). + // Regression coverage: tests/update-restart-resume.test.ts asserts this exact + // stop({parkForRestart:true}) + persistNowSync() sequence reloads as "queued". if (this.manager.isSessionRunning()) { - this.manager.stop(); + this.manager.stop({ parkForRestart: true }); } // Flush any pending async saves BEFORE the update process starts. // This ensures the queue is fully persisted to disk so it survives the restart. diff --git a/src/main/download-manager.ts b/src/main/download-manager.ts index fee74a0..dd7f15d 100644 --- a/src/main/download-manager.ts +++ b/src/main/download-manager.ts @@ -5689,7 +5689,15 @@ export class DownloadManager extends EventEmitter { }); } - public stop(): void { + public stop(options?: { parkForRestart?: boolean }): void { + // parkForRestart: used before an app-update install. Active downloads are + // aborted with the "shutdown" reason so their continuation re-queues them + // (status "queued") instead of marking them "cancelled"/"Gestoppt". A + // cancelled item is NOT picked up by autoResumeOnStart after the update + // relaunch, so the download would silently fail to resume — the user sees + // packages that were downloading "disappear" from the active list. + const parkForRestart = options?.parkForRestart === true; + const abortReason: "stop" | "shutdown" = parkForRestart ? "shutdown" : "stop"; const keepExtraction = this.settings.autoExtractWhenStopped; this.schedulerGeneration += 1; this.session.running = false; @@ -5713,8 +5721,8 @@ export class DownloadManager extends EventEmitter { this.packagePostProcessActive = 0; } for (const active of this.activeTasks.values()) { - active.abortReason = "stop"; - active.abortController.abort("stop"); + active.abortReason = abortReason; + active.abortController.abort(abortReason); } // Reset all non-finished items to clean "Wartet" / "Paket gestoppt" state for (const item of Object.values(this.session.items)) { diff --git a/src/main/storage.ts b/src/main/storage.ts index cb74998..0565f92 100644 --- a/src/main/storage.ts +++ b/src/main/storage.ts @@ -954,13 +954,25 @@ export function emptySession(): SessionState { export function loadSession(paths: StoragePaths): SessionState { ensureBaseDir(paths.baseDir); - if (!fs.existsSync(paths.sessionFile)) { - logger.info("Keine Session-Datei vorhanden, starte mit leerer Session"); - return emptySession(); + const backupFile = sessionBackupPath(paths.sessionFile); + const primaryExists = fs.existsSync(paths.sessionFile); + // A missing primary file is only a genuine "fresh start" when there is also + // nothing to recover from. If a backup or an interrupted-write temp file + // exists, fall through to the recovery chain below instead of returning an + // empty session — otherwise a momentarily-absent primary during an update + // restart would discard a perfectly good backup and wipe the whole queue. + if (!primaryExists) { + const hasRecoverable = fs.existsSync(backupFile) + || fs.existsSync(sessionTempPath(paths.sessionFile, "sync")) + || fs.existsSync(sessionTempPath(paths.sessionFile, "async")); + if (!hasRecoverable) { + logger.info("Keine Session-Datei vorhanden, starte mit leerer Session"); + return emptySession(); + } + logger.warn("Session-Primaerdatei fehlt, aber Backup/Temp vorhanden — Wiederherstellung wird versucht"); } - const primary = readSessionFile(paths.sessionFile); - const backupFile = sessionBackupPath(paths.sessionFile); + const primary = primaryExists ? readSessionFile(paths.sessionFile) : null; // If primary loaded but is empty, check if backup has packages (safety net) if (primary) { @@ -1044,7 +1056,7 @@ export function saveSession(paths: StoragePaths, session: SessionState): void { } let asyncSaveRunning = false; -let asyncSaveQueued: { paths: StoragePaths; payload: string } | null = null; +let asyncSaveQueued: { paths: StoragePaths; payload: string; generation: number } | null = null; let syncSaveGeneration = 0; async function writeSessionPayload(paths: StoragePaths, payload: string, generation: number): Promise { @@ -1074,15 +1086,19 @@ async function writeSessionPayload(paths: StoragePaths, payload: string, generat } } -async function saveSessionPayloadAsync(paths: StoragePaths, payload: string): Promise { +async function saveSessionPayloadAsync(paths: StoragePaths, payload: string, generation: number): Promise { if (asyncSaveRunning) { - asyncSaveQueued = { paths, payload }; + // Keep the freshest payload, but preserve the generation captured when THIS + // payload was snapshotted. Re-reading syncSaveGeneration at re-invoke time + // would let a stale queued write slip past the guard and clobber a newer + // synchronous save (persistNowSync/prepareForShutdown) — which could drop + // packages that the sync save had just persisted. + asyncSaveQueued = { paths, payload, generation }; return; } asyncSaveRunning = true; - const gen = syncSaveGeneration; try { - await writeSessionPayload(paths, payload, gen); + await writeSessionPayload(paths, payload, generation); } catch (error) { logger.error(`Async Session-Save fehlgeschlagen: ${String(error)}`); } finally { @@ -1090,7 +1106,7 @@ async function saveSessionPayloadAsync(paths: StoragePaths, payload: string): Pr if (asyncSaveQueued) { const queued = asyncSaveQueued; asyncSaveQueued = null; - void saveSessionPayloadAsync(queued.paths, queued.payload); + void saveSessionPayloadAsync(queued.paths, queued.payload, queued.generation); } } } @@ -1102,8 +1118,11 @@ export function cancelPendingAsyncSaves(): void { } export async function saveSessionAsync(paths: StoragePaths, session: SessionState): Promise { + // Capture the generation at snapshot time so the guard in writeSessionPayload + // can reliably discard this write if a synchronous save lands afterwards. + const generation = syncSaveGeneration; const payload = JSON.stringify({ ...session, updatedAt: Date.now() }, safeJsonReplacer); - await saveSessionPayloadAsync(paths, payload); + await saveSessionPayloadAsync(paths, payload, generation); } const MAX_HISTORY_ENTRIES = 500; diff --git a/tests/session-restart-loss.test.ts b/tests/session-restart-loss.test.ts new file mode 100644 index 0000000..2d18052 --- /dev/null +++ b/tests/session-restart-loss.test.ts @@ -0,0 +1,141 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { DownloadItem, PackageEntry, SessionState } from "../src/shared/types"; +import { + cancelPendingAsyncSaves, + createStoragePaths, + emptySession, + loadSession, + saveSession, + saveSessionAsync +} from "../src/main/storage"; + +// Regression tests for queue loss across an app-update restart. +// Both scenarios were observed empirically to drop packages before the fix: +// - a queued stale async save clobbering a newer synchronous save +// (persistNowSync / prepareForShutdown), and +// - loadSession ignoring a good .bak when the primary file is momentarily absent. + +const tempDirs: string[] = []; + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +function makePackage(id: string, itemId: string): PackageEntry { + return { + id, + name: `Package ${id}`, + outputDir: "C:/tmp/out", + extractDir: "C:/tmp/extract", + status: "queued", + itemIds: [itemId], + cancelled: false, + enabled: true, + downloadStartedAt: 0, + downloadCompletedAt: 0, + createdAt: 1, + updatedAt: 1 + }; +} + +function makeItem(id: string, packageId: string): DownloadItem { + return { + id, + packageId, + url: `https://example.com/${id}`, + provider: null, + status: "queued", + retries: 0, + speedBps: 0, + downloadedBytes: 0, + totalBytes: null, + progressPercent: 0, + fileName: `${id}.rar`, + targetPath: "", + resumable: true, + attempts: 0, + lastError: "", + fullStatus: "Wartet", + createdAt: 1, + updatedAt: 1 + }; +} + +/** Build a session whose package set is exactly `ids`. */ +function sessionWith(ids: string[]): SessionState { + const s = emptySession(); + for (const id of ids) { + const itemId = `${id}-item`; + s.packageOrder.push(id); + s.packages[id] = makePackage(id, itemId); + s.items[itemId] = makeItem(itemId, id); + } + return s; +} + +const settle = (ms = 250): Promise => new Promise((resolve) => setTimeout(resolve, ms)); + +describe("session restart loss", () => { + it("does not let a queued stale async save clobber a newer synchronous save", async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-loss-")); + tempDirs.push(dir); + const paths = createStoragePaths(dir); + + cancelPendingAsyncSaves(); + await settle(50); + + saveSession(paths, sessionWith(["A", "B"])); + + // An async save goes in-flight, a second async save (stale snapshot) gets + // queued, then a synchronous save persists the live state with package C. + const inflight = saveSessionAsync(paths, sessionWith(["A", "B"])); + const queued = saveSessionAsync(paths, sessionWith(["A", "B"])); + saveSession(paths, sessionWith(["A", "B", "C"])); + + await inflight; + await queued; + await settle(); + + const loaded = loadSession(paths); + expect(Object.keys(loaded.packages).sort()).toEqual(["A", "B", "C"]); + }); + + it("recovers packages from the backup when the primary session file is absent", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-loss-")); + tempDirs.push(dir); + const paths = createStoragePaths(dir); + + fs.writeFileSync(`${paths.sessionFile}.bak`, JSON.stringify(sessionWith(["A", "B"])), "utf8"); + expect(fs.existsSync(paths.sessionFile)).toBe(false); + + const loaded = loadSession(paths); + expect(Object.keys(loaded.packages).sort()).toEqual(["A", "B"]); + }); + + it("still treats a truly fresh install (no primary, no backup, no temp) as empty", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-loss-")); + tempDirs.push(dir); + const paths = createStoragePaths(dir); + + const loaded = loadSession(paths); + expect(Object.keys(loaded.packages)).toEqual([]); + expect(Object.keys(loaded.items)).toEqual([]); + }); + + it("recovers from the backup when the primary exists but is empty", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "rd-loss-")); + tempDirs.push(dir); + const paths = createStoragePaths(dir); + + fs.writeFileSync(paths.sessionFile, JSON.stringify(emptySession()), "utf8"); + fs.writeFileSync(`${paths.sessionFile}.bak`, JSON.stringify(sessionWith(["A", "B"])), "utf8"); + + const loaded = loadSession(paths); + expect(Object.keys(loaded.packages).sort()).toEqual(["A", "B"]); + }); +}); diff --git a/tests/update-restart-resume.test.ts b/tests/update-restart-resume.test.ts new file mode 100644 index 0000000..3051fb7 --- /dev/null +++ b/tests/update-restart-resume.test.ts @@ -0,0 +1,188 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import http from "node:http"; +import { once } from "node:events"; +import { afterEach, describe, expect, it } from "vitest"; +import { DownloadManager } from "../src/main/download-manager"; +import { defaultSettings } from "../src/main/constants"; +import { createStoragePaths, emptySession, loadSession } from "../src/main/storage"; +import { shutdownItemLogs } from "../src/main/item-log"; +import { shutdownPackageLogs } from "../src/main/package-log"; + +// Regression for the reported symptom: after an app update while downloading, +// packages that were in flight do not continue after the restart. +// +// Root cause: installUpdate() called manager.stop(), whose abort continuation +// marks the in-flight item "cancelled"/"Gestoppt". autoResumeOnStart only +// resumes "queued"/"reconnect_wait" items, so after the silent-install relaunch +// the download silently stays parked instead of continuing. + +const tempDirs: string[] = []; +const originalFetch = globalThis.fetch; + +afterEach(async () => { + globalThis.fetch = originalFetch; + shutdownItemLogs(); + shutdownPackageLogs(); + for (const dir of tempDirs.splice(0)) { + for (let attempt = 0; attempt < 5; attempt += 1) { + try { + fs.rmSync(dir, { recursive: true, force: true }); + break; + } catch { + await new Promise((resolve) => setTimeout(resolve, 80)); + } + } + } +}); + +async function waitFor(predicate: () => boolean, timeoutMs = 20000): Promise { + const started = Date.now(); + while (!predicate()) { + if (Date.now() - started > timeoutMs) { + throw new Error("waitFor timeout"); + } + await new Promise((resolve) => setTimeout(resolve, 50)); + } +} + +/** Starts an HTTP server that trickles bytes forever so a download stays + * actively "downloading" until it is aborted. Returns the direct URL plus a + * stop() that tears down all open responses and the server. */ +async function startTricklingServer(): Promise<{ directUrl: string; stop: () => Promise }> { + const openTimers = new Set(); + const openResponses = new Set(); + const server = http.createServer((req, res) => { + if ((req.url || "") !== "/direct") { + res.statusCode = 404; + res.end("not-found"); + return; + } + res.statusCode = 200; + res.setHeader("Accept-Ranges", "bytes"); + res.setHeader("Content-Length", String(64 * 1024 * 1024)); + openResponses.add(res); + res.write(Buffer.alloc(64 * 1024, 7)); + const timer = setInterval(() => { + try { + res.write(Buffer.alloc(16 * 1024, 9)); + } catch { + // socket gone + } + }, 100); + openTimers.add(timer); + res.on("close", () => { + clearInterval(timer); + openTimers.delete(timer); + openResponses.delete(res); + }); + }); + server.listen(0, "127.0.0.1"); + await once(server, "listening"); + const address = server.address(); + if (!address || typeof address === "string") { + throw new Error("server address unavailable"); + } + const directUrl = `http://127.0.0.1:${address.port}/direct`; + const stop = async (): Promise => { + for (const timer of openTimers) { + clearInterval(timer); + } + openTimers.clear(); + for (const res of openResponses) { + try { + res.destroy(); + } catch { + // ignore + } + } + openResponses.clear(); + server.close(); + await once(server, "close"); + }; + return { directUrl, stop }; +} + +function mockUnrestrict(directUrl: string): void { + globalThis.fetch = async (input: RequestInfo | URL, init?: RequestInit): Promise => { + const url = typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url; + if (url.includes("/unrestrict/link")) { + return new Response( + JSON.stringify({ download: directUrl, filename: "episode.mkv", filesize: 64 * 1024 * 1024 }), + { status: 200, headers: { "Content-Type": "application/json" } } + ); + } + return originalFetch(input, init); + }; +} + +async function driveActiveDownload(root: string): Promise<{ manager: DownloadManager; paths: ReturnType; serverStop: () => Promise }> { + const { directUrl, stop: serverStop } = await startTricklingServer(); + mockUnrestrict(directUrl); + const paths = createStoragePaths(path.join(root, "state")); + const manager = new DownloadManager( + { + ...defaultSettings(), + token: "rd-token", + outputDir: path.join(root, "downloads"), + extractDir: path.join(root, "extract"), + autoExtract: false, + autoReconnect: false, + retryLimit: 0 + }, + emptySession(), + paths + ); + manager.addPackages([{ name: "park", links: ["https://dummy/park"] }]); + await manager.start(); + await waitFor(() => { + const item = Object.values(manager.getSnapshot().session.items)[0]; + return item?.status === "downloading" && (manager as unknown as { activeTasks: Map }).activeTasks.size > 0; + }); + return { manager, paths, serverStop }; +} + +describe("update restart resume", () => { + it("characterization: a plain stop() leaves an in-flight item cancelled across a restart", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "rd-update-resume-")); + tempDirs.push(root); + const { manager, paths, serverStop } = await driveActiveDownload(root); + try { + manager.stop(); + manager.persistNowSync(); + await waitFor(() => (manager as unknown as { activeTasks: Map }).activeTasks.size === 0); + manager.prepareForShutdown(); + + const reloaded = loadSession(paths); + const item = Object.values(reloaded.items)[0]; + expect(item).toBeTruthy(); + // Documents the loss of resumability: cancelled items are not auto-resumed. + expect(item.status).toBe("cancelled"); + } finally { + await serverStop(); + } + }); + + it("parks an in-flight item as queued for an update restart so it auto-resumes", async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "rd-update-resume-")); + tempDirs.push(root); + const { manager, paths, serverStop } = await driveActiveDownload(root); + try { + // Mirrors AppController.installUpdate(): park downloads, then sync-persist. + manager.stop({ parkForRestart: true }); + manager.persistNowSync(); + await waitFor(() => (manager as unknown as { activeTasks: Map }).activeTasks.size === 0); + manager.prepareForShutdown(); + + const reloaded = loadSession(paths); + const item = Object.values(reloaded.items)[0]; + expect(item).toBeTruthy(); + // The package/item must survive AND be resumable so auto-resume continues it. + expect(Object.keys(reloaded.packages).length).toBe(1); + expect(item.status).toBe("queued"); + } finally { + await serverStop(); + } + }); +});