mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-08-25 23:11:26 -03:00
fix(download): stop stale aria2 GIDs from spamming errors after queue clears
- Log expected "GID not found" tellStatus probes at DEBUG, and treat a forgotten GID as permanent so the poll loop recovers immediately instead of burning 4 retries x 3s of ERROR lines per cycle - cancel_download tolerates a forgotten GID and always pops the in-memory transfer so concurrent polls cannot re-register a cancelled download - Restore sweep deletes aria2 state records with no resolvable target path instead of skipping them forever - Clearing the download queue now also cancels in-memory tasks, removes live aria2 transfers and drops persisted state for the cleared ids (partial files on disk are preserved)
This commit is contained in:
@@ -57,7 +57,7 @@ async def test_download_file_polls_until_complete(tmp_path, monkeypatch):
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
rpc_calls.append((method, params))
|
||||
if method == "aria2.addUri":
|
||||
return "gid-1"
|
||||
@@ -139,7 +139,7 @@ async def test_download_file_keeps_auth_headers_when_civitai_does_not_redirect(
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
rpc_calls.append((method, params))
|
||||
if method == "aria2.addUri":
|
||||
return "gid-1"
|
||||
@@ -178,7 +178,7 @@ async def test_pause_resume_cancel_forward_to_rpc(monkeypatch):
|
||||
|
||||
calls = []
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
calls.append((method, params))
|
||||
return "gid-1"
|
||||
|
||||
@@ -232,7 +232,7 @@ async def test_download_file_reuses_existing_transfer_without_add_uri(
|
||||
]
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
rpc_calls.append((method, params))
|
||||
if method == "aria2.tellStatus":
|
||||
return next(statuses)
|
||||
@@ -265,7 +265,7 @@ async def test_download_file_recovers_when_transfer_lost_mid_poll(
|
||||
add_uri_count = {"n": 0}
|
||||
poll_count = {"n": 0}
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
if method == "aria2.addUri":
|
||||
add_uri_count["n"] += 1
|
||||
return "gid-1" if add_uri_count["n"] == 1 else "gid-2"
|
||||
@@ -317,7 +317,7 @@ async def test_download_file_recovers_when_rpc_fails_mid_poll(tmp_path, monkeypa
|
||||
add_uri_count = {"n": 0}
|
||||
poll_count = {"n": 0}
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
if method == "aria2.addUri":
|
||||
add_uri_count["n"] += 1
|
||||
return "gid-1" if add_uri_count["n"] == 1 else "gid-2"
|
||||
@@ -366,7 +366,7 @@ async def test_download_file_fails_after_recovery_attempts_exhausted(
|
||||
save_path = tmp_path / "downloads" / "model.safetensors"
|
||||
add_uri_count = {"n": 0}
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
if method == "aria2.addUri":
|
||||
add_uri_count["n"] += 1
|
||||
return f"gid-{add_uri_count['n']}"
|
||||
@@ -402,7 +402,7 @@ async def test_download_file_concurrent_same_id_schedules_once(tmp_path, monkeyp
|
||||
add_uri_count = {"n": 0}
|
||||
poll_count = {"n": 0}
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
if method == "aria2.addUri":
|
||||
add_uri_count["n"] += 1
|
||||
return "gid-1"
|
||||
@@ -458,7 +458,7 @@ async def test_download_file_cleanup_preserves_newer_registration(tmp_path, monk
|
||||
save_path = tmp_path / "downloads" / "model.safetensors"
|
||||
poll_count = {"n": 0}
|
||||
|
||||
async def fake_rpc_call(method, params):
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
if method == "aria2.addUri":
|
||||
return "gid-1"
|
||||
if method == "aria2.tellStatus":
|
||||
@@ -808,3 +808,121 @@ def test_stderr_error_report_prunes_expired_entries():
|
||||
|
||||
assert old_line not in downloader._stderr_error_report
|
||||
assert new_line in downloader._stderr_error_report
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_status_returns_none_without_retry_when_gid_not_found(monkeypatch):
|
||||
"""A forgotten GID is permanent: no retry attempts on a dead GID."""
|
||||
downloader = Aria2Downloader()
|
||||
downloader._transfers["download-1"] = Aria2Transfer(
|
||||
gid="gone-gid", save_path="/tmp/model.safetensors"
|
||||
)
|
||||
|
||||
calls = []
|
||||
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
calls.append(method)
|
||||
raise Aria2Error("GID gone-gid is not found")
|
||||
|
||||
monkeypatch.setattr(downloader, "_rpc_call", fake_rpc_call)
|
||||
monkeypatch.setattr("py.services.aria2_downloader.asyncio.sleep", AsyncMock())
|
||||
|
||||
assert await downloader._get_status_with_retry("download-1") is None
|
||||
assert calls == ["aria2.tellStatus"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_status_still_raises_on_transient_rpc_error(monkeypatch):
|
||||
downloader = Aria2Downloader()
|
||||
downloader._transfers["download-1"] = Aria2Transfer(
|
||||
gid="gid-1", save_path="/tmp/model.safetensors"
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
raise Aria2Error("connection reset")
|
||||
|
||||
monkeypatch.setattr(downloader, "_rpc_call", fake_rpc_call)
|
||||
monkeypatch.setattr("py.services.aria2_downloader.asyncio.sleep", AsyncMock())
|
||||
|
||||
with pytest.raises(Aria2Error, match="Failed to query aria2 download status"):
|
||||
await downloader._get_status_with_retry("download-1")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_download_pops_transfer_on_success(monkeypatch):
|
||||
downloader = Aria2Downloader()
|
||||
downloader._transfers["download-1"] = Aria2Transfer(
|
||||
gid="gid-1", save_path="/tmp/model.safetensors"
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
return "gid-1"
|
||||
|
||||
monkeypatch.setattr(downloader, "_rpc_call", fake_rpc_call)
|
||||
|
||||
result = await downloader.cancel_download("download-1")
|
||||
|
||||
assert result["success"] is True
|
||||
assert "download-1" not in downloader._transfers
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_download_tolerates_missing_gid(monkeypatch):
|
||||
"""Cancelling a transfer the daemon already forgot still succeeds."""
|
||||
downloader = Aria2Downloader()
|
||||
downloader._transfers["download-1"] = Aria2Transfer(
|
||||
gid="gone-gid", save_path="/tmp/model.safetensors"
|
||||
)
|
||||
await downloader._state_store.upsert(
|
||||
"download-1", {"gid": "gone-gid", "status": "downloading"}
|
||||
)
|
||||
|
||||
async def fake_rpc_call(method, params, **_kwargs):
|
||||
raise Aria2Error("GID gone-gid is not found")
|
||||
|
||||
monkeypatch.setattr(downloader, "_rpc_call", fake_rpc_call)
|
||||
|
||||
result = await downloader.cancel_download("download-1")
|
||||
|
||||
assert result["success"] is True
|
||||
assert "download-1" not in downloader._transfers
|
||||
assert await downloader._state_store.get("download-1") is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rpc_call_suppresses_error_log_when_log_errors_false(
|
||||
monkeypatch, caplog
|
||||
):
|
||||
"""Probing calls must not spam ERROR for an expected failure."""
|
||||
|
||||
class FakeResponse:
|
||||
status = 400
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *exc):
|
||||
return False
|
||||
|
||||
async def text(self):
|
||||
return '{"jsonrpc": "2.0", "error": {"code": 1, "message": "GID x is not found"}}'
|
||||
|
||||
class FakeSession:
|
||||
closed = False
|
||||
|
||||
def post(self, *args, **kwargs):
|
||||
return FakeResponse()
|
||||
|
||||
downloader = Aria2Downloader()
|
||||
downloader._rpc_url = "http://127.0.0.1/jsonrpc"
|
||||
downloader._rpc_secret = "secret"
|
||||
monkeypatch.setattr(downloader, "_get_rpc_session", AsyncMock(return_value=FakeSession()))
|
||||
|
||||
with caplog.at_level(logging.DEBUG, logger="py.services.aria2_downloader"):
|
||||
with pytest.raises(Aria2Error, match="not found"):
|
||||
await downloader._rpc_call("aria2.tellStatus", ["x"], log_errors=False)
|
||||
|
||||
error_records = [r for r in caplog.records if r.levelno == logging.ERROR]
|
||||
debug_records = [r for r in caplog.records if r.levelno == logging.DEBUG]
|
||||
assert error_records == []
|
||||
assert any("GID x is not found" in r.message for r in debug_records)
|
||||
|
||||
@@ -2003,3 +2003,98 @@ def test_resolve_target_file_returns_none_for_no_match():
|
||||
assert DownloadManager._resolve_target_file(files, {"id": 9999}) is None
|
||||
assert DownloadManager._resolve_target_file(files, None) is None
|
||||
assert DownloadManager._resolve_target_file(files, {}) is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restore_drops_unrestorable_persisted_records(monkeypatch, tmp_path):
|
||||
"""Records without any resolvable target path can never be restored;
|
||||
the restore sweep must delete them instead of skipping them forever."""
|
||||
manager = DownloadManager()
|
||||
|
||||
await manager._aria2_state_store.upsert(
|
||||
"download-orphan",
|
||||
{
|
||||
"download_id": "download-orphan",
|
||||
"transfer_backend": "aria2",
|
||||
"status": "failed",
|
||||
# no save_path / file_path / resume_context
|
||||
},
|
||||
)
|
||||
|
||||
class DummyAria2Downloader:
|
||||
async def get_status_by_gid(self, gid):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(
|
||||
download_manager,
|
||||
"get_aria2_downloader",
|
||||
AsyncMock(return_value=DummyAria2Downloader()),
|
||||
)
|
||||
|
||||
downloads = await manager.get_active_downloads()
|
||||
|
||||
assert downloads["downloads"] == []
|
||||
assert await manager._aria2_state_store.get("download-orphan") is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_discard_cleared_downloads_stops_tracking_and_preserves_files(
|
||||
monkeypatch, tmp_path
|
||||
):
|
||||
manager = DownloadManager()
|
||||
|
||||
save_path = tmp_path / "file.safetensors"
|
||||
save_path.write_text("partial")
|
||||
control_path = tmp_path / "file.safetensors.aria2"
|
||||
control_path.write_text("control")
|
||||
|
||||
async def _pending():
|
||||
await asyncio.sleep(3600)
|
||||
|
||||
task = asyncio.create_task(_pending())
|
||||
manager._download_tasks["download-1"] = task
|
||||
manager._pause_events["download-1"] = download_manager.DownloadStreamControl()
|
||||
manager._active_downloads["download-1"] = {
|
||||
"status": "downloading",
|
||||
"transfer_backend": "aria2",
|
||||
"file_path": str(save_path),
|
||||
}
|
||||
await manager._aria2_state_store.upsert(
|
||||
"download-1",
|
||||
{
|
||||
"download_id": "download-1",
|
||||
"transfer_backend": "aria2",
|
||||
"status": "downloading",
|
||||
"save_path": str(save_path),
|
||||
"gid": "gid-1",
|
||||
},
|
||||
)
|
||||
|
||||
cancelled = []
|
||||
|
||||
class DummyAria2Downloader:
|
||||
async def has_transfer(self, download_id):
|
||||
return True
|
||||
|
||||
async def cancel_download(self, download_id):
|
||||
cancelled.append(download_id)
|
||||
return {"success": True}
|
||||
|
||||
monkeypatch.setattr(
|
||||
download_manager,
|
||||
"get_aria2_downloader",
|
||||
AsyncMock(return_value=DummyAria2Downloader()),
|
||||
)
|
||||
|
||||
discarded = await manager.discard_cleared_downloads(["download-1", "unknown-id"])
|
||||
|
||||
assert discarded == 1
|
||||
assert cancelled == ["download-1"]
|
||||
assert task.cancelled()
|
||||
assert "download-1" not in manager._download_tasks
|
||||
assert "download-1" not in manager._active_downloads
|
||||
assert "download-1" not in manager._pause_events
|
||||
assert await manager._aria2_state_store.get("download-1") is None
|
||||
# Partial files are preserved for a future resume from disk.
|
||||
assert save_path.exists()
|
||||
assert control_path.exists()
|
||||
|
||||
@@ -495,3 +495,29 @@ async def test_dedup_history_collapses_same_file_and_legacy_rows(tmp_path: Path)
|
||||
history = await svc.get_history()
|
||||
remaining = {item["download_id"] for item in history["items"]}
|
||||
assert remaining == {"dl-x2", "dl-y2"}
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# clear_queue
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_clear_queue_returns_deleted_ids(tmp_path: Path) -> None:
|
||||
"""clear_queue reports the download_ids it removed so callers can tear
|
||||
down any in-memory tracking for them."""
|
||||
svc = _make_service(tmp_path)
|
||||
await svc.add_to_queue(download_id="dl-1", model_id=1)
|
||||
await svc.add_to_queue(download_id="dl-2", model_id=2)
|
||||
await svc.add_to_queue(download_id="dl-3", model_id=3)
|
||||
await svc.update_status("dl-3", "downloading")
|
||||
|
||||
cleared = await svc.clear_queue(status_filter="queued")
|
||||
assert sorted(cleared) == ["dl-1", "dl-2"]
|
||||
|
||||
remaining = await svc.get_queue()
|
||||
assert [row["download_id"] for row in remaining] == ["dl-3"]
|
||||
|
||||
cleared_all = await svc.clear_queue()
|
||||
assert cleared_all == ["dl-3"]
|
||||
assert await svc.get_queue() == []
|
||||
|
||||
Reference in New Issue
Block a user