Harden queue worker crashes and POST validation
This commit is contained in:
@@ -1,5 +1,11 @@
|
|||||||
# Changelog
|
# Changelog
|
||||||
|
|
||||||
|
## 0.29.2 - 2026-05-16
|
||||||
|
|
||||||
|
- Hardened the download queue again so unexpected post-launch worker exceptions now fail the active job cleanly instead of killing the worker thread and leaving the job stuck in `running`.
|
||||||
|
- Fixed malformed JSON POST handling for watchlist action routes so missing required fields such as `show_id` and `data_url` now return `400 Bad Request` instead of surfacing as misleading `404 Not Found` errors.
|
||||||
|
- Expanded regression coverage for post-launch queue worker failures and required-field validation on watchlist POST endpoints.
|
||||||
|
|
||||||
## 0.29.1 - 2026-05-16
|
## 0.29.1 - 2026-05-16
|
||||||
|
|
||||||
- Hardened download queue startup so a failed `ani-cli` process launch now marks the job as failed instead of crashing the background queue worker thread.
|
- Hardened download queue startup so a failed `ani-cli` process launch now marks the job as failed instead of crashing the background queue worker thread.
|
||||||
|
|||||||
@@ -2,7 +2,7 @@
|
|||||||
|
|
||||||
A local web UI for a system-wide `ani-cli` install.
|
A local web UI for a system-wide `ani-cli` install.
|
||||||
|
|
||||||
Current version: `0.29.1`
|
Current version: `0.29.2`
|
||||||
|
|
||||||
## Project Layout
|
## Project Layout
|
||||||
|
|
||||||
@@ -86,6 +86,7 @@ The app currently includes:
|
|||||||
- Queue log writes are batched during active downloads to reduce SQLite churn on noisy jobs.
|
- Queue log writes are batched during active downloads to reduce SQLite churn on noisy jobs.
|
||||||
- Queue persistence now stores core job metadata and logs in structured SQLite columns instead of treating the whole job as one large JSON blob.
|
- Queue persistence now stores core job metadata and logs in structured SQLite columns instead of treating the whole job as one large JSON blob.
|
||||||
- Queue worker startup failures now degrade into a normal failed job state instead of crashing the background queue thread.
|
- Queue worker startup failures now degrade into a normal failed job state instead of crashing the background queue thread.
|
||||||
|
- Unexpected post-launch queue worker failures now also fail the active job cleanly instead of leaving it stuck in `running`.
|
||||||
- Failed and canceled downloads now clean their project-local staging directories automatically so partial files do not accumulate under `.ani-cli-web/staging/`.
|
- Failed and canceled downloads now clean their project-local staging directories automatically so partial files do not accumulate under `.ani-cli-web/staging/`.
|
||||||
- Failed thumbnail refresh attempts now record a retry timestamp, so unresolved covers back off instead of immediately requerying upstream sources on every later warm-up.
|
- Failed thumbnail refresh attempts now record a retry timestamp, so unresolved covers back off instead of immediately requerying upstream sources on every later warm-up.
|
||||||
- Download-to-watchlist fallback matching now keeps season and part identity intact while still tolerating equivalent season notation such as `2nd Season` vs `Season 2`.
|
- Download-to-watchlist fallback matching now keeps season and part identity intact while still tolerating equivalent season notation such as `2nd Season` vs `Season 2`.
|
||||||
@@ -140,6 +141,8 @@ When the app is placed behind a loopback reverse proxy, forwarded external clien
|
|||||||
|
|
||||||
This keeps the queue, config, and watchlist APIs safer when the app is intentionally exposed beyond localhost.
|
This keeps the queue, config, and watchlist APIs safer when the app is intentionally exposed beyond localhost.
|
||||||
|
|
||||||
|
Malformed watchlist action payloads now return `400 Bad Request` when required JSON fields are missing, instead of surfacing those client mistakes as `404 Not Found`.
|
||||||
|
|
||||||
## Download Queue
|
## Download Queue
|
||||||
|
|
||||||
The queue is stored in SQLite and shown 10 jobs at a time.
|
The queue is stored in SQLite and shown 10 jobs at a time.
|
||||||
|
|||||||
+1
-1
@@ -16,7 +16,7 @@ from uuid import uuid4
|
|||||||
PROJECT_ROOT = Path(__file__).resolve().parent
|
PROJECT_ROOT = Path(__file__).resolve().parent
|
||||||
ANI_CLI = os.environ.get("ANI_CLI_BIN") or shutil.which("ani-cli") or "ani-cli"
|
ANI_CLI = os.environ.get("ANI_CLI_BIN") or shutil.which("ani-cli") or "ani-cli"
|
||||||
APP_NAME = "ani-cli-web"
|
APP_NAME = "ani-cli-web"
|
||||||
VERSION = "0.29.1"
|
VERSION = "0.29.2"
|
||||||
ALLANIME_BASE = "allanime.day"
|
ALLANIME_BASE = "allanime.day"
|
||||||
ALLANIME_API = f"https://api.{ALLANIME_BASE}"
|
ALLANIME_API = f"https://api.{ALLANIME_BASE}"
|
||||||
ALLANIME_REFERER = "https://allmanga.to"
|
ALLANIME_REFERER = "https://allmanga.to"
|
||||||
|
|||||||
+14
-4
@@ -255,11 +255,11 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
)
|
)
|
||||||
elif parsed.path == "/api/watchlist/update-status":
|
elif parsed.path == "/api/watchlist/update-status":
|
||||||
Handler._runtime(self)
|
Handler._runtime(self)
|
||||||
payload = self.body_json()
|
payload = Handler.require_fields(self, self.body_json(), "show_id")
|
||||||
self.json(Handler._context(self).update_watchlist_status(payload["show_id"], payload.get("mode")))
|
self.json(Handler._context(self).update_watchlist_status(payload["show_id"], payload.get("mode")))
|
||||||
elif parsed.path == "/api/watchlist/update-category":
|
elif parsed.path == "/api/watchlist/update-category":
|
||||||
Handler._runtime(self)
|
Handler._runtime(self)
|
||||||
payload = self.body_json()
|
payload = Handler.require_fields(self, self.body_json(), "show_id")
|
||||||
self.json(Handler._context(self).update_watchlist_category(payload["show_id"], payload.get("category")))
|
self.json(Handler._context(self).update_watchlist_category(payload["show_id"], payload.get("category")))
|
||||||
elif parsed.path == "/api/watchlist/update-all":
|
elif parsed.path == "/api/watchlist/update-all":
|
||||||
Handler._runtime(self)
|
Handler._runtime(self)
|
||||||
@@ -268,11 +268,11 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
self.json(result, status)
|
self.json(result, status)
|
||||||
elif parsed.path == "/api/watchlist/remove":
|
elif parsed.path == "/api/watchlist/remove":
|
||||||
Handler._runtime(self)
|
Handler._runtime(self)
|
||||||
payload = self.body_json()
|
payload = Handler.require_fields(self, self.body_json(), "show_id")
|
||||||
self.json(Handler._context(self).remove_from_watchlist(payload["show_id"]))
|
self.json(Handler._context(self).remove_from_watchlist(payload["show_id"]))
|
||||||
elif parsed.path == "/api/watchlist/upload-thumbnail":
|
elif parsed.path == "/api/watchlist/upload-thumbnail":
|
||||||
Handler._runtime(self)
|
Handler._runtime(self)
|
||||||
payload = self.body_json()
|
payload = Handler.require_fields(self, self.body_json(), "show_id", "data_url")
|
||||||
self.json(Handler._context(self).upload_watchlist_thumbnail(payload))
|
self.json(Handler._context(self).upload_watchlist_thumbnail(payload))
|
||||||
else:
|
else:
|
||||||
self.error(HTTPStatus.NOT_FOUND, "Not found")
|
self.error(HTTPStatus.NOT_FOUND, "Not found")
|
||||||
@@ -286,6 +286,16 @@ class Handler(BaseHTTPRequestHandler):
|
|||||||
context.save_runtime_config(config)
|
context.save_runtime_config(config)
|
||||||
self.json(context.get_config_snapshot())
|
self.json(context.get_config_snapshot())
|
||||||
|
|
||||||
|
def require_fields(self, payload, *names):
|
||||||
|
if not isinstance(payload, dict):
|
||||||
|
raise ValueError("Expected a JSON object")
|
||||||
|
missing = [name for name in names if not str(payload.get(name) or "").strip()]
|
||||||
|
if missing:
|
||||||
|
if len(missing) == 1:
|
||||||
|
raise ValueError(f"Missing required field: {missing[0]}")
|
||||||
|
raise ValueError(f"Missing required fields: {', '.join(missing)}")
|
||||||
|
return payload
|
||||||
|
|
||||||
def body_json(self):
|
def body_json(self):
|
||||||
try:
|
try:
|
||||||
length = int(self.headers.get("Content-Length", "0") or "0")
|
length = int(self.headers.get("Content-Length", "0") or "0")
|
||||||
|
|||||||
+78
-52
@@ -768,6 +768,29 @@ class DownloadQueue:
|
|||||||
self._save_job_locked(job)
|
self._save_job_locked(job)
|
||||||
self._cleanup_staging_dir(job)
|
self._cleanup_staging_dir(job)
|
||||||
|
|
||||||
|
def _fail_job_runtime(self, job, exc, process=None):
|
||||||
|
if process is not None:
|
||||||
|
try:
|
||||||
|
os.killpg(os.getpgid(process.pid), signal.SIGTERM)
|
||||||
|
except OSError:
|
||||||
|
try:
|
||||||
|
process.terminate()
|
||||||
|
except OSError:
|
||||||
|
pass
|
||||||
|
message = f"Download worker failed unexpectedly: {exc}"
|
||||||
|
with self.lock:
|
||||||
|
job["status"] = "failed"
|
||||||
|
job["exit_code"] = 1
|
||||||
|
job["pid"] = None
|
||||||
|
job["finished_at"] = now_iso()
|
||||||
|
job["updated_at"] = job["finished_at"]
|
||||||
|
job.setdefault("log", []).append(message)
|
||||||
|
self.current_process = None
|
||||||
|
self.current_job_id = None
|
||||||
|
self.current_job = None
|
||||||
|
self._save_job_locked(job)
|
||||||
|
self._cleanup_staging_dir(job)
|
||||||
|
|
||||||
def _run(self):
|
def _run(self):
|
||||||
while not self.stop_event.is_set():
|
while not self.stop_event.is_set():
|
||||||
job = self._next_pending()
|
job = self._next_pending()
|
||||||
@@ -830,56 +853,59 @@ class DownloadQueue:
|
|||||||
job["pid"] = process.pid
|
job["pid"] = process.pid
|
||||||
self._save_job_locked(job)
|
self._save_job_locked(job)
|
||||||
|
|
||||||
assert process.stdout is not None
|
try:
|
||||||
for line in process.stdout:
|
assert process.stdout is not None
|
||||||
self._append_log(job, line)
|
for line in process.stdout:
|
||||||
exit_code = process.wait()
|
self._append_log(job, line)
|
||||||
finalize_error = None
|
exit_code = process.wait()
|
||||||
moved_files = []
|
finalize_error = None
|
||||||
watchlist_sync_error = None
|
moved_files = []
|
||||||
cleanup_staging = False
|
watchlist_sync_error = None
|
||||||
if exit_code == 0 and not job.get("cancel_requested"):
|
cleanup_staging = False
|
||||||
try:
|
if exit_code == 0 and not job.get("cancel_requested"):
|
||||||
moved_files = finalize_library_files(job)
|
try:
|
||||||
except Exception as exc:
|
moved_files = finalize_library_files(job)
|
||||||
finalize_error = exc
|
except Exception as exc:
|
||||||
else:
|
finalize_error = exc
|
||||||
if self.watchlist_sync_fn is not None:
|
else:
|
||||||
try:
|
if self.watchlist_sync_fn is not None:
|
||||||
self.watchlist_sync_fn(job)
|
try:
|
||||||
except Exception as exc:
|
self.watchlist_sync_fn(job)
|
||||||
watchlist_sync_error = exc
|
except Exception as exc:
|
||||||
|
watchlist_sync_error = exc
|
||||||
|
|
||||||
with self.lock:
|
with self.lock:
|
||||||
canceled = job.get("cancel_requested")
|
canceled = job.get("cancel_requested")
|
||||||
job["exit_code"] = 1 if finalize_error else exit_code
|
job["exit_code"] = 1 if finalize_error else exit_code
|
||||||
job["pid"] = None
|
job["pid"] = None
|
||||||
job["finished_at"] = now_iso()
|
job["finished_at"] = now_iso()
|
||||||
job["updated_at"] = job["finished_at"]
|
job["updated_at"] = job["finished_at"]
|
||||||
if canceled:
|
if canceled:
|
||||||
job["status"] = "canceled"
|
job["status"] = "canceled"
|
||||||
job.setdefault("log", []).append("Canceled.")
|
job.setdefault("log", []).append("Canceled.")
|
||||||
elif finalize_error:
|
elif finalize_error:
|
||||||
job["status"] = "failed"
|
job["status"] = "failed"
|
||||||
job.setdefault("log", []).append(f"Library rename failed: {finalize_error}")
|
job.setdefault("log", []).append(f"Library rename failed: {finalize_error}")
|
||||||
elif exit_code == 0:
|
elif exit_code == 0:
|
||||||
job["status"] = "done"
|
job["status"] = "done"
|
||||||
for path in moved_files:
|
for path in moved_files:
|
||||||
job.setdefault("log", []).append(f"Saved: {path}")
|
job.setdefault("log", []).append(f"Saved: {path}")
|
||||||
job.setdefault("log", []).append("Download completed.")
|
job.setdefault("log", []).append("Download completed.")
|
||||||
if watchlist_sync_error:
|
if watchlist_sync_error:
|
||||||
job.setdefault("log", []).append(f"Watchlist sync failed: {watchlist_sync_error}")
|
job.setdefault("log", []).append(f"Watchlist sync failed: {watchlist_sync_error}")
|
||||||
elif self.watchlist_sync_fn is not None:
|
elif self.watchlist_sync_fn is not None:
|
||||||
job.setdefault("log", []).append("Watchlist download state synced.")
|
job.setdefault("log", []).append("Watchlist download state synced.")
|
||||||
else:
|
else:
|
||||||
job["status"] = "failed"
|
job["status"] = "failed"
|
||||||
job.setdefault("log", []).append(f"Download failed with exit code {exit_code}.")
|
job.setdefault("log", []).append(f"Download failed with exit code {exit_code}.")
|
||||||
cleanup_staging = True
|
cleanup_staging = True
|
||||||
if canceled:
|
if canceled:
|
||||||
cleanup_staging = True
|
cleanup_staging = True
|
||||||
self.current_process = None
|
self.current_process = None
|
||||||
self.current_job_id = None
|
self.current_job_id = None
|
||||||
self.current_job = None
|
self.current_job = None
|
||||||
self._save_job_locked(job)
|
self._save_job_locked(job)
|
||||||
if cleanup_staging:
|
if cleanup_staging:
|
||||||
self._cleanup_staging_dir(job)
|
self._cleanup_staging_dir(job)
|
||||||
|
except Exception as exc:
|
||||||
|
self._fail_job_runtime(job, exc, process=process)
|
||||||
|
|||||||
+46
@@ -266,6 +266,39 @@ class DownloadQueueWorkerFailureTests(unittest.TestCase):
|
|||||||
self.assertEqual(stored["status"], "failed")
|
self.assertEqual(stored["status"], "failed")
|
||||||
self.assertFalse(staging_dir.exists())
|
self.assertFalse(staging_dir.exists())
|
||||||
|
|
||||||
|
def test_run_job_marks_post_start_exception_failed_without_killing_worker_state(self):
|
||||||
|
queue = APP.DownloadQueue(lambda: {"mode": "sub", "quality": "best", "download_dir": "/tmp/example"}, start_worker=False)
|
||||||
|
job = queue.add({"query": "Queue Show", "title": "Queue Show", "season": "1", "episodes": "1"})
|
||||||
|
staging_dir = queue_jobs.job_staging_dir(job)
|
||||||
|
|
||||||
|
class BrokenStdout:
|
||||||
|
def __iter__(self):
|
||||||
|
return self
|
||||||
|
|
||||||
|
def __next__(self):
|
||||||
|
raise UnicodeDecodeError("utf-8", b"\xff", 0, 1, "bad output")
|
||||||
|
|
||||||
|
class FakeProc:
|
||||||
|
pid = 4321
|
||||||
|
stdout = BrokenStdout()
|
||||||
|
|
||||||
|
def terminate(self):
|
||||||
|
return None
|
||||||
|
|
||||||
|
with mock.patch.object(queue_jobs.subprocess, "Popen", return_value=FakeProc()), mock.patch.object(
|
||||||
|
queue_jobs.os, "getpgid", side_effect=OSError("gone")
|
||||||
|
):
|
||||||
|
queue._run_job(job)
|
||||||
|
|
||||||
|
stored = queue._find(job["id"])
|
||||||
|
self.assertEqual(stored["status"], "failed")
|
||||||
|
self.assertEqual(stored["exit_code"], 1)
|
||||||
|
self.assertIn("unexpectedly", stored["log"][-1].lower())
|
||||||
|
self.assertIsNone(queue.current_process)
|
||||||
|
self.assertIsNone(queue.current_job_id)
|
||||||
|
self.assertIsNone(queue.current_job)
|
||||||
|
self.assertFalse(staging_dir.exists())
|
||||||
|
|
||||||
|
|
||||||
class WatchlistRefreshAllTests(unittest.TestCase):
|
class WatchlistRefreshAllTests(unittest.TestCase):
|
||||||
def test_refresh_all_iterates_every_show_id(self):
|
def test_refresh_all_iterates_every_show_id(self):
|
||||||
@@ -1070,6 +1103,19 @@ class HandlerRouteTests(unittest.TestCase):
|
|||||||
APP.Handler.body_json(handler)
|
APP.Handler.body_json(handler)
|
||||||
self.assertIn("utf-8", str(ctx.exception).lower())
|
self.assertIn("utf-8", str(ctx.exception).lower())
|
||||||
|
|
||||||
|
def test_remove_post_missing_show_id_returns_bad_request(self):
|
||||||
|
handler = DummyHandler("/api/watchlist/remove", body=b"{}", content_length=2)
|
||||||
|
APP.Handler.do_POST(handler)
|
||||||
|
self.assertEqual(handler.error_status, HTTPStatus.BAD_REQUEST)
|
||||||
|
self.assertIn("show_id", handler.error_message)
|
||||||
|
|
||||||
|
def test_upload_thumbnail_post_missing_required_fields_returns_bad_request(self):
|
||||||
|
body = b'{"show_id":"show-1"}'
|
||||||
|
handler = DummyHandler("/api/watchlist/upload-thumbnail", body=body, content_length=len(body))
|
||||||
|
APP.Handler.do_POST(handler)
|
||||||
|
self.assertEqual(handler.error_status, HTTPStatus.BAD_REQUEST)
|
||||||
|
self.assertIn("data_url", handler.error_message)
|
||||||
|
|
||||||
def test_thumbnail_route_serves_cached_file_without_fetching(self):
|
def test_thumbnail_route_serves_cached_file_without_fetching(self):
|
||||||
thumb_path = APP.THUMBNAIL_ROOT / "show-1.jpg"
|
thumb_path = APP.THUMBNAIL_ROOT / "show-1.jpg"
|
||||||
thumb_path.parent.mkdir(parents=True, exist_ok=True)
|
thumb_path.parent.mkdir(parents=True, exist_ok=True)
|
||||||
|
|||||||
Reference in New Issue
Block a user