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) <noreply@anthropic.com>
This commit is contained in:
parent
53cc6b11eb
commit
8d03ca124f
@ -440,8 +440,15 @@ public async checkDebridAccounts(): Promise<DebridAccountStatus[]> {
|
||||
public async installUpdate(onProgress?: (progress: UpdateInstallProgress) => void): Promise<UpdateInstallResult> {
|
||||
// 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.
|
||||
|
||||
@ -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)) {
|
||||
|
||||
@ -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<void> {
|
||||
@ -1074,15 +1086,19 @@ async function writeSessionPayload(paths: StoragePaths, payload: string, generat
|
||||
}
|
||||
}
|
||||
|
||||
async function saveSessionPayloadAsync(paths: StoragePaths, payload: string): Promise<void> {
|
||||
async function saveSessionPayloadAsync(paths: StoragePaths, payload: string, generation: number): Promise<void> {
|
||||
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<void> {
|
||||
// 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;
|
||||
|
||||
141
tests/session-restart-loss.test.ts
Normal file
141
tests/session-restart-loss.test.ts
Normal file
@ -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<void> => 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"]);
|
||||
});
|
||||
});
|
||||
188
tests/update-restart-resume.test.ts
Normal file
188
tests/update-restart-resume.test.ts
Normal file
@ -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<void> {
|
||||
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<void> }> {
|
||||
const openTimers = new Set<NodeJS.Timeout>();
|
||||
const openResponses = new Set<http.ServerResponse>();
|
||||
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<void> => {
|
||||
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<Response> => {
|
||||
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<typeof createStoragePaths>; serverStop: () => Promise<void> }> {
|
||||
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<string, unknown> }).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<string, unknown> }).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<string, unknown> }).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();
|
||||
}
|
||||
});
|
||||
});
|
||||
Loading…
Reference in New Issue
Block a user