mirror of
https://github.com/unshackle-dl/unshackle.git
synced 2026-11-12 20:15:10 +00:00
fix(tracks): keep a locked segment file from hiding the download error
On Windows a worker can still hold a .!dev file open when a track fails; cleanup now ignores that OSError and still stashes the segments.
This commit is contained in:
@@ -2,7 +2,9 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import os
|
||||
import shutil
|
||||
import time
|
||||
from functools import partial
|
||||
from pathlib import Path
|
||||
@@ -78,6 +80,12 @@ def failing_parser(track, ctx):
|
||||
raise RuntimeError("simulated crash")
|
||||
|
||||
|
||||
def cancelled_parser(track, ctx):
|
||||
seed(ctx)
|
||||
(ctx.save_dir / "1.mp4.p.!dev").write_bytes(b"partial")
|
||||
raise DownloadCancelled
|
||||
|
||||
|
||||
def test_failed_track_resumes_in_next_run(temp_root, monkeypatch):
|
||||
monkeypatch.setattr(config, "continue_downloads", True)
|
||||
|
||||
@@ -170,6 +178,40 @@ def test_rename_failure_never_replaces_download_error(temp_root, monkeypatch):
|
||||
run_download(monkeypatch, failing_parser)
|
||||
|
||||
|
||||
def test_locked_segment_never_replaces_download_error(temp_root, monkeypatch):
|
||||
monkeypatch.setattr(config, "continue_downloads", False)
|
||||
|
||||
rmtree = shutil.rmtree
|
||||
|
||||
def locked(path, *args, **kwargs):
|
||||
if Path(path).name == SEG_NAME:
|
||||
raise PermissionError(32, "being used by another process", str(path))
|
||||
rmtree(path, *args, **kwargs)
|
||||
|
||||
monkeypatch.setattr(shutil, "rmtree", locked)
|
||||
with task_temp_dir():
|
||||
with pytest.raises(RuntimeError, match="simulated crash"):
|
||||
run_download(monkeypatch, failing_parser)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("parser", [failing_parser, cancelled_parser])
|
||||
def test_locked_partial_still_stashes(temp_root, monkeypatch, parser):
|
||||
monkeypatch.setattr(config, "continue_downloads", True)
|
||||
unlink = Path.unlink
|
||||
|
||||
def locked(self, missing_ok=False):
|
||||
if self.name.endswith(".!dev"):
|
||||
raise PermissionError(32, "being used by another process", str(self))
|
||||
unlink(self, missing_ok=missing_ok)
|
||||
|
||||
with task_temp_dir():
|
||||
monkeypatch.setattr(Path, "unlink", locked)
|
||||
with contextlib.suppress(RuntimeError):
|
||||
run_download(monkeypatch, parser)
|
||||
|
||||
assert (temp_root / RESUME_STASH / SEG_NAME / "0.mp4").read_bytes() == b"complete segment"
|
||||
|
||||
|
||||
def test_sweep_removes_only_old_stash_entries(temp_root):
|
||||
stash = temp_root / RESUME_STASH
|
||||
old, fresh = stash / "old_segments", stash / "fresh_segments"
|
||||
@@ -195,10 +237,6 @@ def test_no_stash_outside_task_dir(temp_root):
|
||||
def test_cancelled_track_is_stashed(temp_root, monkeypatch):
|
||||
monkeypatch.setattr(config, "continue_downloads", True)
|
||||
|
||||
def cancelled_parser(track, ctx):
|
||||
seed(ctx)
|
||||
raise DownloadCancelled
|
||||
|
||||
with task_temp_dir():
|
||||
run_download(monkeypatch, cancelled_parser)
|
||||
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import base64
|
||||
import contextlib
|
||||
import html
|
||||
import logging
|
||||
import re
|
||||
@@ -768,6 +769,17 @@ class Track:
|
||||
shutil.rmtree(save_dir)
|
||||
resume.clear_sidecar(save_dir)
|
||||
|
||||
def abandon() -> None:
|
||||
"""Clean up a failed or cancelled download and stash its segments when resume is on.
|
||||
|
||||
The downloader does not wait for its workers on failure, so on Windows one can still hold a
|
||||
.!dev file open (WinError 32). A cleanup error must not replace the download's own error.
|
||||
"""
|
||||
with contextlib.suppress(OSError):
|
||||
cleanup()
|
||||
if keep_segments:
|
||||
stash_segments(save_dir)
|
||||
|
||||
if not DOWNLOAD_LICENCE_ONLY.is_set():
|
||||
if config.directories.temp.is_file():
|
||||
raise ValueError(f"Temp Directory '{config.directories.temp}' must be a Directory, not a file")
|
||||
@@ -927,19 +939,12 @@ class Track:
|
||||
progress(downloaded="[red]FAILED")
|
||||
raise
|
||||
except DownloadCancelled:
|
||||
try:
|
||||
cleanup()
|
||||
if keep_segments:
|
||||
stash_segments(save_dir)
|
||||
except OSError:
|
||||
pass
|
||||
abandon()
|
||||
progress(downloaded="[yellow]SKIPPED")
|
||||
return
|
||||
except (Exception, KeyboardInterrupt):
|
||||
if not DOWNLOAD_LICENCE_ONLY.is_set():
|
||||
cleanup()
|
||||
if keep_segments:
|
||||
stash_segments(save_dir)
|
||||
abandon()
|
||||
raise
|
||||
|
||||
if DOWNLOAD_CANCELLED.is_set():
|
||||
|
||||
Reference in New Issue
Block a user