diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f51243..745ab3a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # 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 - 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. diff --git a/README.md b/README.md index b3e77e2..a231f50 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ A local web UI for a system-wide `ani-cli` install. -Current version: `0.29.1` +Current version: `0.29.2` ## 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 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. +- 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 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`. @@ -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. +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 The queue is stored in SQLite and shown 10 jobs at a time. diff --git a/VERSION b/VERSION index 25939d3..20f0687 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.29.1 +0.29.2 diff --git a/app_support.py b/app_support.py index c22f591..c66db30 100644 --- a/app_support.py +++ b/app_support.py @@ -16,7 +16,7 @@ from uuid import uuid4 PROJECT_ROOT = Path(__file__).resolve().parent ANI_CLI = os.environ.get("ANI_CLI_BIN") or shutil.which("ani-cli") or "ani-cli" APP_NAME = "ani-cli-web" -VERSION = "0.29.1" +VERSION = "0.29.2" ALLANIME_BASE = "allanime.day" ALLANIME_API = f"https://api.{ALLANIME_BASE}" ALLANIME_REFERER = "https://allmanga.to" diff --git a/http_handler.py b/http_handler.py index 24d8702..db3cb65 100644 --- a/http_handler.py +++ b/http_handler.py @@ -255,11 +255,11 @@ class Handler(BaseHTTPRequestHandler): ) elif parsed.path == "/api/watchlist/update-status": 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"))) elif parsed.path == "/api/watchlist/update-category": 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"))) elif parsed.path == "/api/watchlist/update-all": Handler._runtime(self) @@ -268,11 +268,11 @@ class Handler(BaseHTTPRequestHandler): self.json(result, status) elif parsed.path == "/api/watchlist/remove": 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"])) elif parsed.path == "/api/watchlist/upload-thumbnail": 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)) else: self.error(HTTPStatus.NOT_FOUND, "Not found") @@ -286,6 +286,16 @@ class Handler(BaseHTTPRequestHandler): context.save_runtime_config(config) 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): try: length = int(self.headers.get("Content-Length", "0") or "0") diff --git a/queue_jobs.py b/queue_jobs.py index 918894b..8e8f524 100644 --- a/queue_jobs.py +++ b/queue_jobs.py @@ -768,6 +768,29 @@ class DownloadQueue: self._save_job_locked(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): while not self.stop_event.is_set(): job = self._next_pending() @@ -830,56 +853,59 @@ class DownloadQueue: job["pid"] = process.pid self._save_job_locked(job) - assert process.stdout is not None - for line in process.stdout: - self._append_log(job, line) - exit_code = process.wait() - finalize_error = None - moved_files = [] - watchlist_sync_error = None - cleanup_staging = False - if exit_code == 0 and not job.get("cancel_requested"): - try: - moved_files = finalize_library_files(job) - except Exception as exc: - finalize_error = exc - else: - if self.watchlist_sync_fn is not None: - try: - self.watchlist_sync_fn(job) - except Exception as exc: - watchlist_sync_error = exc + try: + assert process.stdout is not None + for line in process.stdout: + self._append_log(job, line) + exit_code = process.wait() + finalize_error = None + moved_files = [] + watchlist_sync_error = None + cleanup_staging = False + if exit_code == 0 and not job.get("cancel_requested"): + try: + moved_files = finalize_library_files(job) + except Exception as exc: + finalize_error = exc + else: + if self.watchlist_sync_fn is not None: + try: + self.watchlist_sync_fn(job) + except Exception as exc: + watchlist_sync_error = exc - with self.lock: - canceled = job.get("cancel_requested") - job["exit_code"] = 1 if finalize_error else exit_code - job["pid"] = None - job["finished_at"] = now_iso() - job["updated_at"] = job["finished_at"] - if canceled: - job["status"] = "canceled" - job.setdefault("log", []).append("Canceled.") - elif finalize_error: - job["status"] = "failed" - job.setdefault("log", []).append(f"Library rename failed: {finalize_error}") - elif exit_code == 0: - job["status"] = "done" - for path in moved_files: - job.setdefault("log", []).append(f"Saved: {path}") - job.setdefault("log", []).append("Download completed.") - if watchlist_sync_error: - job.setdefault("log", []).append(f"Watchlist sync failed: {watchlist_sync_error}") - elif self.watchlist_sync_fn is not None: - job.setdefault("log", []).append("Watchlist download state synced.") - else: - job["status"] = "failed" - job.setdefault("log", []).append(f"Download failed with exit code {exit_code}.") - cleanup_staging = True - if canceled: - cleanup_staging = True - self.current_process = None - self.current_job_id = None - self.current_job = None - self._save_job_locked(job) - if cleanup_staging: - self._cleanup_staging_dir(job) + with self.lock: + canceled = job.get("cancel_requested") + job["exit_code"] = 1 if finalize_error else exit_code + job["pid"] = None + job["finished_at"] = now_iso() + job["updated_at"] = job["finished_at"] + if canceled: + job["status"] = "canceled" + job.setdefault("log", []).append("Canceled.") + elif finalize_error: + job["status"] = "failed" + job.setdefault("log", []).append(f"Library rename failed: {finalize_error}") + elif exit_code == 0: + job["status"] = "done" + for path in moved_files: + job.setdefault("log", []).append(f"Saved: {path}") + job.setdefault("log", []).append("Download completed.") + if watchlist_sync_error: + job.setdefault("log", []).append(f"Watchlist sync failed: {watchlist_sync_error}") + elif self.watchlist_sync_fn is not None: + job.setdefault("log", []).append("Watchlist download state synced.") + else: + job["status"] = "failed" + job.setdefault("log", []).append(f"Download failed with exit code {exit_code}.") + cleanup_staging = True + if canceled: + cleanup_staging = True + self.current_process = None + self.current_job_id = None + self.current_job = None + self._save_job_locked(job) + if cleanup_staging: + self._cleanup_staging_dir(job) + except Exception as exc: + self._fail_job_runtime(job, exc, process=process) diff --git a/test_app.py b/test_app.py index 317a5c0..82b41d2 100644 --- a/test_app.py +++ b/test_app.py @@ -266,6 +266,39 @@ class DownloadQueueWorkerFailureTests(unittest.TestCase): self.assertEqual(stored["status"], "failed") 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): def test_refresh_all_iterates_every_show_id(self): @@ -1070,6 +1103,19 @@ class HandlerRouteTests(unittest.TestCase): APP.Handler.body_json(handler) 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): thumb_path = APP.THUMBNAIL_ROOT / "show-1.jpg" thumb_path.parent.mkdir(parents=True, exist_ok=True)