Fix: Tonspur-Remux konnte bei Windows-Datei-Lock Original UND Remux verlieren
Der atomare Ersetzen-Schritt loeschte das Original bevor der Ersatz bestaetigt war; schlug das anschliessende Rename fehl (z.B. AV/Indexer-Lock), raeumte der aeussere catch zusaetzlich die Temp-Datei weg -> null Kopien auf der Platte. - Atomares Replace-over (MoveFileEx REPLACE_EXISTING / rename(2)) statt rm-dann-rename: filePath haelt zu jedem Zeitpunkt entweder das volle Original oder den vollen Remux. - renameWithRetry: transiente Locks (EBUSY/EACCES/EPERM/EEXIST) mit Backoff (200/500/1000ms) statt sofort abzubrechen. - Eindeutiger Temp-Name (~rd<pid><rand>) statt fixem ~rdtmp -> keine Kollision zwischen parallelen Paketen/Retries. - 3 neue Tests (Recovery bei Replace-Fehler, Retry-Pfad EBUSY/EXDEV).
This commit is contained in:
Sucukdeluxe
parent
92890f9649
commit
189af2242f
@ -1,6 +1,7 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import crypto from "node:crypto";
|
||||
import { spawn } from "node:child_process";
|
||||
|
||||
// Removes only-German audio handling for "Dual Language" (.DL.) scene releases.
|
||||
@ -55,6 +56,9 @@ export interface ProcessVideoOptions {
|
||||
export interface ProcessVideoDeps {
|
||||
resolveTooling?: () => Promise<{ ffmpeg: string; ffprobe: string } | null>;
|
||||
runProcess?: typeof runVideoProcess;
|
||||
// Seam for the atomic-replace rename so its failure/recovery path is testable
|
||||
// without provoking a real OS file lock. Production uses renameWithRetry.
|
||||
rename?: (from: string, to: string) => Promise<void>;
|
||||
}
|
||||
|
||||
const VIDEO_REMUX_EXTENSIONS = new Set([".mkv", ".mp4"]);
|
||||
@ -378,6 +382,41 @@ async function getFreeSpaceBytes(dir: string): Promise<number | null> {
|
||||
}
|
||||
}
|
||||
|
||||
const RENAME_RETRY_DELAYS_MS = [200, 500, 1000];
|
||||
const RENAME_RETRYABLE_CODES = new Set(["EBUSY", "EACCES", "EPERM", "EEXIST"]);
|
||||
|
||||
function delayMs(ms: number): Promise<void> {
|
||||
return new Promise((resolve) => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
// Windows file locks from antivirus, the search indexer, or a media scanner are
|
||||
// transient: a rename that hits EBUSY/EACCES/EPERM/EEXIST often succeeds a moment
|
||||
// later. Retry with backoff before giving up so a momentary lock doesn't abort
|
||||
// the atomic replace and leave the file unprocessed.
|
||||
export async function renameWithRetry(from: string, to: string): Promise<void> {
|
||||
for (let attempt = 0; ; attempt += 1) {
|
||||
try {
|
||||
await fs.promises.rename(from, to);
|
||||
return;
|
||||
} catch (error) {
|
||||
const code = (error as NodeJS.ErrnoException)?.code;
|
||||
if (!code || !RENAME_RETRYABLE_CODES.has(code) || attempt >= RENAME_RETRY_DELAYS_MS.length) {
|
||||
throw error;
|
||||
}
|
||||
await delayMs(RENAME_RETRY_DELAYS_MS[attempt]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Short, unique, same-directory sidecar name (never longer than the original file
|
||||
// name) so concurrent packages / retries never collide on a fixed temp name and a
|
||||
// long scene filename + suffix cannot push the path past Windows MAX_PATH.
|
||||
function uniqueTempPath(filePath: string): string {
|
||||
const ext = path.extname(filePath);
|
||||
const token = `${process.pid.toString(36)}${crypto.randomBytes(3).toString("hex")}`;
|
||||
return path.join(path.dirname(filePath), `~rd${token}${ext}`);
|
||||
}
|
||||
|
||||
export async function processVideoFile(filePath: string, opts: ProcessVideoOptions, deps: ProcessVideoDeps = {}): Promise<VideoProcessResult> {
|
||||
const resolveTool = deps.resolveTooling || resolveVideoTooling;
|
||||
const run = deps.runProcess || runVideoProcess;
|
||||
@ -424,11 +463,7 @@ export async function processVideoFile(filePath: string, opts: ProcessVideoOptio
|
||||
return { action: "skipped-no-space", reason: "zu wenig freier Speicher fuer Remux", totalAudioTracks: streams.length, audioLanguages };
|
||||
}
|
||||
|
||||
const ext = path.extname(filePath);
|
||||
// Short, same-directory temp name (never longer than the original file name) so
|
||||
// a long scene filename + temp suffix cannot push the temp path past Windows
|
||||
// MAX_PATH and make ffmpeg fail (which would leave the file unprocessed).
|
||||
const tempPath = path.join(path.dirname(filePath), `~rdtmp${ext}`);
|
||||
const tempPath = uniqueTempPath(filePath);
|
||||
await fs.promises.rm(tempPath, { force: true }).catch(() => {});
|
||||
|
||||
const remux = await run(
|
||||
@ -451,15 +486,18 @@ export async function processVideoFile(filePath: string, opts: ProcessVideoOptio
|
||||
return { action: "error", reason: "Remux ergab leere Datei", totalAudioTracks: streams.length, audioLanguages };
|
||||
}
|
||||
|
||||
const renameOp = deps.rename || renameWithRetry;
|
||||
try {
|
||||
// libuv rename replaces an existing destination on Windows; fall back if not.
|
||||
await fs.promises.rename(tempPath, filePath).catch(async () => {
|
||||
await fs.promises.rm(filePath, { force: true });
|
||||
await fs.promises.rename(tempPath, filePath);
|
||||
});
|
||||
// Atomic replace-over: libuv maps fs.rename to MoveFileEx(REPLACE_EXISTING) on
|
||||
// Windows and rename(2) on POSIX, both atomic on the same volume, so filePath
|
||||
// holds either the full original or the full remux at every instant. Retried
|
||||
// for transient locks. We must NEVER rm the original first (the old fallback
|
||||
// did): an rm-then-failed-rename left zero copies of the file on disk.
|
||||
await renameOp(tempPath, filePath);
|
||||
// Preserve original mtime so freshness gates (hybrid collect) don't skip it.
|
||||
await fs.promises.utimes(filePath, originalStat.atime, originalStat.mtime).catch(() => {});
|
||||
} catch (error) {
|
||||
// Replace failed -> the original is untouched at filePath. Drop the temp only.
|
||||
await fs.promises.rm(tempPath, { force: true }).catch(() => {});
|
||||
return { action: "error", reason: "Ersetzen der Datei fehlgeschlagen", error: String(error), totalAudioTracks: streams.length, audioLanguages };
|
||||
}
|
||||
|
||||
@ -1,8 +1,51 @@
|
||||
# Real-Debrid-Downloader — Tasks (Stand 2026-06-07)
|
||||
# Real-Debrid-Downloader — Tasks (Stand 2026-06-08)
|
||||
|
||||
**Status:** Alle zugesagten Features erledigt+released (Archiv unten). EIN Bug analysiert
|
||||
+ geparkt (Mega-Web Account-3-Rotation, siehe direkt unten — wartet auf 1 Log-Zahl vom User).
|
||||
Rest ist freiwilliger Backlog.
|
||||
**Status:** Alle zugesagten Features erledigt+released (Archiv unten). Aktuell läuft ein
|
||||
**intensiver Bug-Audit** (User-Goal 2026-06-08, "schaue intensiv nach weiteren Bugs") —
|
||||
Fortschritt direkt unten.
|
||||
|
||||
---
|
||||
|
||||
## 🔴 LAUFEND — Bug-Audit 2026-06-08 (Multi-Agent find→verify, 18 bestätigt)
|
||||
|
||||
Advisor-Triage: **A = einzige echte Daten-Verlust-Notlage** → zuerst, eigener Release.
|
||||
Sequenz: Release 1 (v1.7.189) = A + B; Release 2 (v1.7.190) = C,D/E,F,G,H,I,J,L,M,N,O,P,Q.
|
||||
Ein Commit pro Fix, jeder einzeln verifiziert. **K übersprungen** (auto-rename-Reorder,
|
||||
schlechtestes Risiko/Nutzen, kann für diesen User gar nicht feuern).
|
||||
|
||||
### Release 1 — Daten-Verlust-Stopper (v1.7.189)
|
||||
- [x] **A** `video-processor.ts` atomic-replace zerstörte bei Windows-Lock BEIDE Kopien
|
||||
(rm(original) VOR bestätigtem Replace + outer-catch rm(temp) → 0 Kopien). **GEFIXT:**
|
||||
atomic replace-over + `renameWithRetry` (EBUSY/EACCES/EPERM/EEXIST, Backoff 200/500/1000ms),
|
||||
rm-first-Fallback entfernt, **unique** Temp-Name (`~rd<pid><rand>`, löst auch C-Kollision).
|
||||
Advisor bestätigt Ansatz besser als bak-dance (kein Missing-File-Window). 3 neue Tests
|
||||
(Recovery + Retry-Pfad), 41 video-processor-Tests grün, tsc=6 (Baseline).
|
||||
- [ ] **B** `app-controller.ts` importBackup (settings-only) ruft setSettings VOR `if(!hasSession)`
|
||||
→ applyRetroactiveCleanupPolicy löscht fertige Downloads. + **I** live-usage-Counter nicht
|
||||
erhalten (anders als updateSettings).
|
||||
|
||||
### Release 2 — Medium/Low (v1.7.190), ein Commit pro Fix
|
||||
- [ ] **C** ~~fixe Temp-Name-Kollision~~ → bereits in A subsumiert (unique Name).
|
||||
- [ ] **D/E** debrid.ts Rotation: abort-Klassifizierung über `signal.reason` (TimeoutError vs
|
||||
cancel) statt Text/elapsedMs; API-Pfad 'cancel' umgeht. **VORHER empirisch bestätigen:**
|
||||
`AbortSignal.any([ac.signal, AbortSignal.timeout(x)]).reason?.name==='TimeoutError'` in DIESEM
|
||||
Electron-Build; konservativen Fallback behalten, alte Guard nicht blind löschen.
|
||||
- [ ] **F** Mega-Web empty-streak Concurrency (streak permanent-park unreachable-to-clear vorher
|
||||
re-verifizieren bevor Maschinerie).
|
||||
- [ ] **G** download-manager `dropItemContribution` subtrahiert Session-Totals nicht.
|
||||
- [ ] **H** logger.ts `flushAsync` snapshot-by-slice korrumpiert bei 1MB-Cap-Trim während await
|
||||
→ move-snapshot (`linesSnapshot = pendingLines; pendingLines = []`).
|
||||
- [ ] **I** → mit B zusammen (app-controller live-usage-Counter).
|
||||
- [ ] **J** download-manager `abortPackagePostProcessing` löscht Task-Handle ohne Identity-Guard.
|
||||
- [ ] **L** `isGermanStream` Title-Regex False-Positive.
|
||||
- [ ] **M** `looksLikeGermanRelease` 'dubbed' zu breit.
|
||||
- [ ] **N** `stripDualLangFromFileName` Kollision.
|
||||
- [ ] **O** classifyAccountFailure abort-Branch jetzt tot (nach v1.7.187-Fix).
|
||||
- [ ] **P** extractor.ts nested-Resume-Keys (`nested:<name>`) bei jedem extractPackageArchives
|
||||
gepurged (prune-Whitelist nur top-level) → `startsWith("nested:")` in prune skippen.
|
||||
- [ ] **Q** (NEU, aus A-Review) `collectFilesByExtensions` filtert `~rd`-Temp-Präfix NICHT →
|
||||
crash-verwaiste Teil-Remuxe könnten in Library gesammelt werden. Vorbestehend (alter fixer
|
||||
`~rdtmp` wurde überschrieben, neuer unique akkumuliert) → `~`-Präfix in collect skippen.
|
||||
|
||||
---
|
||||
|
||||
|
||||
@ -1,7 +1,7 @@
|
||||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
stripDualLangMarker,
|
||||
hasDualLangMarker,
|
||||
@ -13,6 +13,7 @@ import {
|
||||
buildFfmpegRemuxArgs,
|
||||
computeRemuxTimeoutMs,
|
||||
processVideoFile,
|
||||
renameWithRetry,
|
||||
type VideoSpawnResult
|
||||
} from "../src/main/video-processor";
|
||||
|
||||
@ -208,6 +209,11 @@ describe("processVideoFile (real fs body, fake runner)", () => {
|
||||
};
|
||||
}
|
||||
|
||||
// Any sidecar the replace machinery may leave behind (unique "~rd…" temp names).
|
||||
function leftoverTemps(file: string): string[] {
|
||||
return fs.readdirSync(path.dirname(file)).filter((n) => n.startsWith("~rd"));
|
||||
}
|
||||
|
||||
const tooling = async (): Promise<{ ffmpeg: string; ffprobe: string }> => ({ ffmpeg: "ffmpeg", ffprobe: "ffprobe" });
|
||||
const twoTracksGerSecond = JSON.stringify({ streams: [{ tags: { language: "eng" } }, { tags: { language: "ger" } }] });
|
||||
|
||||
@ -226,7 +232,7 @@ describe("processVideoFile (real fs body, fake runner)", () => {
|
||||
expect(result.keptTrackIndex).toBe(1); // German was second
|
||||
expect(fs.readFileSync(file, "utf8")).toBe("REMUXED-GERMAN-ONLY"); // original overwritten
|
||||
expect(Math.abs(fs.statSync(file).mtimeMs - beforeMtime)).toBeLessThan(1500); // mtime preserved
|
||||
expect(fs.existsSync(`${file}.gertmp.mkv`)).toBe(false); // temp cleaned up
|
||||
expect(leftoverTemps(file)).toEqual([]); // unique temp cleaned up
|
||||
});
|
||||
|
||||
it("leaves the original intact and removes temp when ffmpeg fails", async () => {
|
||||
@ -238,7 +244,23 @@ describe("processVideoFile (real fs body, fake runner)", () => {
|
||||
|
||||
expect(result.action).toBe("error");
|
||||
expect(fs.readFileSync(file, "utf8")).toBe("ORIGINAL"); // never lost
|
||||
expect(fs.existsSync(`${file}.gertmp.mkv`)).toBe(false);
|
||||
expect(leftoverTemps(file)).toEqual([]);
|
||||
});
|
||||
|
||||
it("keeps the original intact and cleans the temp when the atomic replace rename fails (no zero-copy window)", async () => {
|
||||
// Simulate a Windows file lock that defeats the replace even after retries.
|
||||
// The original must survive: the old rm-then-rename fallback could leave the
|
||||
// file with NEITHER the original nor the remux on disk.
|
||||
const file = makeFile("ORIGINAL");
|
||||
const result = await processVideoFile(file, { mode: "tag" }, {
|
||||
resolveTooling: tooling,
|
||||
runProcess: fakeRunner({ probeJson: twoTracksGerSecond }),
|
||||
rename: async () => { throw Object.assign(new Error("locked"), { code: "EBUSY" }); }
|
||||
});
|
||||
|
||||
expect(result.action).toBe("error");
|
||||
expect(fs.readFileSync(file, "utf8")).toBe("ORIGINAL"); // original never destroyed
|
||||
expect(leftoverTemps(file)).toEqual([]); // remux temp removed
|
||||
});
|
||||
|
||||
it("does not touch a single-audio file (no remux)", async () => {
|
||||
@ -281,3 +303,35 @@ describe("processVideoFile (real fs body, fake runner)", () => {
|
||||
expect(fs.readFileSync(file, "utf8")).toBe("ORIGINAL");
|
||||
});
|
||||
});
|
||||
|
||||
describe("renameWithRetry", () => {
|
||||
afterEach(() => { vi.restoreAllMocks(); });
|
||||
const busy = (): NodeJS.ErrnoException => Object.assign(new Error("locked"), { code: "EBUSY" });
|
||||
|
||||
it("retries a transient EBUSY and then succeeds", async () => {
|
||||
let calls = 0;
|
||||
vi.spyOn(fs.promises, "rename").mockImplementation(async () => {
|
||||
calls += 1;
|
||||
if (calls <= 2) { throw busy(); }
|
||||
});
|
||||
await expect(renameWithRetry("a", "b")).resolves.toBeUndefined();
|
||||
expect(calls).toBe(3); // failed twice, succeeded on the third attempt
|
||||
});
|
||||
|
||||
it("gives up after exhausting retries on a persistent lock", async () => {
|
||||
let calls = 0;
|
||||
vi.spyOn(fs.promises, "rename").mockImplementation(async () => { calls += 1; throw busy(); });
|
||||
await expect(renameWithRetry("a", "b")).rejects.toThrow("locked");
|
||||
expect(calls).toBe(4); // initial attempt + 3 backoff retries
|
||||
});
|
||||
|
||||
it("does not retry a non-retryable error (e.g. EXDEV) — fails fast", async () => {
|
||||
let calls = 0;
|
||||
vi.spyOn(fs.promises, "rename").mockImplementation(async () => {
|
||||
calls += 1;
|
||||
throw Object.assign(new Error("cross-device"), { code: "EXDEV" });
|
||||
});
|
||||
await expect(renameWithRetry("a", "b")).rejects.toThrow("cross-device");
|
||||
expect(calls).toBe(1);
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user