From 739e5ed4edd3f2897aa07ad27b66d4ff77c0509c Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:21:59 -0500 Subject: [PATCH 01/33] fix(postproc): deliver archives for download-only and repair-only jobs A job at post-processing level 0 (download only) or 1 (repair only) whose payload is archives ended Failed with "No usable output produced; only archive or PAR2 artifacts remain" and nothing was delivered. That rule assumes the job was meant to be unpacked, which only holds for levels 2 and 3. Follow SABnzbd's semantics: - pp=0: no verify, repair or unpack. The job completes and the raw files are moved to the complete dir, even when some articles were missing (a skipped Verify stage records how many). Hopeless jobs are still failed earlier by the download path. - pp=1: PAR2 verify/repair only, via the new nzb_postproc::run_repair_pipeline, which skips extraction and cleanup. Direct unpack is not started for levels below 2. - The usable-output check runs only for levels 2 and 3. History now records the effective level (job pp override, else the category, else 3) in a new history.post_processing column (migration v14). SAB history slots report it through sab_pp_label, SABnzbd's PP_LOOKUP (0 -> "", 1 -> "R", 2 -> "U", 3 -> "D"), instead of a fixed "D"; rows written before v14 keep "D". Post-processing slots use the live level. Fixes #97 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit a4e9a1db1e6c737de7188cc4189dff1e8f3b36c0) --- crates/nzb-core/src/db.rs | 59 +++- crates/nzb-core/src/models.rs | 5 + crates/nzb-postproc/src/lib.rs | 2 +- crates/nzb-postproc/src/pipeline.rs | 62 ++++- crates/nzb-web/src/queue_manager.rs | 254 ++++++++++++++++-- crates/nzb-web/src/sabnzbd_compat.rs | 59 +++- .../tests/harness_incomplete_cleanup.rs | 1 + 7 files changed, 406 insertions(+), 36 deletions(-) diff --git a/crates/nzb-core/src/db.rs b/crates/nzb-core/src/db.rs index 70c1e76f..91039d5d 100644 --- a/crates/nzb-core/src/db.rs +++ b/crates/nzb-core/src/db.rs @@ -405,6 +405,26 @@ impl Database { )?; } + if version < 14 { + info!("Applying database migration v14: history post-processing level"); + // Existing rows keep NULL: their level was never recorded. + let has_history: i64 = self.conn.query_row( + "SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = 'history'", + [], + |row| row.get(0), + )?; + if has_history > 0 { + self.conn + .execute_batch("ALTER TABLE history ADD COLUMN post_processing INTEGER;")?; + } + self.conn.execute_batch( + " + DELETE FROM schema_version; + INSERT INTO schema_version (version) VALUES (14); + ", + )?; + } + Ok(()) } @@ -740,8 +760,9 @@ impl Database { self.conn.execute( "INSERT INTO history (id, name, category, status, total_bytes, downloaded_bytes, added_at, completed_at, download_time_secs, output_dir, stages, error_message, - nzb_data, server_stats, retry_data, failure_code) - VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16)", + nzb_data, server_stats, retry_data, failure_code, post_processing) + VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13, ?14, ?15, ?16, + ?17)", params![ entry.id, entry.name, @@ -759,6 +780,7 @@ impl Database { server_stats_json, entry.retry_data, entry.failure_code.map(|code| code.to_string()), + entry.post_processing, ], )?; @@ -823,7 +845,8 @@ impl Database { let mut stmt = self.conn.prepare( "SELECT id, name, category, status, total_bytes, downloaded_bytes, added_at, completed_at, download_time_secs, output_dir, stages, error_message, server_stats, - CASE WHEN nzb_data IS NOT NULL THEN 1 ELSE 0 END as has_nzb, failure_code + CASE WHEN nzb_data IS NOT NULL THEN 1 ELSE 0 END as has_nzb, failure_code, + post_processing FROM history ORDER BY completed_at DESC LIMIT ?1", )?; @@ -851,6 +874,7 @@ impl Database { stages, error_message: row.get(11)?, failure_code: parse_failure_code(row.get(14)?)?, + post_processing: row.get(15)?, server_stats, // Don't load actual blob in list - just note if it exists nzb_data: if has_nzb != 0 { Some(Vec::new()) } else { None }, @@ -914,7 +938,7 @@ impl Database { let mut stmt = self.conn.prepare( "SELECT id, name, category, status, total_bytes, downloaded_bytes, added_at, completed_at, download_time_secs, output_dir, stages, error_message, server_stats, - failure_code + failure_code, post_processing FROM history WHERE id = ?1", )?; @@ -939,6 +963,7 @@ impl Database { stages, error_message: row.get(11)?, failure_code: parse_failure_code(row.get(13)?)?, + post_processing: row.get(14)?, server_stats, nzb_data: None, retry_data: None, @@ -1407,6 +1432,7 @@ mod tests { }], error_message: None, failure_code: None, + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, @@ -1530,6 +1556,31 @@ mod tests { assert!(db.queue_list().unwrap().is_empty()); } + #[test] + fn history_round_trips_post_processing_level() { + let db = Database::open_memory().unwrap(); + let mut stored = make_history("pp-level", "PP Level"); + stored.post_processing = Some(1); + db.history_insert(&stored).unwrap(); + db.history_insert(&make_history("pp-legacy", "Legacy")) + .unwrap(); + + assert_eq!( + db.history_get("pp-level").unwrap().unwrap().post_processing, + Some(1) + ); + assert_eq!( + db.history_get("pp-legacy") + .unwrap() + .unwrap() + .post_processing, + None + ); + let listed = db.history_list(10).unwrap(); + let level = listed.iter().find(|entry| entry.id == "pp-level").unwrap(); + assert_eq!(level.post_processing, Some(1)); + } + #[test] fn typed_failure_migration_backfills_only_failed_history() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/nzb-core/src/models.rs b/crates/nzb-core/src/models.rs index c5661394..b537e8da 100644 --- a/crates/nzb-core/src/models.rs +++ b/crates/nzb-core/src/models.rs @@ -249,6 +249,11 @@ pub struct HistoryEntry { pub stages: Vec, pub error_message: Option, pub failure_code: Option, + /// Effective post-processing level the job ran with (0=none, 1=repair, + /// 2=unpack, 3=repair+unpack). `None` for rows recorded before it was + /// stored. + #[serde(default)] + pub post_processing: Option, /// Per-server download statistics #[serde(default)] pub server_stats: Vec, diff --git a/crates/nzb-postproc/src/lib.rs b/crates/nzb-postproc/src/lib.rs index daba0790..841c2d3c 100644 --- a/crates/nzb-postproc/src/lib.rs +++ b/crates/nzb-postproc/src/lib.rs @@ -23,7 +23,7 @@ pub use detect::{ pub use par2::recovery_can_cover; pub use pipeline::{ PostProcConfig, PostProcResult, run_pipeline, run_pipeline_with_cleanup, - run_pipeline_with_resources, + run_pipeline_with_resources, run_repair_pipeline, }; pub use resources::{PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot}; pub use unpack::{extract_tar, find_unrar, normalize_extracted_permissions}; diff --git a/crates/nzb-postproc/src/pipeline.rs b/crates/nzb-postproc/src/pipeline.rs index 225bc21e..e9257162 100644 --- a/crates/nzb-postproc/src/pipeline.rs +++ b/crates/nzb-postproc/src/pipeline.rs @@ -189,6 +189,38 @@ pub async fn run_pipeline_with_cleanup( resources: Option<&Arc>, cleanup_patterns: &[String], unwanted_extensions: &[String], +) -> PostProcResult { + run_stages( + job_dir, + config, + resources, + cleanup_patterns, + unwanted_extensions, + true, + ) + .await +} + +/// Run only PAR2 verification and repair, without extraction or cleanup. +/// +/// This is SABnzbd's "repair only" post-processing level (`pp=1`): the +/// archives themselves are the job's output, so they are neither unpacked +/// nor removed. +pub async fn run_repair_pipeline( + job_dir: &Path, + config: &PostProcConfig, + resources: Option<&Arc>, +) -> PostProcResult { + run_stages(job_dir, config, resources, &[], &[], false).await +} + +async fn run_stages( + job_dir: &Path, + config: &PostProcConfig, + resources: Option<&Arc>, + cleanup_patterns: &[String], + unwanted_extensions: &[String], + unpack: bool, ) -> PostProcResult { let mut stages: Vec = Vec::new(); let mut pipeline_ok = true; @@ -451,7 +483,7 @@ pub async fn run_pipeline_with_cleanup( // Extraction is safe only after verification/repair succeeded (or source // content was already known-good). Confirmed unrepaired content damage // must not be mistaken for a successful archive. - let should_extract = pipeline_ok; + let should_extract = unpack && pipeline_ok; let mut extracted_archives = Vec::new(); if should_extract { let output_dir = config.output_dir.as_deref().unwrap_or(job_dir); @@ -502,7 +534,7 @@ pub async fn run_pipeline_with_cleanup( // ------------------------------------------------------------------ // Stage 4: Cleanup // ------------------------------------------------------------------ - if pipeline_ok && config.cleanup_after_extract { + if unpack && pipeline_ok && config.cleanup_after_extract { let cleanup_root = config.output_dir.as_deref().unwrap_or(job_dir); let result = run_cleanup_stage_with_rules( job_dir, @@ -1117,6 +1149,32 @@ mod tests { assert!(!output.path().join("payload.txt").exists()); } + #[tokio::test] + async fn repair_pipeline_neither_extracts_nor_cleans_up() { + let source = tempfile::tempdir().unwrap(); + let output = tempfile::tempdir().unwrap(); + let archive = source.path().join("release.zip"); + write_zip(&archive, &[("payload.txt", b"payload")]); + + let config = PostProcConfig { + output_dir: Some(output.path().to_path_buf()), + ..Default::default() + }; + let result = run_repair_pipeline(source.path(), &config, None).await; + + assert!(result.success, "{result:?}"); + assert!( + result + .stages + .iter() + .all(|stage| stage.name != "Extract" && stage.name != "Cleanup"), + "{:?}", + result.stages + ); + assert!(archive.exists()); + assert!(!output.path().join("payload.txt").exists()); + } + #[tokio::test] async fn recursive_cleanup_removes_nested_archives_from_output_directory() { let source = tempfile::tempdir().unwrap(); diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index 00e69747..7a2f2fbd 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -23,7 +23,7 @@ use crate::nzb_core::models::*; use crate::nzb_core::nzb_parser; use nzb_postproc::{ PostProcConfig, PostProcLimits, PostProcResourcePool, PostProcResourceSnapshot, - has_usable_output, parse_rar_volume, run_pipeline_with_cleanup, + has_usable_output, parse_rar_volume, run_pipeline_with_cleanup, run_repair_pipeline, }; use crate::direct_unpack::DirectUnpacker; @@ -1283,11 +1283,17 @@ impl QueueManager { /// Post-processing level for a job: its own override (e.g. SABnzbd's /// `pp`), else its category's setting, else repair+unpack (3). pub fn post_processing_level(&self, job: &NzbJob) -> u8 { - job.pp_override.unwrap_or_else(|| { + self.post_processing_level_for(job.pp_override, &job.category) + } + + /// [`Self::post_processing_level`] from the job's fields, for callers + /// that hold a borrow of another part of the job. + fn post_processing_level_for(&self, pp_override: Option, category: &str) -> u8 { + pp_override.unwrap_or_else(|| { self.categories .lock() .iter() - .find(|c| c.name == job.category) + .find(|c| c.name == category) .map(|c| c.post_processing) .unwrap_or(3) // default: repair+unpack }) @@ -2155,6 +2161,12 @@ impl QueueManager { // unpacker so extraction overlaps with download. if self.direct_unpack_enabled.load(Ordering::Relaxed) && state.job.articles_failed == 0 + // Download-only and repair-only jobs + // deliver their archives unpacked. + && self.post_processing_level_for( + state.job.pp_override, + &state.job.category, + ) >= 2 && let Some(vol_info) = parse_rar_volume(&file.filename) { if state.direct_unpacker.is_none() { @@ -2644,14 +2656,19 @@ impl QueueManager { max_nested_archive_depth: self.max_nested_archive_depth, }; - let result = run_pipeline_with_cleanup( - &work_dir, - &config, - Some(&self.postproc_resources), - &cleanup_patterns, - &unwanted_extensions, - ) - .await; + let result = if pp_level >= 2 { + run_pipeline_with_cleanup( + &work_dir, + &config, + Some(&self.postproc_resources), + &cleanup_patterns, + &unwanted_extensions, + ) + .await + } else { + // Repair only (SABnzbd `pp=1`): the archives are the output. + run_repair_pipeline(&work_dir, &config, Some(&self.postproc_resources)).await + }; info!( job_id = %job_id, @@ -2676,18 +2693,22 @@ impl QueueManager { result.stages } else { - info!(job_id = %job_id, pp_level, "Post-processing disabled for category, skipping pipeline"); - // No pipeline to repair — if articles failed, mark as failed now - if !success { - let mut jobs = self.jobs.lock(); - if let Some(state) = jobs.get_mut(job_id) { - state.job.status = JobStatus::Failed; - state.job.error_message = - Some(format!("{articles_failed} article(s) failed to download")); - state.failure_code = Some(JobFailureCode::ArticlesUnavailable); - } + info!(job_id = %job_id, pp_level, "Post-processing disabled for job, skipping pipeline"); + // Download only (SABnzbd `pp=0`): nothing is verified or + // repaired, so the raw files are delivered as downloaded, as + // SABnzbd does. Hopeless jobs never reach this point. + if success { + Vec::new() + } else { + vec![StageResult { + name: "Verify".to_string(), + status: StageStatus::Skipped, + message: Some(format!( + "Post-processing disabled; {articles_failed} article(s) failed to download" + )), + duration_secs: 0.0, + }] } - Vec::new() }; // Run the configured hook after the final status is known. Its output @@ -2877,7 +2898,10 @@ impl QueueManager { // A post-processing job that contains only raw archive/PAR2 artifacts // is not a usable completion. Check before moving residual work files // so a bad job cannot pollute the completed directory. - if final_status == JobStatus::Completed && !stages.is_empty() { + // Only unpacking jobs promise extracted output: download-only (0) and + // repair-only (1) jobs deliver the archives themselves. + let pp_level = self.post_processing_level(&state.job); + if final_status == JobStatus::Completed && pp_level >= 2 && !stages.is_empty() { let output_has_payload = has_usable_output(&state.job.output_dir).unwrap_or(false); let work_has_payload = has_usable_output(&state.job.work_dir).unwrap_or(false); if !output_has_payload && !work_has_payload { @@ -2984,6 +3008,7 @@ impl QueueManager { error_message: state.job.error_message.clone(), failure_code: (final_status == JobStatus::Failed) .then(|| state.failure_code.unwrap_or(JobFailureCode::DownloadFailed)), + post_processing: Some(pp_level), server_stats: state.job.server_stats.clone(), nzb_data: state.nzb_data.clone(), retry_data, @@ -3461,6 +3486,7 @@ impl QueueManager { failure_code: Some( state.failure_code.unwrap_or(JobFailureCode::DownloadFailed), ), + post_processing: Some(self.post_processing_level(&state.job)), server_stats: state.job.server_stats.clone(), nzb_data: state.nzb_data.clone(), retry_data: None, @@ -5104,6 +5130,7 @@ mod global_pause_tests { stages: Vec::new(), error_message: None, failure_code: None, + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, @@ -5542,6 +5569,187 @@ mod global_pause_tests { ); } + fn set_default_pp(manager: &QueueManager, level: u8) { + manager.set_categories(vec![CategoryConfig { + post_processing: level, + ..CategoryConfig::default() + }]); + } + + fn write_raw_artifacts(work_dir: &std::path::Path) { + std::fs::create_dir_all(work_dir).unwrap(); + std::fs::write(work_dir.join("release.part001.rar"), b"raw").unwrap(); + std::fs::write(work_dir.join("release.part002.rar"), b"raw").unwrap(); + std::fs::write(work_dir.join("release.par2"), b"par2").unwrap(); + } + + /// SABnzbd `pp=0` is download only: the raw archives are the output, so + /// the "no usable output" rule must not fail the job (even when a + /// post-processing script contributed a stage). + #[tokio::test] + async fn download_only_job_with_raw_archives_is_completed() { + let (manager, tempdir) = manager(); + set_default_pp(&manager, 0); + let raw = job("download-only", JobStatus::PostProcessing, tempdir.path()); + write_raw_artifacts(&raw.work_dir); + let output = raw.output_dir.clone(); + insert_job(&manager, raw); + + let stages = vec![StageResult { + name: "Script".to_string(), + status: StageStatus::Success, + message: Some("ok".to_string()), + duration_secs: 0.0, + }]; + let mut jobs = manager.jobs.lock(); + manager.move_to_history(jobs.get_mut("download-only").unwrap(), stages); + drop(jobs); + + let entry = manager + .db + .lock() + .history_get("download-only") + .unwrap() + .unwrap(); + assert_eq!( + entry.status, + JobStatus::Completed, + "{:?}", + entry.error_message + ); + assert_eq!(entry.post_processing, Some(0)); + assert!(output.join("release.part001.rar").exists()); + assert!(output.join("release.par2").exists()); + } + + /// SABnzbd `pp=1` is repair only: archives are the expected output. + #[tokio::test] + async fn repair_only_job_with_raw_archives_is_completed() { + let (manager, tempdir) = manager(); + set_default_pp(&manager, 1); + let raw = job("repair-only", JobStatus::PostProcessing, tempdir.path()); + write_raw_artifacts(&raw.work_dir); + let output = raw.output_dir.clone(); + insert_job(&manager, raw); + + let stages = vec![StageResult { + name: "Verify".to_string(), + status: StageStatus::Skipped, + message: Some("Skipped".to_string()), + duration_secs: 0.0, + }]; + let mut jobs = manager.jobs.lock(); + manager.move_to_history(jobs.get_mut("repair-only").unwrap(), stages); + drop(jobs); + + let entry = manager + .db + .lock() + .history_get("repair-only") + .unwrap() + .unwrap(); + assert_eq!( + entry.status, + JobStatus::Completed, + "{:?}", + entry.error_message + ); + assert_eq!(entry.post_processing, Some(1)); + assert!(output.join("release.part002.rar").exists()); + } + + /// The per-job override decides the level recorded in history. + #[tokio::test] + async fn history_records_the_job_override_level() { + let (manager, tempdir) = manager(); + set_default_pp(&manager, 3); + let mut raw = job("override-level", JobStatus::PostProcessing, tempdir.path()); + raw.pp_override = Some(1); + write_raw_artifacts(&raw.work_dir); + insert_job(&manager, raw); + + let mut jobs = manager.jobs.lock(); + manager.move_to_history(jobs.get_mut("override-level").unwrap(), Vec::new()); + drop(jobs); + + let entry = manager + .db + .lock() + .history_get("override-level") + .unwrap() + .unwrap(); + assert_eq!(entry.post_processing, Some(1)); + assert_eq!(entry.status, JobStatus::Completed); + } + + /// `pp=1` must not unpack: a (bogus) archive is delivered as-is instead + /// of failing extraction. + #[tokio::test] + async fn repair_only_pipeline_does_not_extract() { + let (manager, tempdir) = manager(); + set_default_pp(&manager, 1); + let raw = job( + "repair-no-unpack", + JobStatus::PostProcessing, + tempdir.path(), + ); + std::fs::create_dir_all(&raw.work_dir).unwrap(); + std::fs::write(raw.work_dir.join("release.zip"), b"not really a zip").unwrap(); + let output = raw.output_dir.clone(); + insert_job(&manager, raw); + + manager.on_job_finished("repair-no-unpack", true, 0).await; + + let entry = manager + .db + .lock() + .history_get("repair-no-unpack") + .unwrap() + .unwrap(); + assert_eq!( + entry.status, + JobStatus::Completed, + "{:?}", + entry.error_message + ); + assert!(entry.stages.iter().all(|stage| stage.name != "Extract")); + assert!(output.join("release.zip").exists()); + } + + /// `pp=0` does not verify or repair, so missing articles do not fail + /// the job; the raw files are delivered like SABnzbd does. + #[tokio::test] + async fn download_only_job_with_failed_articles_is_completed() { + let (manager, tempdir) = manager(); + set_default_pp(&manager, 0); + let raw = job( + "download-only-damaged", + JobStatus::PostProcessing, + tempdir.path(), + ); + write_raw_artifacts(&raw.work_dir); + let output = raw.output_dir.clone(); + insert_job(&manager, raw); + + manager + .on_job_finished("download-only-damaged", false, 2) + .await; + + let entry = manager + .db + .lock() + .history_get("download-only-damaged") + .unwrap() + .unwrap(); + assert_eq!( + entry.status, + JobStatus::Completed, + "{:?}", + entry.error_message + ); + assert!(output.join("release.part001.rar").exists()); + } + #[tokio::test] async fn post_processing_with_payload_is_completed() { let (manager, tempdir) = manager(); diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index 13b2562e..40606d65 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -1588,6 +1588,10 @@ fn handle_history(state: &AppState, req: &SabApiRequest) -> Json Json serde_json::Value { let mut slots: Vec = postprocessing .iter() - .map(SabHistorySlot::from_postprocessing) + .map(|(job, level)| SabHistorySlot::from_postprocessing(job, *level)) .chain(entries.iter().map(SabHistorySlot::from_entry)) .filter(|slot| history_slot_matches(slot, req)) .collect(); @@ -2510,6 +2514,18 @@ fn sab_queue_status(status: JobStatus) -> &'static str { } } +/// SABnzbd's history `pp` label for an effective post-processing level +/// (`sabnzbd/constants.py::PP_LOOKUP`); unknown levels map to `"X"` as there. +fn sab_pp_label(level: u8) -> &'static str { + match level { + 0 => "", + 1 => "R", + 2 => "U", + 3 => "D", + _ => "X", + } +} + #[derive(Serialize)] struct SabHistorySlot { completed: i64, @@ -2572,7 +2588,8 @@ impl SabHistorySlot { name: entry.name.clone(), nzb_name: format!("{}.nzb", entry.name), category: sab_category_label(&entry.category), - pp: "D".into(), + // Rows written before the level was recorded keep reporting "D". + pp: entry.post_processing.map_or("D", sab_pp_label).into(), script: String::new(), report: String::new(), url: String::new(), @@ -2619,7 +2636,7 @@ impl SabHistorySlot { } } - fn from_postprocessing(job: &NzbJob) -> Self { + fn from_postprocessing(job: &NzbJob, pp_level: u8) -> Self { let path = job.work_dir.to_string_lossy().to_string(); Self { completed: job @@ -2629,7 +2646,7 @@ impl SabHistorySlot { name: job.name.clone(), nzb_name: format!("{}.nzb", job.name), category: sab_category_label(&job.category), - pp: "D".into(), + pp: sab_pp_label(pp_level).into(), script: String::new(), report: String::new(), url: String::new(), @@ -2867,6 +2884,7 @@ mod tests { }], error_message: (status == JobStatus::Failed).then(|| "broken archive".into()), failure_code: (status == JobStatus::Failed).then_some(JobFailureCode::ArchiveInvalid), + post_processing: None, server_stats: Vec::new(), nzb_data: (status == JobStatus::Failed).then(Vec::new), retry_data: None, @@ -3115,6 +3133,7 @@ mod tests { stages: Vec::new(), error_message: None, failure_code: None, + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, @@ -3189,6 +3208,32 @@ mod tests { assert!(slots[0]["completeness"].is_null()); } + /// SABnzbd reports the job's post-processing level as + /// `PP_LOOKUP = {0: "", 1: "R", 2: "U", 3: "D"}`. + #[test] + fn history_slot_pp_reflects_the_effective_level() { + for (level, label) in [(0, ""), (1, "R"), (2, "U"), (3, "D")] { + let mut entry = history_entry("pp", "PP", "movies", JobStatus::Completed, 1); + entry.post_processing = Some(level); + assert_eq!( + SabHistorySlot::from_entry(&entry).pp, + label, + "level {level}" + ); + } + // Rows recorded before the level was stored keep the old answer. + let legacy = history_entry("legacy", "Legacy", "movies", JobStatus::Completed, 1); + assert_eq!(SabHistorySlot::from_entry(&legacy).pp, "D"); + + let response = build_history_response( + &[], + &[(postprocessing_job(), 1)], + &SabApiRequest::default(), + 1, + ); + assert_eq!(response["history"]["slots"][0]["pp"], "R"); + } + #[test] fn history_includes_postprocessing_before_terminal_slots() { let response = build_history_response( @@ -3199,7 +3244,7 @@ mod tests { JobStatus::Completed, 1, )], - &[postprocessing_job()], + &[(postprocessing_job(), 3)], &SabApiRequest::default(), 4, ); @@ -3364,6 +3409,7 @@ mod tests { stages: Vec::new(), error_message: None, failure_code: None, + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, @@ -4456,6 +4502,7 @@ mod tests { stages: Vec::new(), error_message: None, failure_code: None, + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, diff --git a/crates/nzb-web/tests/harness_incomplete_cleanup.rs b/crates/nzb-web/tests/harness_incomplete_cleanup.rs index f5b92f61..99cd6caa 100644 --- a/crates/nzb-web/tests/harness_incomplete_cleanup.rs +++ b/crates/nzb-web/tests/harness_incomplete_cleanup.rs @@ -144,6 +144,7 @@ fn history_entry(id: &str, complete: &Path) -> HistoryEntry { stages: Vec::new(), error_message: Some("failed".to_string()), failure_code: Some(JobFailureCode::DownloadFailed), + post_processing: None, server_stats: Vec::new(), nzb_data: None, retry_data: None, From a54bb8dc6f80e1e7fe83e2a8c7253931bad6eb7d Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:30:35 -0500 Subject: [PATCH 02/33] fix(web): honour sort field and direction in queue sort POST /api/queue/sort always sorted by remaining percentage and silently ignored any other request: its body had only an `ascending` flag. SAB mode=queue&name=sort had the same limitation and ignored SABnzbd's `sort` and `dir` parameters. - QueueManager::sort_queue(QueueSortField, ascending) sorts by remaining, name (case-insensitive), size, age or priority. sort_by_remaining_percentage now delegates to it. Like the existing sort, it is stable and reorders job_order only, so job status is untouched and active downloads keep running. - The native body accepts optional `sort_by` (default `remaining`) and `direction` (`asc`/`desc`), which takes precedence over the legacy `ascending` flag. An unknown value is a 400 and leaves the queue as it was. - SAB name=sort reads `sort` (avg_age, name, size/bytes, remaining) and `dir`, as SABnzbd's _api_queue_sort does; only `dir=desc` reverses. An unknown field returns status false. Without `sort`, the earlier form (remaining, direction in `value`) still works. Age ascending is youngest first, matching SABnzbd's avg_age. RustNZB has no average post date, so it uses the job's added time. Fixes #99 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 1951094d316227263400041637e13cd48831355e) --- apps/rustnzb/src/handlers.rs | 28 ++- apps/rustnzb/tests/queue_sort_api.rs | 269 +++++++++++++++++++++++++++ crates/nzb-web/src/lib.rs | 3 +- crates/nzb-web/src/queue_manager.rs | 174 ++++++++++++++++- crates/nzb-web/src/sabnzbd_compat.rs | 51 +++-- 5 files changed, 498 insertions(+), 27 deletions(-) create mode 100644 apps/rustnzb/tests/queue_sort_api.rs diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 37f5c888..cf462274 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -96,7 +96,14 @@ pub struct MoveJobBody { #[derive(Deserialize)] pub struct SortQueueBody { - /// Sort in ascending remaining percentage order when true. + /// Field to sort by: `remaining` (default), `name`, `size`, `age` or + /// `priority`. + #[serde(default)] + pub sort_by: Option, + /// `asc` or `desc`. Takes precedence over `ascending` when present. + #[serde(default)] + pub direction: Option, + /// Legacy direction flag, used when `direction` is absent. #[serde(default = "default_sort_ascending")] pub ascending: bool, } @@ -426,14 +433,25 @@ pub async fn h_queue_set_priority( Ok(Json(SimpleResponse { status: true })) } -/// POST /api/queue/sort -- Stable sort by remaining work percentage. +/// POST /api/queue/sort -- Stable sort by `sort_by` (default: remaining +/// work percentage) in `direction` (or the legacy `ascending` flag). pub async fn h_queue_sort( State(state): State>, Json(body): Json, ) -> Result, ApiError> { - state - .queue_manager - .sort_by_remaining_percentage(body.ascending); + let field = match body.sort_by.as_deref() { + None => nzb_web::QueueSortField::Remaining, + Some(value) => value.parse().map_err(|()| { + ApiError::bad_request("sort_by must be one of: remaining, name, size, age, priority") + })?, + }; + let ascending = match body.direction.as_deref() { + None => body.ascending, + Some(value) if value.eq_ignore_ascii_case("asc") => true, + Some(value) if value.eq_ignore_ascii_case("desc") => false, + Some(_) => return Err(ApiError::bad_request("direction must be asc or desc")), + }; + state.queue_manager.sort_queue(field, ascending); Ok(Json(SimpleResponse { status: true })) } diff --git a/apps/rustnzb/tests/queue_sort_api.rs b/apps/rustnzb/tests/queue_sort_api.rs new file mode 100644 index 00000000..c1166ebb --- /dev/null +++ b/apps/rustnzb/tests/queue_sort_api.rs @@ -0,0 +1,269 @@ +//! BUG-62: `POST /api/queue/sort` and SABnzbd `mode=queue&name=sort` honour +//! the requested sort field and direction instead of always sorting by +//! remaining percentage. + +use std::sync::Arc; + +use arc_swap::ArcSwap; +use chrono::{Duration, Utc}; +use nzb_web::auth::{CredentialStore, StoredCredentials, TokenStore}; +use nzb_web::nzb_core::config::AppConfig; +use nzb_web::nzb_core::db::Database; +use nzb_web::nzb_core::models::{JobStatus, NzbJob, Priority}; +use nzb_web::{AppState, LogBuffer, QueueManager}; +use rustnzb::server::build_router; + +const API_KEY: &str = "sort-test-key"; + +struct App { + base: String, + state: Arc, + _temp: tempfile::TempDir, + handle: tokio::task::JoinHandle<()>, +} + +impl Drop for App { + fn drop(&mut self) { + self.handle.abort(); + } +} + +async fn start_app() -> App { + let temp = tempfile::tempdir().unwrap(); + let mut config = AppConfig::default(); + config.general.api_key = Some(API_KEY.into()); + let logs = LogBuffer::new(); + let queue = QueueManager::new( + Vec::new(), + Database::open_memory().unwrap(), + temp.path().join("incomplete"), + temp.path().join("complete"), + logs.clone(), + config.general.max_active_downloads, + config.categories.clone(), + config.general.min_free_space_bytes, + config.general.speed_limit_bps, + false, + config.general.max_nested_archive_depth, + config.general.abort_hopeless, + config.general.early_failure_check, + config.general.required_completion_pct, + config.general.article_timeout_secs, + ); + // Keep every job queued: the test only exercises ordering. + queue.pause_all(); + let credentials = Arc::new(CredentialStore::new(temp.path().to_path_buf())); + credentials + .set_credentials(StoredCredentials { + username: "admin".into(), + password: "password".into(), + }) + .unwrap(); + let state = Arc::new(AppState::new( + Arc::new(ArcSwap::from_pointee(config)), + temp.path().join("config.toml"), + queue, + logs, + Arc::new(TokenStore::new()), + credentials, + )); + let router = build_router(state.clone()); + #[cfg(feature = "webdav")] + let router = router.layer(axum::Extension(None::>)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let handle = tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + }); + + let root = temp.path().to_path_buf(); + let now = Utc::now(); + // (id, name, total, downloaded, priority, minutes old) + for (id, name, total, downloaded, priority, age) in [ + ("job-a", "charlie", 300u64, 0u64, Priority::Normal, 10i64), + ("job-b", "Alpha", 100, 90, Priority::High, 30), + ("job-c", "bravo", 200, 20, Priority::Normal, 20), + ] { + let job = NzbJob { + id: id.into(), + name: name.into(), + category: "Default".into(), + status: JobStatus::Queued, + priority, + total_bytes: total, + downloaded_bytes: downloaded, + file_count: 0, + files_completed: 0, + article_count: 0, + articles_downloaded: 0, + articles_failed: 0, + added_at: now - Duration::minutes(age), + completed_at: None, + work_dir: root.join("incomplete").join(id), + output_dir: root.join("complete").join(id), + password: None, + error_message: None, + speed_bps: 0, + server_stats: Vec::new(), + pp_override: None, + files: Vec::new(), + }; + state.queue_manager.add_job(job, None).unwrap(); + } + + App { + base, + state, + _temp: temp, + handle, + } +} + +async fn login(app: &App, client: &reqwest::Client) -> String { + let tokens = client + .post(format!("{}/api/auth/login", app.base)) + .json(&serde_json::json!({"username": "admin", "password": "password"})) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + tokens["access_token"].as_str().unwrap().to_string() +} + +fn order(app: &App) -> Vec { + app.state + .queue_manager + .get_jobs() + .into_iter() + .map(|job| job.id) + .collect() +} + +async fn sort( + app: &App, + client: &reqwest::Client, + access: &str, + body: serde_json::Value, +) -> (u16, serde_json::Value) { + let response = client + .post(format!("{}/api/queue/sort", app.base)) + .bearer_auth(access) + .json(&body) + .send() + .await + .unwrap(); + let status = response.status().as_u16(); + let text = response.text().await.unwrap(); + ( + status, + serde_json::from_str(&text).unwrap_or(serde_json::Value::Null), + ) +} + +#[tokio::test] +async fn native_sort_honours_sort_by_and_direction() { + let app = start_app().await; + let client = reqwest::Client::new(); + let access = login(&app, &client).await; + + for (body, expected) in [ + ( + serde_json::json!({"sort_by": "name", "direction": "asc"}), + ["job-b", "job-c", "job-a"], + ), + ( + serde_json::json!({"sort_by": "name", "direction": "desc"}), + ["job-a", "job-c", "job-b"], + ), + ( + serde_json::json!({"sort_by": "size"}), + ["job-b", "job-c", "job-a"], + ), + ( + serde_json::json!({"sort_by": "size", "ascending": false}), + ["job-a", "job-c", "job-b"], + ), + // `direction` wins over `ascending` when both are sent. + ( + serde_json::json!({"sort_by": "size", "ascending": false, "direction": "asc"}), + ["job-b", "job-c", "job-a"], + ), + ( + serde_json::json!({"sort_by": "age", "direction": "asc"}), + ["job-a", "job-c", "job-b"], + ), + ( + serde_json::json!({"sort_by": "priority", "direction": "desc"}), + ["job-b", "job-a", "job-c"], + ), + // Legacy body: remaining percentage, ascending by default. + (serde_json::json!({}), ["job-b", "job-c", "job-a"]), + ( + serde_json::json!({"ascending": false}), + ["job-a", "job-c", "job-b"], + ), + ] { + let (status, response) = sort(&app, &client, &access, body.clone()).await; + assert_eq!(status, 200, "{body}: {response}"); + assert_eq!(response["status"], true, "{body}"); + assert_eq!(order(&app), expected, "{body}"); + } +} + +#[tokio::test] +async fn native_sort_rejects_unknown_field_or_direction() { + let app = start_app().await; + let client = reqwest::Client::new(); + let access = login(&app, &client).await; + let before = order(&app); + + for body in [ + serde_json::json!({"sort_by": "colour"}), + serde_json::json!({"sort_by": "name", "direction": "sideways"}), + ] { + let (status, response) = sort(&app, &client, &access, body.clone()).await; + assert_eq!(status, 400, "{body}: {response}"); + assert_eq!(response["error_kind"], "bad_request", "{body}"); + assert_eq!(order(&app), before, "{body} must not reorder the queue"); + } +} + +async fn sab(app: &App, client: &reqwest::Client, query: &str) -> serde_json::Value { + client + .get(format!( + "{}/api?mode=queue&name=sort&output=json&apikey={API_KEY}&{query}", + app.base + )) + .send() + .await + .unwrap() + .json() + .await + .unwrap() +} + +#[tokio::test] +async fn sab_queue_sort_honours_sort_and_dir() { + let app = start_app().await; + let client = reqwest::Client::new(); + + for (query, expected) in [ + ("sort=name&dir=asc", ["job-b", "job-c", "job-a"]), + ("sort=name&dir=desc", ["job-a", "job-c", "job-b"]), + ("sort=size&dir=desc", ["job-a", "job-c", "job-b"]), + ("sort=avg_age&dir=asc", ["job-a", "job-c", "job-b"]), + ("sort=remaining&dir=asc", ["job-b", "job-c", "job-a"]), + ("sort=remaining&dir=desc", ["job-a", "job-c", "job-b"]), + ] { + let response = sab(&app, &client, query).await; + assert_eq!(response["status"], true, "{query}: {response}"); + assert_eq!(order(&app), expected, "{query}"); + } + + let before = order(&app); + let response = sab(&app, &client, "sort=colour&dir=asc").await; + assert_eq!(response["status"], false, "{response}"); + assert_eq!(order(&app), before); +} diff --git a/crates/nzb-web/src/lib.rs b/crates/nzb-web/src/lib.rs index e5625d45..9b9241a9 100644 --- a/crates/nzb-web/src/lib.rs +++ b/crates/nzb-web/src/lib.rs @@ -21,7 +21,8 @@ pub mod util; pub use log_buffer::{LogBuffer, LogBufferLayer}; pub use nzb_dispatch::{ArticleFailure, ArticleFailureKind}; pub use queue_manager::{ - DailyStatisticsData, GlobalStatisticsData, QueueManager, ServerStatsData, StatisticsPeriodData, + DailyStatisticsData, GlobalStatisticsData, QueueManager, QueueSortField, ServerStatsData, + StatisticsPeriodData, }; pub use startup::{StartupConfig, StartupResult}; pub use state::AppState; diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index 7a2f2fbd..c46ea43b 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -1081,6 +1081,65 @@ struct JobState { output_dir_claimed: bool, } +/// Field a manual queue sort orders by. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum QueueSortField { + /// Fraction of the job's bytes still to download. + Remaining, + /// Job name, case-insensitively. + Name, + /// Total job size in bytes. + Size, + /// Time since the job was added; ascending is youngest first, matching + /// SABnzbd's `avg_age`. + Age, + /// Job priority, `Low` first when ascending. + Priority, +} + +impl QueueSortField { + fn compare(self, left: &NzbJob, right: &NzbJob) -> std::cmp::Ordering { + match self { + Self::Remaining => { + let remaining = |job: &NzbJob| { + let total = job.total_bytes.max(1); + (total.saturating_sub(job.downloaded_bytes), total) + }; + let (left_remaining, left_total) = remaining(left); + let (right_remaining, right_total) = remaining(right); + (left_remaining as u128 * right_total as u128) + .cmp(&(right_remaining as u128 * left_total as u128)) + } + Self::Name => left + .name + .chars() + .flat_map(char::to_lowercase) + .cmp(right.name.chars().flat_map(char::to_lowercase)), + Self::Size => left.total_bytes.cmp(&right.total_bytes), + // Younger means added later. + Self::Age => right.added_at.cmp(&left.added_at), + Self::Priority => (left.priority as u8).cmp(&(right.priority as u8)), + } + } +} + +impl std::str::FromStr for QueueSortField { + type Err = (); + + /// Parse a sort field name, case-insensitively. Accepts SABnzbd's + /// `avg_age` and `bytes` spellings alongside the native names. + fn from_str(value: &str) -> Result { + match value.to_ascii_lowercase().as_str() { + "remaining" => Ok(Self::Remaining), + "name" => Ok(Self::Name), + "size" | "bytes" => Ok(Self::Size), + "age" | "avg_age" => Ok(Self::Age), + "priority" => Ok(Self::Priority), + _ => Err(()), + } + } +} + /// Notification fired immediately when a job is accepted into the queue. #[derive(Debug, Clone)] pub struct JobAddedEvent { @@ -1470,19 +1529,24 @@ impl QueueManager { /// queue order is retained for equal percentages, which keeps repeated /// manual sorts deterministic and avoids active-job churn. pub fn sort_by_remaining_percentage(&self, ascending: bool) { + self.sort_queue(QueueSortField::Remaining, ascending); + } + + /// Stable sort of the queue by `field`. Only the queue order changes: + /// job status is untouched, so active downloads keep running, and the + /// original order is retained for equal keys, which keeps repeated + /// manual sorts deterministic and avoids active-job churn. + pub fn sort_queue(&self, field: QueueSortField, ascending: bool) { let jobs = self.jobs.lock(); let mut order = self.job_order.lock(); order.sort_by(|left, right| { - let remaining = |id: &String| { - jobs.get(id).map_or((0u64, 1u64), |state| { - let total = state.job.total_bytes.max(1); - (total.saturating_sub(state.job.downloaded_bytes), total) - }) + let left = jobs.get(left).map(|state| &state.job); + let right = jobs.get(right).map(|state| &state.job); + let ordering = match (left, right) { + (Some(left), Some(right)) => field.compare(left, right), + // Ids without a job state keep their relative position. + _ => std::cmp::Ordering::Equal, }; - let (left_remaining, left_total) = remaining(left); - let (right_remaining, right_total) = remaining(right); - let ordering = (left_remaining as u128 * right_total as u128) - .cmp(&(right_remaining as u128 * left_total as u128)); if ascending { ordering } else { @@ -5071,6 +5135,98 @@ mod global_pause_tests { ); } + fn order_of(manager: &QueueManager) -> Vec { + manager.job_order.lock().clone() + } + + fn sort_fixture(manager: &QueueManager, root: &std::path::Path) { + let base = Utc::now(); + // (id, name, total, downloaded, priority, age in minutes, status) + for (id, name, total, downloaded, priority, age_min, status) in [ + ( + "a", + "charlie", + 300, + 0, + Priority::Normal, + 10, + JobStatus::Downloading, + ), + ("b", "Alpha", 100, 0, Priority::High, 30, JobStatus::Queued), + ("c", "bravo", 200, 0, Priority::Low, 20, JobStatus::Queued), + ] { + let mut j = job(id, status, root); + j.name = name.to_string(); + j.total_bytes = total; + j.downloaded_bytes = downloaded; + j.priority = priority; + j.added_at = base - chrono::Duration::minutes(age_min); + insert_job(manager, j); + } + } + + #[tokio::test] + async fn queue_sort_by_name_is_case_insensitive() { + let (manager, tempdir) = manager(); + sort_fixture(&manager, tempdir.path()); + manager.sort_queue(QueueSortField::Name, true); + assert_eq!(order_of(&manager), vec!["b", "c", "a"]); + manager.sort_queue(QueueSortField::Name, false); + assert_eq!(order_of(&manager), vec!["a", "c", "b"]); + // Sorting reorders only: the active job keeps downloading. + assert_eq!(manager.get_job("a").unwrap().status, JobStatus::Downloading); + } + + #[tokio::test] + async fn queue_sort_by_size_age_and_priority() { + let (manager, tempdir) = manager(); + sort_fixture(&manager, tempdir.path()); + manager.sort_queue(QueueSortField::Size, true); + assert_eq!(order_of(&manager), vec!["b", "c", "a"]); + manager.sort_queue(QueueSortField::Size, false); + assert_eq!(order_of(&manager), vec!["a", "c", "b"]); + // Age ascending is youngest first, like SABnzbd's avg_age. + manager.sort_queue(QueueSortField::Age, true); + assert_eq!(order_of(&manager), vec!["a", "c", "b"]); + manager.sort_queue(QueueSortField::Age, false); + assert_eq!(order_of(&manager), vec!["b", "c", "a"]); + manager.sort_queue(QueueSortField::Priority, true); + assert_eq!(order_of(&manager), vec!["c", "a", "b"]); + manager.sort_queue(QueueSortField::Priority, false); + assert_eq!(order_of(&manager), vec!["b", "a", "c"]); + } + + #[tokio::test] + async fn queue_sort_is_stable_for_equal_keys() { + let (manager, tempdir) = manager(); + for id in ["x", "y", "z"] { + let mut j = job(id, JobStatus::Queued, tempdir.path()); + j.name = "Same".into(); + insert_job(&manager, j); + } + manager.sort_queue(QueueSortField::Name, true); + assert_eq!(order_of(&manager), vec!["x", "y", "z"]); + manager.sort_queue(QueueSortField::Name, false); + assert_eq!(order_of(&manager), vec!["x", "y", "z"]); + } + + #[test] + fn queue_sort_field_parses_known_names_only() { + for (input, expected) in [ + ("remaining", QueueSortField::Remaining), + ("NAME", QueueSortField::Name), + ("size", QueueSortField::Size), + ("bytes", QueueSortField::Size), + ("age", QueueSortField::Age), + ("avg_age", QueueSortField::Age), + ("priority", QueueSortField::Priority), + ] { + assert_eq!(input.parse::(), Ok(expected), "{input}"); + } + assert!("bogus".parse::().is_err()); + assert!("".parse::().is_err()); + } + #[tokio::test] async fn speed_limit_above_u32_saturates_instead_of_wrapping() { let (manager, _tempdir) = manager(); diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index 40606d65..84bc3619 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -55,6 +55,11 @@ pub struct SabApiRequest { /// SABnzbd's `script` parameter is accepted and ignored (unknown query /// fields are not rejected); RustNZB has no per-job scripts. pub pp: Option, + /// Sort field for `mode=queue&name=sort` (`avg_age`, `name`, `size`, + /// `remaining`). + pub sort: Option, + /// Sort direction for `mode=queue&name=sort` (`asc` or `desc`). + pub dir: Option, } /// Validate API key. Returns Err with JSON response on failure. @@ -840,6 +845,8 @@ async fn dispatch_post( del_files: query_req.del_files, nzbname: query_req.nzbname, pp: query_req.pp, + sort: query_req.sort, + dir: query_req.dir, }; Ok(dispatch_mode( state, @@ -1138,13 +1145,42 @@ fn handle_server_stats(state: &AppState) -> Json { // Mode handlers // --------------------------------------------------------------------------- +/// `mode=queue&name=sort&sort=&dir=`, as SABnzbd's +/// `_api_queue_sort`. Only `dir=desc` reverses, like SABnzbd. Without +/// `sort`, the earlier RustNZB form (remaining percentage, direction in +/// `value`) still works. +fn handle_queue_sort(state: &AppState, req: &SabApiRequest) -> Json { + let is_desc = |value: &str| { + value.eq_ignore_ascii_case("desc") || value.eq_ignore_ascii_case("descending") + }; + let field = match req.sort.as_deref().filter(|sort| !sort.is_empty()) { + None => crate::QueueSortField::Remaining, + Some(sort) => match sort.parse() { + Ok(field) => field, + Err(()) => { + return Json(serde_json::json!({ + "status": false, + "error": format!("Unknown sort field: {sort}") + })); + } + }, + }; + let descending = req + .dir + .as_deref() + .or(req.value.as_deref()) + .is_some_and(is_desc); + state.queue_manager.sort_queue(field, !descending); + Json(serde_json::json!({ "status": true })) +} + fn handle_queue(state: &AppState, req: &SabApiRequest) -> Json { let qm = &state.queue_manager; // Sub-commands dispatched via mode=queue&name=, matching SABnzbd's // real `_api_queue_table` (delete, pause, resume, priority, rename, - // purge, change_complete_action). `sort` and `delete_nzf` have no - // equivalent capability in RustNZB's queue manager yet. + // purge, change_complete_action, sort). `delete_nzf` has no equivalent + // capability in RustNZB's queue manager yet. match req.name.as_deref() { Some("delete") => return handle_queue_delete(state, req), Some("pause") => return handle_queue_item_pause(state, req), @@ -1152,16 +1188,7 @@ fn handle_queue(state: &AppState, req: &SabApiRequest) -> Json return handle_queue_priority(state, req), Some("rename") => return handle_queue_rename(state, req), Some("purge") => return handle_queue_purge(state), - Some("sort") => { - let ascending = !matches!( - req.value.as_deref(), - Some(value) - if value.eq_ignore_ascii_case("descending") - || value.eq_ignore_ascii_case("desc") - ); - qm.sort_by_remaining_percentage(ascending); - return Json(serde_json::json!({ "status": true })); - } + Some("sort") => return handle_queue_sort(state, req), Some("change_complete_action") => return Json(serde_json::json!({ "status": true })), _ => {} } From a8ca12fa9587b070643b8ee7a01499afa02dc1ce Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 07:35:22 -0500 Subject: [PATCH 03/33] fix(web): count unknown ids as failed in bulk queue actions POST /api/queue/bulk with action "delete" reported an id that matched no job as succeeded: remove_job is idempotent and returns Ok for an unknown id, and the handler counted every Ok as a success. Deleting [J1, J2, ] returned succeeded 3, failed 0. The handler now checks the id with the new QueueManager::contains_job before deleting, and records a JobNotFound failure for an unknown id. pause, resume, priority and category already return JobNotFound. remove_job and the single-job DELETE stay idempotent. The response gains a `failures` array: one {id, error} entry per id that failed, in request order. `error` has the same shape as an API error body (error_kind, human_readable, status). `failed` is its length. Fixes #101 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 5e6b59fe466d6c602e4ede1fbcb708373640e0ba) --- apps/rustnzb/src/handlers.rs | 27 +++- apps/rustnzb/tests/queue_bulk_api.rs | 217 +++++++++++++++++++++++++++ crates/nzb-web/src/queue_manager.rs | 5 + 3 files changed, 245 insertions(+), 4 deletions(-) create mode 100644 apps/rustnzb/tests/queue_bulk_api.rs diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index cf462274..81a7f753 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -2013,6 +2013,16 @@ pub struct BulkActionResponse { pub status: bool, pub succeeded: usize, pub failed: usize, + /// One entry per id that failed, in request order. + pub failures: Vec, +} + +#[derive(Serialize)] +pub struct BulkActionFailure { + pub id: String, + /// Same shape as an API error body (`error_kind`, `human_readable`, + /// `status`). + pub error: ApiError, } /// POST /api/queue/bulk -- Perform an action on multiple jobs. @@ -2022,12 +2032,17 @@ pub async fn h_queue_bulk_action( ) -> Result, ApiError> { let qm = &state.queue_manager; let mut succeeded = 0usize; - let mut failed = 0usize; + let mut failures = Vec::new(); for id in &body.ids { let result = match body.action.as_str() { "pause" => qm.pause_job(id), "resume" => qm.resume_job(id), + // `remove_job` is idempotent and returns Ok for an unknown id; + // a bulk request must report that id as failed. + "delete" if !qm.contains_job(id) => { + Err(nzb_web::nzb_core::NzbError::JobNotFound(id.clone())) + } "delete" => qm.remove_job(id), "priority" => { let p = body.value.as_ref().and_then(|v| v.as_i64()).unwrap_or(1) as i32; @@ -2044,14 +2059,18 @@ pub async fn h_queue_bulk_action( }; match result { Ok(_) => succeeded += 1, - Err(_) => failed += 1, + Err(error) => failures.push(BulkActionFailure { + id: id.clone(), + error: ApiError::from(error), + }), } } Ok(Json(BulkActionResponse { - status: failed == 0, + status: failures.is_empty(), succeeded, - failed, + failed: failures.len(), + failures, })) } diff --git a/apps/rustnzb/tests/queue_bulk_api.rs b/apps/rustnzb/tests/queue_bulk_api.rs new file mode 100644 index 00000000..694a8613 --- /dev/null +++ b/apps/rustnzb/tests/queue_bulk_api.rs @@ -0,0 +1,217 @@ +//! BUG-63: `POST /api/queue/bulk` counts an id that matches no job as a +//! failure, and reports which ids failed and why. + +use std::sync::Arc; + +use arc_swap::ArcSwap; +use chrono::Utc; +use nzb_web::auth::{CredentialStore, StoredCredentials, TokenStore}; +use nzb_web::nzb_core::config::AppConfig; +use nzb_web::nzb_core::db::Database; +use nzb_web::nzb_core::models::{JobStatus, NzbJob, Priority}; +use nzb_web::{AppState, LogBuffer, QueueManager}; +use rustnzb::server::build_router; + +struct App { + base: String, + state: Arc, + _temp: tempfile::TempDir, + handle: tokio::task::JoinHandle<()>, +} + +impl Drop for App { + fn drop(&mut self) { + self.handle.abort(); + } +} + +async fn start_app() -> App { + let temp = tempfile::tempdir().unwrap(); + let config = AppConfig::default(); + let logs = LogBuffer::new(); + let queue = QueueManager::new( + Vec::new(), + Database::open_memory().unwrap(), + temp.path().join("incomplete"), + temp.path().join("complete"), + logs.clone(), + config.general.max_active_downloads, + config.categories.clone(), + config.general.min_free_space_bytes, + config.general.speed_limit_bps, + false, + config.general.max_nested_archive_depth, + config.general.abort_hopeless, + config.general.early_failure_check, + config.general.required_completion_pct, + config.general.article_timeout_secs, + ); + // Keep every job queued: no servers are configured. + queue.pause_all(); + let credentials = Arc::new(CredentialStore::new(temp.path().to_path_buf())); + credentials + .set_credentials(StoredCredentials { + username: "admin".into(), + password: "password".into(), + }) + .unwrap(); + let state = Arc::new(AppState::new( + Arc::new(ArcSwap::from_pointee(config)), + temp.path().join("config.toml"), + queue, + logs, + Arc::new(TokenStore::new()), + credentials, + )); + let router = build_router(state.clone()); + #[cfg(feature = "webdav")] + let router = router.layer(axum::Extension(None::>)); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let base = format!("http://{}", listener.local_addr().unwrap()); + let handle = tokio::spawn(async move { + axum::serve(listener, router).await.unwrap(); + }); + + let root = temp.path().to_path_buf(); + for id in ["job-a", "job-b"] { + let job = NzbJob { + id: id.into(), + name: id.into(), + category: "Default".into(), + status: JobStatus::Queued, + priority: Priority::Normal, + total_bytes: 100, + downloaded_bytes: 0, + file_count: 0, + files_completed: 0, + article_count: 0, + articles_downloaded: 0, + articles_failed: 0, + added_at: Utc::now(), + completed_at: None, + work_dir: root.join("incomplete").join(id), + output_dir: root.join("complete").join(id), + password: None, + error_message: None, + speed_bps: 0, + server_stats: Vec::new(), + pp_override: None, + files: Vec::new(), + }; + state.queue_manager.add_job(job, None).unwrap(); + } + + App { + base, + state, + _temp: temp, + handle, + } +} + +async fn login(app: &App, client: &reqwest::Client) -> String { + let tokens = client + .post(format!("{}/api/auth/login", app.base)) + .json(&serde_json::json!({"username": "admin", "password": "password"})) + .send() + .await + .unwrap() + .json::() + .await + .unwrap(); + tokens["access_token"].as_str().unwrap().to_string() +} + +fn job_ids(app: &App) -> Vec { + app.state + .queue_manager + .get_jobs() + .into_iter() + .map(|job| job.id) + .collect() +} + +const MISSING: &str = "00000000-0000-0000-0000-000000000000"; + +async fn bulk( + app: &App, + client: &reqwest::Client, + access: &str, + body: serde_json::Value, +) -> serde_json::Value { + let response = client + .post(format!("{}/api/queue/bulk", app.base)) + .bearer_auth(access) + .json(&body) + .send() + .await + .unwrap(); + assert_eq!(response.status().as_u16(), 200); + response.json().await.unwrap() +} + +#[tokio::test] +async fn bulk_delete_counts_unknown_ids_as_failed() { + let app = start_app().await; + let client = reqwest::Client::new(); + let access = login(&app, &client).await; + + let response = bulk( + &app, + &client, + &access, + serde_json::json!({"action": "delete", "ids": ["job-a", "job-b", MISSING]}), + ) + .await; + assert_eq!(response["status"], false, "{response}"); + assert_eq!(response["succeeded"], 2, "{response}"); + assert_eq!(response["failed"], 1, "{response}"); + let failures = response["failures"].as_array().expect("failures array"); + assert_eq!(failures.len(), 1, "{response}"); + assert_eq!(failures[0]["id"], MISSING); + assert_eq!(failures[0]["error"]["error_kind"], "job_not_found"); + assert!(job_ids(&app).is_empty()); +} + +#[tokio::test] +async fn every_bulk_action_counts_unknown_ids_as_failed() { + let app = start_app().await; + let client = reqwest::Client::new(); + let access = login(&app, &client).await; + + for (body, succeeded) in [ + (serde_json::json!({"action": "pause"}), 2), + (serde_json::json!({"action": "priority", "value": 2}), 2), + (serde_json::json!({"action": "category", "value": "tv"}), 2), + // Individual resume is refused while downloads are globally paused, + // so only the unknown id's outcome is checked here. + (serde_json::json!({"action": "resume"}), 0), + ] { + let mut body = body; + body["ids"] = serde_json::json!(["job-a", "job-b", MISSING]); + let response = bulk(&app, &client, &access, body.clone()).await; + assert_eq!(response["status"], false, "{body}: {response}"); + assert_eq!(response["succeeded"], succeeded, "{body}: {response}"); + assert_eq!(response["failed"], 3 - succeeded, "{body}: {response}"); + let failures = response["failures"].as_array().expect("failures array"); + let missing = failures + .iter() + .find(|failure| failure["id"] == MISSING) + .unwrap_or_else(|| panic!("{body}: {response}")); + if succeeded == 2 { + assert_eq!(missing["error"]["error_kind"], "job_not_found", "{body}"); + } + } + + // A fully successful request reports no failures. + let response = bulk( + &app, + &client, + &access, + serde_json::json!({"action": "pause", "ids": ["job-a"]}), + ) + .await; + assert_eq!(response["status"], true, "{response}"); + assert_eq!(response["succeeded"], 1); + assert_eq!(response["failures"], serde_json::json!([])); +} diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index c46ea43b..fcabde3e 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -3866,6 +3866,11 @@ impl QueueManager { .collect() } + /// Whether a job with exactly this id is in the queue. + pub fn contains_job(&self, job_id: &str) -> bool { + self.jobs.lock().contains_key(job_id) + } + /// Get a single job by ID (with files included). pub fn get_job(&self, job_id: &str) -> Option { let jobs = self.jobs.lock(); From bedecf35307e0f018978bdd3cc9bee0c4b3605ec Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:04:53 -0500 Subject: [PATCH 04/33] fix(frontend): surface server error messages on failed mutations Failed form submits and other mutating requests threw away the response body. For example, adding an RSS feed with a private-address URL returns 400 with "Feed URL rejected: ..." in `human_readable`, but the UI showed at most a generic toast and the add-feed form stayed open with no explanation. Some handlers read `error.message`, which the central API error mapping never sends. Others had no error handler, so history retry/delete/clear, queue bulk actions, the priority change, group mark-all-read and pause-for failed silently. This adds a shared core/http/http-error helper. It takes `human_readable`, falls back to the legacy `error` field, then `message`, then short plain-text bodies, and prefixes the result with what the user was trying to do. Every mutation in settings, RSS, queue, history, groups and the app shell now uses it. Login and the setup wizard use it for their inline messages. The RSS feed and rule forms also show the error inline while the form stays open. Bulk queue actions now report failures once instead of dropping them. 401s are not toasted, because the auth interceptor already refreshes the session or redirects to login. Fixes #104 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 6a2b0c1ffb5ae0ed6e41ebc8f56baaee5f784c59) --- apps/rustnzb/frontend/src/app/app.spec.ts | 9 +- apps/rustnzb/frontend/src/app/app.ts | 15 ++- .../src/app/core/http/http-error.spec.ts | 82 ++++++++++++++ .../frontend/src/app/core/http/http-error.ts | 80 ++++++++++++++ .../src/app/features/auth/login.component.ts | 5 +- .../groups/group-browser-dialog.component.ts | 17 ++- .../features/groups/groups-view.component.ts | 19 +++- .../history/history-view.component.ts | 26 +++-- .../queue/queue-view.component.spec.ts | 51 ++++++++- .../features/queue/queue-view.component.ts | 104 +++++++++++++----- .../features/rss/rss-view.component.spec.ts | 37 ++++++- .../app/features/rss/rss-view.component.ts | 37 ++++++- .../settings/settings-view.component.spec.ts | 19 +++- .../settings/settings-view.component.ts | 34 +++--- .../app/features/welcome/welcome.component.ts | 7 +- 15 files changed, 452 insertions(+), 90 deletions(-) create mode 100644 apps/rustnzb/frontend/src/app/core/http/http-error.spec.ts create mode 100644 apps/rustnzb/frontend/src/app/core/http/http-error.ts diff --git a/apps/rustnzb/frontend/src/app/app.spec.ts b/apps/rustnzb/frontend/src/app/app.spec.ts index 6cb50f48..591230f0 100644 --- a/apps/rustnzb/frontend/src/app/app.spec.ts +++ b/apps/rustnzb/frontend/src/app/app.spec.ts @@ -23,6 +23,7 @@ function makeApp(postResult = new Subject()) { navigate: vi.fn(() => Promise.resolve(true)), }; const pauseState = new PauseStateService(); + const snack = { open: vi.fn() }; const app = new App( api as never, auth as never, @@ -31,8 +32,9 @@ function makeApp(postResult = new Subject()) { {} as never, {} as never, pauseState, + snack as never, ); - return { app, api, pauseState, postResult }; + return { app, api, pauseState, postResult, snack }; } describe('App global pause control', () => { @@ -46,13 +48,16 @@ describe('App global pause control', () => { }); it('rolls back the shared state if the global request fails', () => { - const { app, pauseState, postResult } = makeApp(); + const { app, pauseState, postResult, snack } = makeApp(); vi.spyOn(app, 'pollStatus').mockImplementation(() => {}); app.togglePause(); postResult.error(new Error('request failed')); expect(pauseState.paused()).toBe(false); + expect(snack.open).toHaveBeenCalledWith('Failed to pause downloads', 'Close', { + duration: 5000, + }); }); }); diff --git a/apps/rustnzb/frontend/src/app/app.ts b/apps/rustnzb/frontend/src/app/app.ts index e5a787e6..b4558cba 100644 --- a/apps/rustnzb/frontend/src/app/app.ts +++ b/apps/rustnzb/frontend/src/app/app.ts @@ -21,6 +21,8 @@ import { WidthModeService } from './core/services/width-mode.service'; import { PauseStateService } from './core/services/pause-state.service'; import { ThemeService } from './core/services/theme.service'; import { IconComponent } from './shared/icon.component'; +import { MatSnackBar } from '@angular/material/snack-bar'; +import { showHttpError } from './core/http/http-error'; export function isDemoPath(pathname: string): boolean { return pathname === '/demo' || pathname.startsWith('/demo/'); @@ -451,6 +453,7 @@ export class App implements OnInit, OnDestroy { public widthMode: WidthModeService, readonly theme: ThemeService, pauseState: PauseStateService, + private snack: MatSnackBar, ) { this.paused = pauseState.paused; this.authenticated = authService.authenticated; @@ -509,9 +512,14 @@ export class App implements OnInit, OnDestroy { this.paused.set(!wasPaused); this.api.post(action).subscribe({ next: () => this.pollStatus(), - error: () => { + error: (err) => { this.paused.set(wasPaused); this.pollStatus(); + showHttpError( + this.snack, + err, + wasPaused ? 'Failed to resume downloads' : 'Failed to pause downloads', + ); }, }); this.pauseMenuOpen = false; @@ -524,7 +532,10 @@ export class App implements OnInit, OnDestroy { } pauseFor(secs: number): void { - this.api.post(`/queue/pause-for?duration_secs=${secs}`).subscribe(() => this.pollStatus()); + this.api.post(`/queue/pause-for?duration_secs=${secs}`).subscribe({ + next: () => this.pollStatus(), + error: (err) => showHttpError(this.snack, err, 'Failed to pause downloads'), + }); this.pauseMenuOpen = false; } diff --git a/apps/rustnzb/frontend/src/app/core/http/http-error.spec.ts b/apps/rustnzb/frontend/src/app/core/http/http-error.spec.ts new file mode 100644 index 00000000..d8163f80 --- /dev/null +++ b/apps/rustnzb/frontend/src/app/core/http/http-error.spec.ts @@ -0,0 +1,82 @@ +import '@angular/compiler'; + +import { HttpErrorResponse } from '@angular/common/http'; +import { describe, expect, it, vi } from 'vitest'; + +import { httpErrorDetail, httpErrorMessage, isAuthHandledError, showHttpError } from './http-error'; + +function httpError(status: number, error: unknown): HttpErrorResponse { + return new HttpErrorResponse({ status, error, url: '/api/x' }); +} + +describe('httpErrorMessage', () => { + it('prefers the central human_readable field', () => { + const err = httpError(400, { + error_kind: 'bad_request', + human_readable: 'Feed URL rejected: URL targets a private/reserved address', + status: 400, + }); + expect(httpErrorMessage(err, 'Failed to save feed')).toBe( + 'Failed to save feed: Feed URL rejected: URL targets a private/reserved address', + ); + }); + + it('falls back to the legacy error and message fields', () => { + expect(httpErrorMessage(httpError(400, { status: false, error: 'No job ID' }), 'Failed')).toBe( + 'Failed: No job ID', + ); + expect(httpErrorMessage(httpError(422, { message: 'Bad NZB' }), 'Failed')).toBe( + 'Failed: Bad NZB', + ); + }); + + it('parses JSON bodies delivered as text and accepts short plain-text bodies', () => { + expect( + httpErrorMessage(httpError(400, '{"human_readable":"Invalid regex"}'), 'Failed to save rule'), + ).toBe('Failed to save rule: Invalid regex'); + expect(httpErrorMessage(httpError(415, 'Expected JSON content type'), 'Failed')).toBe( + 'Failed: Expected JSON content type', + ); + }); + + it('uses the fallback when there is no usable server message', () => { + expect(httpErrorMessage(httpError(500, null), 'Failed to save')).toBe('Failed to save'); + expect( + httpErrorMessage(httpError(502, 'Bad gateway'), 'Failed'), + ).toBe('Failed'); + expect(httpErrorMessage(httpError(400, { human_readable: ' ' }), 'Failed')).toBe('Failed'); + expect(httpErrorMessage(new Error('boom'), 'Failed')).toBe('Failed'); + expect(httpErrorMessage(undefined, 'Failed')).toBe('Failed'); + }); + + it('does not repeat the fallback when the server message already is it', () => { + expect(httpErrorMessage(httpError(400, { error: 'Failed' }), 'Failed')).toBe('Failed'); + }); +}); + +describe('httpErrorDetail', () => { + it('returns only the server text, or null', () => { + expect(httpErrorDetail(httpError(400, { human_readable: 'Bad', error: 'ignored' }))).toBe( + 'Bad', + ); + expect(httpErrorDetail(httpError(0, null))).toBeNull(); + expect(httpErrorDetail(new Error('client'))).toBeNull(); + }); +}); + +describe('showHttpError', () => { + it('opens a snackbar with the extracted message', () => { + const snack = { open: vi.fn() }; + showHttpError(snack, httpError(400, { human_readable: 'Bad' }), 'Failed to save'); + expect(snack.open).toHaveBeenCalledWith('Failed to save: Bad', 'Close', { duration: 5000 }); + }); + + it('stays silent on 401s, which the auth interceptor already handles', () => { + const snack = { open: vi.fn() }; + const err = httpError(401, { human_readable: 'Unauthorized' }); + expect(isAuthHandledError(err)).toBe(true); + showHttpError(snack, err, 'Failed to save'); + expect(snack.open).not.toHaveBeenCalled(); + expect(isAuthHandledError(httpError(403, null))).toBe(false); + }); +}); diff --git a/apps/rustnzb/frontend/src/app/core/http/http-error.ts b/apps/rustnzb/frontend/src/app/core/http/http-error.ts new file mode 100644 index 00000000..2dd6e1e6 --- /dev/null +++ b/apps/rustnzb/frontend/src/app/core/http/http-error.ts @@ -0,0 +1,80 @@ +import type { HttpErrorResponse } from '@angular/common/http'; + +/** Anything with MatSnackBar's `open` signature (kept narrow for tests). */ +export interface SnackOpener { + open(message: string, action?: string, config?: { duration?: number }): unknown; +} + +const MAX_TEXT_BODY = 300; + +function nonEmpty(value: unknown): string | null { + return typeof value === 'string' && value.trim() ? value.trim() : null; +} + +function fromBody(body: unknown): string | null { + if (typeof body === 'string') { + const text = body.trim(); + if (!text) return null; + if (text.startsWith('{')) { + try { + return fromBody(JSON.parse(text)); + } catch { + // Not JSON after all; treat it as plain text below. + } + } + // Short plain-text bodies (e.g. axum extractor rejections) are useful; + // HTML error pages from a proxy are not. + if (text.startsWith('<') || text.length > MAX_TEXT_BODY) return null; + return text; + } + if (body && typeof body === 'object') { + const b = body as Record; + // Central API error shape first, then the legacy `{status:false, error}` + // shape, then older `{message}` bodies. + return nonEmpty(b['human_readable']) ?? nonEmpty(b['error']) ?? nonEmpty(b['message']); + } + return null; +} + +/** + * The server's own explanation of a failed request, or `null` when the + * response carries nothing usable (network error, HTML proxy page, ...). + */ +export function httpErrorDetail(err: unknown): string | null { + // Duck-typed rather than `instanceof HttpErrorResponse` so errors that were + // re-thrown or built by hand with the same `{status, error}` shape work too. + if (!err || typeof err !== 'object' || !('error' in err)) return null; + return fromBody((err as HttpErrorResponse).error); +} + +/** + * Turn a failed request into a user-facing message: the server's own + * explanation (`human_readable`, falling back to `error`/`message`) prefixed + * with what the user was trying to do, or just `fallback` when the response + * carries nothing usable. + */ +export function httpErrorMessage(err: unknown, fallback: string): string { + const detail = httpErrorDetail(err); + if (!detail || detail === fallback) return fallback; + return `${fallback}: ${detail}`; +} + +/** + * A 401 reaching a component means the auth interceptor already tried to + * refresh the session and has redirected to the login page; a toast on top + * of that redirect is noise. + */ +export function isAuthHandledError(err: unknown): boolean { + return !!err && typeof err === 'object' && (err as HttpErrorResponse).status === 401; +} + +/** Show a failed request as a toast, unless the auth flow is handling it. */ +export function showHttpError( + snack: SnackOpener, + err: unknown, + fallback: string, + duration = 5000, +): void { + if (isAuthHandledError(err)) return; + snack.open(httpErrorMessage(err, fallback), 'Close', { duration }); +} diff --git a/apps/rustnzb/frontend/src/app/features/auth/login.component.ts b/apps/rustnzb/frontend/src/app/features/auth/login.component.ts index d9a3f7ab..0d1e1a67 100644 --- a/apps/rustnzb/frontend/src/app/features/auth/login.component.ts +++ b/apps/rustnzb/frontend/src/app/features/auth/login.component.ts @@ -3,6 +3,7 @@ import { CommonModule } from '@angular/common'; import { FormsModule } from '@angular/forms'; import { Router } from '@angular/router'; import { AuthService } from '../../core/services/auth.service'; +import { httpErrorDetail } from '../../core/http/http-error'; @Component({ selector: 'app-login', @@ -210,10 +211,8 @@ export class LoginComponent implements OnInit { this.errorMessage.set('Invalid username or password.'); } else if (err.status === 409) { this.errorMessage.set('An account already exists. Please sign in instead.'); - } else if (err.error?.message) { - this.errorMessage.set(err.error.message); } else { - this.errorMessage.set('An error occurred. Please try again.'); + this.errorMessage.set(httpErrorDetail(err) ?? 'An error occurred. Please try again.'); } }, }); diff --git a/apps/rustnzb/frontend/src/app/features/groups/group-browser-dialog.component.ts b/apps/rustnzb/frontend/src/app/features/groups/group-browser-dialog.component.ts index 78d6c38b..47d25741 100644 --- a/apps/rustnzb/frontend/src/app/features/groups/group-browser-dialog.component.ts +++ b/apps/rustnzb/frontend/src/app/features/groups/group-browser-dialog.component.ts @@ -6,10 +6,10 @@ import { MatButtonModule } from '@angular/material/button'; import { MatIconModule } from '@angular/material/icon'; import { MatProgressBarModule } from '@angular/material/progress-bar'; import { MatSnackBar, MatSnackBarModule } from '@angular/material/snack-bar'; -import { HttpErrorResponse } from '@angular/common/http'; import { GroupService } from '../../core/services/group.service'; import { GroupRow } from '../../core/models/group.model'; import { IconComponent } from '../../shared/icon.component'; +import { showHttpError } from '../../core/http/http-error'; @Component({ selector: 'app-group-browser-dialog', @@ -100,12 +100,9 @@ export class GroupBrowserDialogComponent implements OnInit { this.refreshing.set(true); this.svc.refresh().subscribe({ next: r => { this.refreshing.set(false); this.snack.open(r.message, 'Close', { duration: 3000 }); this.loadGroups(); }, - error: (e: HttpErrorResponse) => { + error: (e) => { this.refreshing.set(false); - const msg = e.status === 400 - ? (e.error?.human_readable || 'No servers configured — add one in Settings first.') - : 'Refresh failed'; - this.snack.open(msg, 'Close', { duration: 5000 }); + showHttpError(this.snack, e, 'Refresh failed'); }, }); } @@ -132,11 +129,11 @@ export class GroupBrowserDialogComponent implements OnInit { ); clearPending(); }, - error: () => { - this.snack.open( + error: (err) => { + showHttpError( + this.snack, + err, `Failed to ${wasSubscribed ? 'unsubscribe from' : 'subscribe to'} ${g.name}`, - 'Close', - { duration: 4000 }, ); clearPending(); }, diff --git a/apps/rustnzb/frontend/src/app/features/groups/groups-view.component.ts b/apps/rustnzb/frontend/src/app/features/groups/groups-view.component.ts index df5ca485..0b69b546 100644 --- a/apps/rustnzb/frontend/src/app/features/groups/groups-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/groups/groups-view.component.ts @@ -7,6 +7,7 @@ import { GroupService } from '../../core/services/group.service'; import { GroupRow, HeaderRow } from '../../core/models/group.model'; import { GroupBrowserDialogComponent } from './group-browser-dialog.component'; import { IconComponent } from '../../shared/icon.component'; +import { showHttpError } from '../../core/http/http-error'; @Component({ selector: 'app-groups-view', @@ -493,7 +494,10 @@ export class GroupsViewComponent implements OnInit { this.fetching.set(true); this.svc.fetchHeaders(g.id).subscribe({ next: () => this.snack.open('Fetching headers…', 'Close', { duration: 2000 }), - error: () => this.fetching.set(false), + error: (err) => { + this.fetching.set(false); + showHttpError(this.snack, err, 'Failed to fetch headers'); + }, }); const poll = setInterval(() => { this.loadHeaders(); @@ -519,10 +523,13 @@ export class GroupsViewComponent implements OnInit { markAllRead(): void { const g = this.selectedGroup(); if (!g) return; - this.svc.markAllRead(g.id).subscribe(() => { - this.loadHeaders(); - this.loadGroups(); - this.snack.open('All marked read', 'Close', { duration: 2000 }); + this.svc.markAllRead(g.id).subscribe({ + next: () => { + this.loadHeaders(); + this.loadGroups(); + this.snack.open('All marked read', 'Close', { duration: 2000 }); + }, + error: (err) => showHttpError(this.snack, err, 'Failed to mark all read'), }); } @@ -567,7 +574,7 @@ export class GroupsViewComponent implements OnInit { this.snack.open(r.message, 'Close', { duration: 3000 }); this.selectedIds.set([]); }, - error: () => this.snack.open('Download failed', 'Close', { duration: 5000 }), + error: (err) => showHttpError(this.snack, err, 'Download failed'), }); } diff --git a/apps/rustnzb/frontend/src/app/features/history/history-view.component.ts b/apps/rustnzb/frontend/src/app/features/history/history-view.component.ts index db64d0dd..ea8a9a53 100644 --- a/apps/rustnzb/frontend/src/app/features/history/history-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/history/history-view.component.ts @@ -6,6 +6,7 @@ import { ApiService } from '../../core/services/api.service'; import { HistoryEntry, StatusResponse } from '../../core/models/queue.model'; import { ConfirmService } from '../../shared/confirm.service'; import { IconComponent } from '../../shared/icon.component'; +import { showHttpError } from '../../core/http/http-error'; type StatusFilter = 'all' | 'completed' | 'failed'; type TimeFilter = '7d' | '30d' | 'all'; @@ -407,21 +408,27 @@ export class HistoryViewComponent implements OnInit, OnDestroy { }); retry(id: string): void { - this.api.post(`/history/${id}/retry`).subscribe(() => { - this.load(); - this.snack.open('Retrying…', 'Close', { duration: 2000 }); + this.api.post(`/history/${id}/retry`).subscribe({ + next: () => { + this.load(); + this.snack.open('Retrying…', 'Close', { duration: 2000 }); + }, + error: (err) => showHttpError(this.snack, err, 'Failed to retry job'), }); } addToMedia(id: string): void { this.api.post(`/dav/add?id=${id}`).subscribe({ next: () => this.snack.open('Queued for Media Library', 'Close', { duration: 3000 }), - error: () => this.snack.open('Failed to add to Media Library', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to add to Media Library'), }); } remove(id: string): void { - this.api.delete(`/history/${id}`).subscribe(() => this.load()); + this.api.delete(`/history/${id}`).subscribe({ + next: () => this.load(), + error: (err) => showHttpError(this.snack, err, 'Failed to delete history entry'), + }); } clearAll(): void { @@ -434,9 +441,12 @@ export class HistoryViewComponent implements OnInit, OnDestroy { }) .subscribe((ok) => { if (!ok) return; - this.api.delete('/history').subscribe(() => { - this.load(); - this.snack.open('History cleared', 'Close', { duration: 2000 }); + this.api.delete('/history').subscribe({ + next: () => { + this.load(); + this.snack.open('History cleared', 'Close', { duration: 2000 }); + }, + error: (err) => showHttpError(this.snack, err, 'Failed to clear history'), }); }); } diff --git a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts index e30dca81..172de47e 100644 --- a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts +++ b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts @@ -1,5 +1,6 @@ import '@angular/compiler'; +import { HttpErrorResponse } from '@angular/common/http'; import { Observable, of, Subject, throwError } from 'rxjs'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { convertToParamMap, ParamMap } from '@angular/router'; @@ -236,14 +237,26 @@ describe('QueueViewComponent', () => { it('reloads queue and shows feedback after a failed row action', () => { const { component, snackBar } = makeComponent({ - delete: vi.fn(() => throwError(() => new Error('Delete failed'))), + delete: vi.fn(() => + throwError( + () => + new HttpErrorResponse({ + status: 409, + error: { error_kind: 'conflict', human_readable: 'Job is post-processing', status: 409 }, + }), + ), + ), }); const loadQueue = vi.spyOn(component, 'loadQueue').mockImplementation(() => {}); component.deleteJob(makeJob({ id: 'job-1' })); expect(loadQueue).toHaveBeenCalledTimes(1); - expect(snackBar.open).toHaveBeenCalledWith('Delete failed', 'Close', { duration: 4000 }); + expect(snackBar.open).toHaveBeenCalledWith( + 'Failed to remove job: Job is post-processing', + 'Close', + { duration: 5000 }, + ); }); it('reorders jobs before the target row', () => { @@ -318,4 +331,38 @@ describe('QueueViewComponent', () => { expect(component.historyCollapsed()).toBe(false); expect(localStorage.getItem(component.HISTORY_KEY)).toBe('false'); }); + + it('reports bulk action failures once with the server message', () => { + const notFound = new HttpErrorResponse({ + status: 404, + error: { error_kind: 'job_not_found', human_readable: 'Job not found: job-2', status: 404 }, + }); + const { component, snackBar } = makeComponent({ + post: vi.fn((path: string) => (path.includes('job-2') ? throwError(() => notFound) : of({}))), + }); + const loadQueue = vi.spyOn(component, 'loadQueue').mockImplementation(() => {}); + component.selectedIds.set(new Set(['job-1', 'job-2'])); + + component.bulkPause(); + + expect(snackBar.open).toHaveBeenCalledTimes(1); + expect(snackBar.open).toHaveBeenCalledWith( + 'Failed to pause 1 of 2 job(s): Job not found: job-2', + 'Close', + { duration: 5000 }, + ); + expect(loadQueue).toHaveBeenCalledTimes(1); + expect(component.selectedIds().size).toBe(0); + }); + + it('does not toast a 401 that the auth interceptor is handling', () => { + const { component, snackBar } = makeComponent({ + post: vi.fn(() => throwError(() => new HttpErrorResponse({ status: 401 }))), + }); + vi.spyOn(component, 'loadQueue').mockImplementation(() => {}); + + component.pauseJob('job-1'); + + expect(snackBar.open).not.toHaveBeenCalled(); + }); }); diff --git a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts index 0f2d64ec..77ffd57d 100644 --- a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts @@ -4,7 +4,7 @@ import { FormsModule } from '@angular/forms'; import { ActivatedRoute, Router, RouterModule } from '@angular/router'; import { HttpClient } from '@angular/common/http'; import { MatSnackBar, MatSnackBarModule } from '@angular/material/snack-bar'; -import { Observable, Subscription, finalize } from 'rxjs'; +import { Observable, Subscription, catchError, finalize, forkJoin, map, of } from 'rxjs'; import { ApiService } from '../../core/services/api.service'; import { AddNzbService } from '../../core/services/add-nzb.service'; import { PauseStateService } from '../../core/services/pause-state.service'; @@ -12,6 +12,7 @@ import { NzbJob, QueueResponse, StatusResponse } from '../../core/models/queue.m import { HistoryViewComponent } from '../history/history-view.component'; import { ConfirmService } from '../../shared/confirm.service'; import { IconComponent } from '../../shared/icon.component'; +import { isAuthHandledError, showHttpError } from '../../core/http/http-error'; interface CategoryConfig { name: string; @@ -1441,12 +1442,9 @@ export class QueueViewComponent implements OnInit, OnDestroy { this.loadQueue(); }, error: (err) => { - const msg = - err.error?.message || - (err.status === 413 ? 'Upload too large' : err.statusText) || - 'Upload failed'; - this.snackBar.open('Failed: ' + msg, 'Close', { duration: 5000 }); this.uploading = false; + const fallback = err?.status === 413 ? 'Upload too large' : 'Upload failed'; + showHttpError(this.snackBar, err, fallback); }, }); } @@ -1465,10 +1463,9 @@ export class QueueViewComponent implements OnInit, OnDestroy { this.showAddPanel = false; this.loadQueue(); }, - error: (err: any) => { - const msg = err.error?.message || err.statusText || 'Failed'; - this.snackBar.open('Failed: ' + msg, 'Close', { duration: 5000 }); + error: (err) => { this.uploading = false; + showHttpError(this.snackBar, err, 'Failed to add NZB from URL'); }, }); } @@ -1483,6 +1480,7 @@ export class QueueViewComponent implements OnInit, OnDestroy { id: string, actionFactory: () => Observable, successMessage?: string, + failureMessage = 'Action failed', ): void { if (this.isActionPending(id)) return; @@ -1505,16 +1503,20 @@ export class QueueViewComponent implements OnInit, OnDestroy { } this.loadQueue(); }, - error: (err: any) => { - const msg = err?.error?.message || err?.message || 'Action failed. Please try again.'; - this.snackBar.open(msg, 'Close', { duration: 4000 }); + error: (err) => { + showHttpError(this.snackBar, err, failureMessage); this.loadQueue(); }, }); } pauseJob(id: string): void { - this.withPendingJobAction(id, () => this.api.post(`/queue/${id}/pause`), 'Job paused'); + this.withPendingJobAction( + id, + () => this.api.post(`/queue/${id}/pause`), + 'Job paused', + 'Failed to pause job', + ); } resumeJob(id: string): void { @@ -1522,13 +1524,21 @@ export class QueueViewComponent implements OnInit, OnDestroy { // a misleading request/toast if stale DOM or keyboard input fires while // the global pause state is active. if (this.paused()) return; - this.withPendingJobAction(id, () => this.api.post(`/queue/${id}/resume`), 'Job resumed'); + this.withPendingJobAction( + id, + () => this.api.post(`/queue/${id}/resume`), + 'Job resumed', + 'Failed to resume job', + ); } setPriority(job: NzbJob, priority: number): void { this.api.put(`/queue/${job.id}/priority`, { priority }).subscribe({ next: () => this.loadQueue(), - error: () => {}, + error: (err) => { + showHttpError(this.snackBar, err, 'Failed to change priority'); + this.loadQueue(); + }, }); } @@ -1545,7 +1555,12 @@ export class QueueViewComponent implements OnInit, OnDestroy { }) .subscribe((ok) => { if (!ok) return; - this.withPendingJobAction(job.id, () => this.api.delete(`/queue/${job.id}`)); + this.withPendingJobAction( + job.id, + () => this.api.delete(`/queue/${job.id}`), + undefined, + 'Failed to remove job', + ); }); } @@ -1609,12 +1624,10 @@ export class QueueViewComponent implements OnInit, OnDestroy { this.reorderPending.set(false); this.loadQueue(); }, - error: (err: any) => { + error: (err) => { this.reorderPending.set(false); this.jobs.set(previousJobs); - const msg = - err?.error?.message || err?.message || 'Unable to reorder queue. Please try again.'; - this.snackBar.open(msg, 'Close', { duration: 4000 }); + showHttpError(this.snackBar, err, 'Unable to reorder queue'); }, }); } @@ -1668,17 +1681,50 @@ export class QueueViewComponent implements OnInit, OnDestroy { this.clearSelection(); } } + + /** + * Run one request per selected job and report failures once, with the + * first server message, instead of silently dropping them. + */ + private runBulk( + ids: string[], + request: (id: string) => Observable, + verb: string, + ): void { + this.clearSelection(); + if (ids.length === 0) return; + forkJoin( + ids.map((id) => + request(id).pipe( + map(() => null), + catchError((err: unknown) => of(err)), + ), + ), + ).subscribe((results) => { + const failures = results.filter((r) => r !== null && !isAuthHandledError(r)); + if (failures.length > 0) { + showHttpError( + this.snackBar, + failures[0], + `Failed to ${verb} ${failures.length} of ${ids.length} job(s)`, + ); + } + this.loadQueue(); + }); + } bulkResume(): void { - Array.from(this.selectedIds()).forEach((id) => - this.api.post(`/queue/${id}/resume`).subscribe(), + this.runBulk( + Array.from(this.selectedIds()), + (id) => this.api.post(`/queue/${id}/resume`), + 'resume', ); - this.clearSelection(); - setTimeout(() => this.loadQueue(), 300); } bulkPause(): void { - Array.from(this.selectedIds()).forEach((id) => this.api.post(`/queue/${id}/pause`).subscribe()); - this.clearSelection(); - setTimeout(() => this.loadQueue(), 300); + this.runBulk( + Array.from(this.selectedIds()), + (id) => this.api.post(`/queue/${id}/pause`), + 'pause', + ); } bulkDelete(): void { const ids = Array.from(this.selectedIds()); @@ -1692,9 +1738,7 @@ export class QueueViewComponent implements OnInit, OnDestroy { }) .subscribe((ok) => { if (!ok) return; - ids.forEach((id) => this.api.delete(`/queue/${id}`).subscribe()); - this.clearSelection(); - setTimeout(() => this.loadQueue(), 300); + this.runBulk(ids, (id) => this.api.delete(`/queue/${id}`), 'remove'); }); } diff --git a/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.spec.ts b/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.spec.ts index 2f514205..7cd00f89 100644 --- a/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.spec.ts +++ b/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.spec.ts @@ -1,6 +1,7 @@ import '@angular/compiler'; import { describe, expect, it, vi } from 'vitest'; +import { HttpErrorResponse } from '@angular/common/http'; import { of, throwError } from 'rxjs'; import { ApiService } from '../../core/services/api.service'; @@ -42,7 +43,7 @@ describe('RssViewComponent', () => { component.feedForm = { ...component.feedForm, name: 'daily', url: 'https://example.test' }; component.saveFeed(); expect(api.post).toHaveBeenCalledWith('/config/rss-feeds', expect.objectContaining({ name: 'daily' })); - expect(snack.open).toHaveBeenCalledWith('Failed to save feed', 'Close', { duration: 3000 }); + expect(snack.open).toHaveBeenCalledWith('Failed to save feed', 'Close', { duration: 5000 }); }); it('encodes edited feed names and submits parsed rule feed names', () => { @@ -56,4 +57,38 @@ describe('RssViewComponent', () => { component.saveRule(); expect(api.post).toHaveBeenCalledWith('/rss/rules', expect.objectContaining({ feed_names: ['daily', 'tv'] })); }); + + it('shows the server message inline and as a toast when adding a feed is rejected', () => { + const rejected = new HttpErrorResponse({ + status: 400, + url: '/api/config/rss-feeds', + error: { + error_kind: 'bad_request', + human_readable: 'Feed URL rejected: URL targets a private/reserved address', + status: 400, + }, + }); + const { component, snack } = createComponent({ post: vi.fn(() => throwError(() => rejected)) }); + component.showAddFeed(); + component.feedForm = { ...component.feedForm, name: 'lan', url: 'http://192.168.1.10/rss' }; + component.saveFeed(); + + const expected = 'Failed to save feed: Feed URL rejected: URL targets a private/reserved address'; + expect(component.feedFormVisible()).toBe(true); + expect(component.feedError()).toBe(expected); + expect(snack.open).toHaveBeenCalledWith(expected, 'Close', { duration: 5000 }); + + component.cancelFeedForm(); + component.showAddFeed(); + expect(component.feedError()).toBeNull(); + }); + + it('shows the server message inline when saving a rule is rejected', () => { + const rejected = new HttpErrorResponse({ status: 400, error: { human_readable: 'Invalid regex' } }); + const { component } = createComponent({ post: vi.fn(() => throwError(() => rejected)) }); + component.showAddRule(); + component.ruleForm = { ...component.ruleForm, name: 'TV', match_regex: '(' }; + component.saveRule(); + expect(component.ruleError()).toBe('Failed to save rule: Invalid regex'); + }); }); diff --git a/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.ts b/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.ts index 57129654..1f1519e2 100644 --- a/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/rss/rss-view.component.ts @@ -4,6 +4,7 @@ import { FormsModule } from '@angular/forms'; import { MatSnackBar, MatSnackBarModule } from '@angular/material/snack-bar'; import { ApiService } from '../../core/services/api.service'; import { ConfirmService } from '../../shared/confirm.service'; +import { httpErrorMessage, isAuthHandledError, showHttpError } from '../../core/http/http-error'; interface RssFeed { name: string; url: string; poll_interval_secs: number; category: string | null; @@ -86,6 +87,9 @@ interface RuleFormModel { + @if (feedError()) { + + }
@@ -154,6 +158,9 @@ interface RuleFormModel {
+ @if (ruleError()) { + + }
@@ -270,6 +277,12 @@ interface RuleFormModel { border-bottom: 1px solid var(--line); } .form-actions { margin-top: 14px; display: flex; gap: 8px; } + .form-error { + margin-top: 14px; + background: color-mix(in srgb, var(--danger) 10%, transparent); + border: 1px solid var(--danger); + border-radius: 6px; padding: 10px 14px; color: var(--danger); font-size: 13px; + } td.dim { color: var(--mute); } .dim { color: var(--mute); } @@ -285,10 +298,12 @@ export class RssViewComponent implements OnInit { feedFormVisible = signal(false); editingFeedName = signal(null); feedForm: FeedFormModel = this.emptyFeedForm(); + feedError = signal(null); ruleFormVisible = signal(false); editingRuleId = signal(null); ruleForm: RuleFormModel = this.emptyRuleForm(); + ruleError = signal(null); // ---- Stat-card derivations ---- enabledFeedCount = computed(() => this.feeds().filter(f => f.enabled).length); @@ -332,6 +347,7 @@ export class RssViewComponent implements OnInit { showAddFeed(): void { this.feedForm = this.emptyFeedForm(); this.editingFeedName.set(null); + this.feedError.set(null); this.feedFormVisible.set(true); } @@ -346,12 +362,14 @@ export class RssViewComponent implements OnInit { auto_download: f.auto_download, }; this.editingFeedName.set(f.name); + this.feedError.set(null); this.feedFormVisible.set(true); } cancelFeedForm(): void { this.feedFormVisible.set(false); this.editingFeedName.set(null); + this.feedError.set(null); } saveFeed(): void { @@ -378,7 +396,10 @@ export class RssViewComponent implements OnInit { this.cancelFeedForm(); this.loadAll(); }, - error: () => this.snack.open('Failed to save feed', 'Close', { duration: 3000 }), + error: (err) => { + if (!isAuthHandledError(err)) this.feedError.set(httpErrorMessage(err, 'Failed to save feed')); + showHttpError(this.snack, err, 'Failed to save feed'); + }, }); } @@ -397,7 +418,7 @@ export class RssViewComponent implements OnInit { this.snack.open('Feed deleted', 'Close', { duration: 2000 }); this.loadAll(); }, - error: () => this.snack.open('Failed to delete feed', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to delete feed'), }); }); } @@ -407,6 +428,7 @@ export class RssViewComponent implements OnInit { showAddRule(): void { this.ruleForm = this.emptyRuleForm(); this.editingRuleId.set(null); + this.ruleError.set(null); this.ruleFormVisible.set(true); } @@ -420,12 +442,14 @@ export class RssViewComponent implements OnInit { feed_names_csv: r.feed_names.join(', '), }; this.editingRuleId.set(r.id); + this.ruleError.set(null); this.ruleFormVisible.set(true); } cancelRuleForm(): void { this.ruleFormVisible.set(false); this.editingRuleId.set(null); + this.ruleError.set(null); } saveRule(): void { @@ -455,7 +479,10 @@ export class RssViewComponent implements OnInit { this.cancelRuleForm(); this.loadAll(); }, - error: () => this.snack.open('Failed to save rule', 'Close', { duration: 3000 }), + error: (err) => { + if (!isAuthHandledError(err)) this.ruleError.set(httpErrorMessage(err, 'Failed to save rule')); + showHttpError(this.snack, err, 'Failed to save rule'); + }, }); } @@ -474,7 +501,7 @@ export class RssViewComponent implements OnInit { this.snack.open('Rule deleted', 'Close', { duration: 2000 }); this.loadAll(); }, - error: () => this.snack.open('Failed to delete rule', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to delete rule'), }); }); } @@ -487,7 +514,7 @@ export class RssViewComponent implements OnInit { this.snack.open('Added to queue', 'Close', { duration: 2000 }); this.loadAll(); }, - error: () => this.snack.open('Download failed', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Download failed'), }); } diff --git a/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.spec.ts b/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.spec.ts index 88a19e89..5ddc4a08 100644 --- a/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.spec.ts +++ b/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.spec.ts @@ -72,7 +72,7 @@ describe('SettingsViewComponent', () => { }); component.speedLimit = 999; component.saveSpeedLimit(); - expect(snack.open).toHaveBeenCalledWith('Failed to save speed limit', 'Close', { duration: 3000 }); + expect(snack.open).toHaveBeenCalledWith('Failed to save speed limit', 'Close', { duration: 5000 }); component.rotateSabApiKey(); expect(confirm.confirm).toHaveBeenCalledWith(expect.objectContaining({ title: 'Generate SABnzbd API key?' })); @@ -80,4 +80,21 @@ describe('SettingsViewComponent', () => { expect(component.sabApiKey).toBe('rotated'); expect(component.showSabApiKey).toBe(true); }); + + it('shows the server explanation when a settings save is rejected', () => { + const { component, snack } = makeComponent({ + put: vi.fn(() => + throwError(() => ({ + status: 400, + error: { error_kind: 'bad_request', human_readable: 'speed_limit_bps must be >= 0', status: 400 }, + })), + ), + }); + component.saveSpeedLimit(); + expect(snack.open).toHaveBeenCalledWith( + 'Failed to save speed limit: speed_limit_bps must be >= 0', + 'Close', + { duration: 5000 }, + ); + }); }); diff --git a/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.ts b/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.ts index 1f80e6d3..119f54d3 100644 --- a/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/settings/settings-view.component.ts @@ -8,6 +8,7 @@ import { StatusResponse } from '../../core/models/queue.model'; import { ConfirmService } from '../../shared/confirm.service'; import { IconComponent } from '../../shared/icon.component'; import { AppTheme, ThemeService } from '../../core/services/theme.service'; +import { showHttpError } from '../../core/http/http-error'; interface ServerConfig { id: string; @@ -1524,7 +1525,7 @@ export class SettingsViewComponent implements OnInit { this.cancelServerEdit(); this.loadServers(); }, - error: () => this.snack.open('Failed to update server', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to update server'), }); } else { server.id = ''; @@ -1534,7 +1535,7 @@ export class SettingsViewComponent implements OnInit { this.cancelServerEdit(); this.loadServers(); }, - error: () => this.snack.open('Failed to add server', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to add server'), }); } } @@ -1542,7 +1543,7 @@ export class SettingsViewComponent implements OnInit { testServer(id: string): void { this.api.post<{ success: boolean; message: string }>(`/config/servers/${id}/test`).subscribe({ next: (r) => this.snack.open(r.message, 'Close', { duration: 3000 }), - error: () => this.snack.open('Test failed', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Test failed'), }); } @@ -1559,7 +1560,7 @@ export class SettingsViewComponent implements OnInit { .post<{ success: boolean; message: string }>(`/config/servers/test-config`, body) .subscribe({ next: (r) => this.snack.open(r.message, 'Close', { duration: 4000 }), - error: () => this.snack.open('Test failed', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Test failed'), }); } @@ -1574,7 +1575,7 @@ export class SettingsViewComponent implements OnInit { duration: 2000, }); }, - error: () => this.snack.open('Failed to update server', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to update server'), }); } @@ -1593,7 +1594,7 @@ export class SettingsViewComponent implements OnInit { this.loadServers(); this.snack.open('Server removed', 'Close', { duration: 2000 }); }, - error: () => this.snack.open('Failed to delete server', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to delete server'), }); }); } @@ -1635,7 +1636,7 @@ export class SettingsViewComponent implements OnInit { this.cancelCategoryEdit(); this.loadCategories(); }, - error: () => this.snack.open('Failed to update category', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to update category'), }); } else { this.api.post('/config/categories', cat).subscribe({ @@ -1644,7 +1645,7 @@ export class SettingsViewComponent implements OnInit { this.cancelCategoryEdit(); this.loadCategories(); }, - error: () => this.snack.open('Failed to add category', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to add category'), }); } } @@ -1665,7 +1666,7 @@ export class SettingsViewComponent implements OnInit { this.loadCategories(); this.snack.open('Category removed', 'Close', { duration: 2000 }); }, - error: () => this.snack.open('Failed to delete category', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to delete category'), }); }); } @@ -1718,7 +1719,7 @@ export class SettingsViewComponent implements OnInit { loadSabApiKey(): void { this.api.get('/config/sabnzbd-api-key').subscribe({ next: (response) => (this.sabApiKey = response.api_key), - error: () => this.snack.open('Failed to load SABnzbd API key', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to load SABnzbd API key'), }); } @@ -1741,7 +1742,7 @@ export class SettingsViewComponent implements OnInit { this.showSabApiKey = true; this.snack.open(`SABnzbd API key ${action.toLowerCase()}d`, 'Close', { duration: 2500 }); }, - error: () => this.snack.open(`Failed to ${action.toLowerCase()} SABnzbd API key`, 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, `Failed to ${action.toLowerCase()} SABnzbd API key`), }); }); } @@ -1754,14 +1755,14 @@ export class SettingsViewComponent implements OnInit { }) .subscribe({ next: () => this.snack.open('Disk guards saved', 'Close', { duration: 2000 }), - error: () => this.snack.open('Failed to save disk guards', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to save disk guards'), }); } saveSpeedLimit(): void { this.api.put('/config/speed-limit', { speed_limit_bps: this.speedLimit }).subscribe({ next: () => this.snack.open('Speed limit saved', 'Close', { duration: 2000 }), - error: () => this.snack.open('Failed to save speed limit', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to save speed limit'), }); } @@ -1770,14 +1771,14 @@ export class SettingsViewComponent implements OnInit { .put('/config/max-active-downloads', { max_active_downloads: this.maxActiveDownloads }) .subscribe({ next: () => this.snack.open('Max downloads saved', 'Close', { duration: 2000 }), - error: () => this.snack.open('Failed to save max downloads', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to save max downloads'), }); } saveRetention(): void { this.api.put('/config/history-retention', { retention: this.historyRetention }).subscribe({ next: () => this.snack.open('History retention saved', 'Close', { duration: 2000 }), - error: () => this.snack.open('Failed to save retention', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to save retention'), }); } @@ -1825,8 +1826,7 @@ export class SettingsViewComponent implements OnInit { 'Close', { duration: 2500 }, ), - error: () => - this.snack.open('Failed to save Media Library settings', 'Close', { duration: 3000 }), + error: (err) => showHttpError(this.snack, err, 'Failed to save Media Library settings'), }); } diff --git a/apps/rustnzb/frontend/src/app/features/welcome/welcome.component.ts b/apps/rustnzb/frontend/src/app/features/welcome/welcome.component.ts index 780a98da..0d7a80d2 100644 --- a/apps/rustnzb/frontend/src/app/features/welcome/welcome.component.ts +++ b/apps/rustnzb/frontend/src/app/features/welcome/welcome.component.ts @@ -3,6 +3,7 @@ import { CommonModule } from '@angular/common'; import { FormsModule } from '@angular/forms'; import { Router } from '@angular/router'; import { ApiService } from '../../core/services/api.service'; +import { httpErrorDetail } from '../../core/http/http-error'; type Step = 'landing' | 'connect' | 'preview' | 'applying'; type ImportMethod = 'api' | 'ini'; @@ -468,7 +469,7 @@ export class WelcomeComponent implements OnInit { error: (err) => { this.fetching.set(false); this.connectError.set( - err.error?.message ?? err.error?.error ?? 'Failed to connect to SABnzbd. Check the URL and API key.' + httpErrorDetail(err) ?? 'Failed to connect to SABnzbd. Check the URL and API key.' ); }, }); @@ -492,7 +493,7 @@ export class WelcomeComponent implements OnInit { error: (err) => { this.fetching.set(false); this.connectError.set( - err.error?.message ?? err.error?.error ?? 'Failed to parse the ini file.' + httpErrorDetail(err) ?? 'Failed to parse the ini file.' ); }, }); @@ -535,7 +536,7 @@ export class WelcomeComponent implements OnInit { }, error: (err) => { this.applyError.set( - err.error?.message ?? err.error?.error ?? 'Failed to apply settings.' + httpErrorDetail(err) ?? 'Failed to apply settings.' ); this.step.set('preview'); }, From 6bbe3ced3893d9eda099a5203679aecefb0794f1 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:05:11 -0500 Subject: [PATCH 05/33] fix(web): make history retry idempotent per history entry (cherry picked from commit 333b99fb24c99668751e2d4660453a79519ab685) Adapted: kept both our QueueSortField re-export and the new HistoryRetryOutcome, and added the post_processing field our HistoryEntry requires to the new test fixture. --- apps/rustnzb/src/handlers.rs | 57 ++++----- .../rustnzb/tests/history_retry_idempotent.rs | 88 +++++++++++++ apps/rustnzb/tests/support/mod.rs | 5 +- crates/nzb-web/src/lib.rs | 4 +- crates/nzb-web/src/queue_manager.rs | 60 +++++++++ crates/nzb-web/src/sabnzbd_compat.rs | 117 +++++++++++++----- 6 files changed, 269 insertions(+), 62 deletions(-) create mode 100644 apps/rustnzb/tests/history_retry_idempotent.rs diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 81a7f753..c2f9edcc 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -28,6 +28,7 @@ use nzb_web::nzb_core::models::*; use nzb_web::nzb_core::nzb_parser; use nzb_web::nzb_core::sabnzbd_import; +use nzb_web::HistoryRetryOutcome; use nzb_web::error::ApiError; use nzb_web::fetch_guard::{ FetchPolicy, MAX_FETCH_BODY_BYTES, build_fetch_client, check_fetch_url_allowed, @@ -678,54 +679,48 @@ pub async fn h_history_clear( } /// POST /api/history/{id}/retry -- Re-add a failed/completed NZB from history. +/// +/// Idempotent per history entry: while a retry of `{id}` is still queued or +/// running, a repeat request enqueues nothing and returns `200` with the +/// existing job's id in `nzo_ids` (the same replay semantics as an +/// `Idempotency-Key` queue add). Once that job has left the queue the entry +/// can be retried again. pub async fn h_history_retry( State(state): State>, Path(id): Path, ) -> Result { - // Get the history entry to get the name/category let entry = state .queue_manager .history_get(&id) .map_err(ApiError::from)? .ok_or(ApiError::not_found("History entry not found"))?; - // Get the raw NZB data - let nzb_data = state + let job_id = match state .queue_manager - .history_get_nzb_data(&id) + .retry_history_entry(&entry) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("No NZB data stored for this entry")))?; - - let qm = &state.queue_manager; - let retry_data = qm.history_get_retry_data(&id).map_err(ApiError::from)?; - let job = qm - .prepare_retry_job(&entry, &nzb_data, retry_data.as_deref()) - .map_err(ApiError::from)?; - - std::fs::create_dir_all(&job.work_dir).map_err(|e| { - ApiError::from(anyhow::anyhow!( - "Failed to create work dir '{}': {}", - job.work_dir.display(), - e - )) - })?; - - let new_id = job.id.clone(); - - tracing::info!( - name = %job.name, - id = %new_id, - original_id = %id, - "Retrying NZB from history" - ); - - qm.add_job(job, Some(nzb_data)).map_err(ApiError::from)?; + { + HistoryRetryOutcome::Started(job_id) => job_id, + HistoryRetryOutcome::InProgress(job_id) => { + tracing::info!( + id = %job_id, + original_id = %id, + "History retry already in progress" + ); + job_id + } + HistoryRetryOutcome::NoNzbData => { + return Err(ApiError::from(anyhow::anyhow!( + "No NZB data stored for this entry" + ))); + } + }; Ok(( StatusCode::OK, Json(AddNzbResponse { status: true, - nzo_ids: vec![new_id], + nzo_ids: vec![job_id], }), )) } diff --git a/apps/rustnzb/tests/history_retry_idempotent.rs b/apps/rustnzb/tests/history_retry_idempotent.rs new file mode 100644 index 00000000..3c3acd0f --- /dev/null +++ b/apps/rustnzb/tests/history_retry_idempotent.rs @@ -0,0 +1,88 @@ +//! BUG-64: `POST /api/history/{id}/retry` is idempotent per history entry. + +mod support; + +use chrono::Utc; +use nzb_web::nzb_core::models::{HistoryEntry, JobFailureCode, JobStatus}; +use reqwest::StatusCode; +use support::{sample_nzb_bytes, start_test_server}; + +async fn setup_auth(client: &reqwest::Client, base_url: &str) -> String { + let setup = client + .post(format!("{base_url}/api/auth/setup")) + .json(&serde_json::json!({ + "username": "retry-test", + "password": "retry-test-password" + })) + .send() + .await + .expect("auth setup failed"); + assert_eq!(setup.status(), StatusCode::OK); + setup.json::().await.unwrap()["access_token"] + .as_str() + .expect("auth setup should return an access token") + .to_string() +} + +fn failed_history_entry(id: &str, complete_dir: &std::path::Path) -> HistoryEntry { + HistoryEntry { + id: id.into(), + name: "Retry Me".into(), + category: "Default".into(), + status: JobStatus::Failed, + total_bytes: 10, + downloaded_bytes: 5, + added_at: Utc::now(), + completed_at: Utc::now(), + download_time_secs: None, + output_dir: complete_dir.join("Retry Me"), + stages: Vec::new(), + error_message: Some("missing articles".into()), + failure_code: Some(JobFailureCode::ArticlesUnavailable), + post_processing: None, + server_stats: Vec::new(), + nzb_data: Some(sample_nzb_bytes()), + retry_data: None, + } +} + +#[tokio::test] +async fn concurrent_history_retries_enqueue_one_job() { + let app = start_test_server(Vec::new()).await; + let client = reqwest::Client::new(); + let access = setup_auth(&client, &app.base_url).await; + client + .post(format!("{}/api/queue/pause", app.base_url)) + .bearer_auth(&access) + .send() + .await + .unwrap() + .error_for_status() + .unwrap(); + let entry = failed_history_entry("failed-history-entry", &app.complete_dir); + app.state + .queue_manager + .with_db(|db| db.history_insert(&entry).expect("insert history")); + + let url = format!("{}/api/history/failed-history-entry/retry", app.base_url); + let mut requests = tokio::task::JoinSet::new(); + for _ in 0..5 { + requests.spawn(client.post(&url).bearer_auth(&access).send()); + } + + let mut job_ids = Vec::new(); + while let Some(response) = requests.join_next().await { + let response = response.unwrap().unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let body = response.json::().await.unwrap(); + assert_eq!(body["status"], true, "{body}"); + job_ids.push(body["nzo_ids"][0].as_str().unwrap().to_string()); + } + + let jobs = app.state.queue_manager.get_jobs(); + assert_eq!(jobs.len(), 1, "five retries must enqueue one job"); + assert!( + job_ids.iter().all(|id| id == &jobs[0].id), + "every caller gets the one retried job id: {job_ids:?}" + ); +} diff --git a/apps/rustnzb/tests/support/mod.rs b/apps/rustnzb/tests/support/mod.rs index f3f8b6ce..312aac60 100644 --- a/apps/rustnzb/tests/support/mod.rs +++ b/apps/rustnzb/tests/support/mod.rs @@ -26,6 +26,8 @@ fn workspace_path(path: &str) -> PathBuf { pub struct TestApp { pub base_url: String, pub complete_dir: PathBuf, + /// Shared app state behind the router, for seeding fixtures directly. + pub state: Arc, _tmp_dir: TempDir, handle: JoinHandle<()>, } @@ -123,7 +125,7 @@ async fn start_test_server_inner( credential_store, )); - let router = build_router(state); + let router = build_router(state.clone()); #[cfg(feature = "webdav")] let router = router.layer(axum::Extension(None::>)); let listener = tokio::net::TcpListener::bind("127.0.0.1:0") @@ -141,6 +143,7 @@ async fn start_test_server_inner( TestApp { base_url, complete_dir, + state, _tmp_dir: tmp_dir, handle, } diff --git a/crates/nzb-web/src/lib.rs b/crates/nzb-web/src/lib.rs index 9b9241a9..9adec749 100644 --- a/crates/nzb-web/src/lib.rs +++ b/crates/nzb-web/src/lib.rs @@ -21,8 +21,8 @@ pub mod util; pub use log_buffer::{LogBuffer, LogBufferLayer}; pub use nzb_dispatch::{ArticleFailure, ArticleFailureKind}; pub use queue_manager::{ - DailyStatisticsData, GlobalStatisticsData, QueueManager, QueueSortField, ServerStatsData, - StatisticsPeriodData, + DailyStatisticsData, GlobalStatisticsData, HistoryRetryOutcome, QueueManager, QueueSortField, + ServerStatsData, StatisticsPeriodData, }; pub use startup::{StartupConfig, StartupResult}; pub use state::AppState; diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index fcabde3e..94697ed5 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -1154,6 +1154,18 @@ pub struct JobAddedEvent { // QueueManager // --------------------------------------------------------------------------- +/// Result of [`QueueManager::retry_history_entry`]. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum HistoryRetryOutcome { + /// A new job was enqueued; carries its id. + Started(String), + /// A retry of this entry is already queued or running; carries that + /// job's id. Nothing new was enqueued. + InProgress(String), + /// The history entry has no stored NZB to retry from. + NoNzbData, +} + /// Thread-safe queue manager that coordinates all downloads. /// /// Wrapped in `Arc` for sharing between the background task and HTTP handlers. @@ -1194,6 +1206,10 @@ pub struct QueueManager { add_tx: broadcast::Sender, /// Max concurrent active downloads (0 = unlimited). max_active_downloads: AtomicUsize, + /// History retries in flight: history entry id -> id of the job the + /// retry enqueued. Held from the in-flight check through the enqueue so + /// concurrent retries of one entry admit exactly one job. + history_retries: Mutex>, /// Automatically keep the queue ordered by remaining percentage. auto_sort_remaining_pct: AtomicBool, /// Optional bounded post-processing hooks. @@ -1319,6 +1335,7 @@ impl QueueManager { log_buffer: Some(log_buffer), add_tx, max_active_downloads: AtomicUsize::new(max_active_downloads), + history_retries: Mutex::new(HashMap::new()), auto_sort_remaining_pct: AtomicBool::new(false), postproc_scripts: Mutex::new(PostProcScriptConfig::default()), postproc_resources: PostProcResourcePool::new(postproc_limits), @@ -1851,6 +1868,49 @@ impl QueueManager { self.start_next_queued(); } + /// Retry one history entry at most once at a time. + /// + /// The first caller claims the entry, rebuilds the job (reusing a + /// retained partial work directory via [`Self::prepare_retry_job`]) and + /// enqueues it. While that job is still in the queue -- queued, paused, + /// downloading or post-processing -- any further retry of the same entry + /// returns [`HistoryRetryOutcome::InProgress`] with the existing job id + /// instead of enqueueing a duplicate. Once the job has left the queue the + /// entry can be retried again. + /// + /// This is an in-flight guard, not the durable `Idempotency-Key` + /// admission: a history entry stays retryable after its retry finishes, + /// whereas an admission key binds exactly one job forever. + pub fn retry_history_entry( + self: &Arc, + entry: &HistoryEntry, + ) -> crate::nzb_core::Result { + let mut retries = self.history_retries.lock(); + { + let jobs = self.jobs.lock(); + retries.retain(|_, job_id| jobs.contains_key(job_id)); + if let Some(job_id) = retries.get(&entry.id) { + return Ok(HistoryRetryOutcome::InProgress(job_id.clone())); + } + } + + let Some(nzb_data) = self.history_get_nzb_data(&entry.id)? else { + return Ok(HistoryRetryOutcome::NoNzbData); + }; + let retry_data = self.history_get_retry_data(&entry.id)?; + let job = self.prepare_retry_job(entry, &nzb_data, retry_data.as_deref())?; + let job_id = job.id.clone(); + info!( + name = %job.name, + id = %job_id, + original_id = %entry.id, + "Retrying NZB from history" + ); + self.add_job(job, Some(nzb_data))?; + retries.insert(entry.id.clone(), job_id.clone()); + Ok(HistoryRetryOutcome::Started(job_id)) + } + /// Rebuild a history job for retry. Newer history rows carry a checkpoint /// and retain a partial work directory when at least one article was /// written, so the dispatcher can enqueue only unresolved articles and diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index 84bc3619..67006ade 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -16,7 +16,7 @@ use crate::nzb_core::models::*; use crate::nzb_core::nzb_parser; use crate::error::ApiError; -use crate::queue_manager::job_id_matches; +use crate::queue_manager::{HistoryRetryOutcome, job_id_matches}; use crate::state::AppState; /// SABnzbd release whose public response contract this compatibility layer @@ -2051,9 +2051,17 @@ fn handle_retry(state: &AppState, req: &SabApiRequest) -> Json data, - Ok(None) => { + // At most one retry of an entry is in flight: a repeat while it is still + // queued or running returns the existing job's nzo_id (status true) + // rather than enqueueing a duplicate. + let job_id = match state.queue_manager.retry_history_entry(&entry) { + Ok(HistoryRetryOutcome::Started(job_id)) => job_id, + Ok(HistoryRetryOutcome::InProgress(job_id)) => { + let nzo_id = format!("SABnzbd_nzo_{}", sab_id_prefix(&job_id)); + tracing::info!(history_id = %entry.id, retried_id = %nzo_id, "History job retry already in progress"); + return Json(serde_json::json!({ "status": true, "nzo_ids": [nzo_id] })); + } + Ok(HistoryRetryOutcome::NoNzbData) => { return Json(serde_json::json!({ "status": false, "error": "The original NZB data is unavailable for this history job" @@ -2063,30 +2071,7 @@ fn handle_retry(state: &AppState, req: &SabApiRequest) -> Json data, - Err(error) => { - return Json(serde_json::json!({ "status": false, "error": error.to_string() })); - } - }; - let job = match state - .queue_manager - .prepare_retry_job(&entry, &data, retry_data.as_deref()) - { - Ok(job) => job, - Err(error) => { - return Json(serde_json::json!({ - "status": false, - "error": format!("Failed to parse stored NZB: {error}") - })); - } - }; - - let nzo_id = format!("SABnzbd_nzo_{}", sab_id_prefix(&job.id)); - if let Err(error) = state.queue_manager.add_job(job, Some(data)) { - return Json(serde_json::json!({ "status": false, "error": error.to_string() })); - } + let nzo_id = format!("SABnzbd_nzo_{}", sab_id_prefix(&job_id)); tracing::info!(history_id = %entry.id, retried_id = %nzo_id, "History job retried via arr API"); Json(serde_json::json!({ "status": true, "nzo_ids": [nzo_id] })) @@ -4743,6 +4728,82 @@ mod tests { assert!(!history_ids(&test_state).contains(&"oldest-failed".to_string())); } + const RETRY_NZB: &[u8] = br#" + + + alt.binaries.test + retry-1@test + +"#; + + fn insert_retryable_history(test_state: &TestState, id: &str) { + let mut entry = history_entry(id, id, "tv", JobStatus::Failed, 1); + entry.output_dir = test_state.state.config().general.complete_dir.join(id); + entry.nzb_data = Some(RETRY_NZB.to_vec()); + test_state + .state + .queue_manager + .with_db(|database| database.history_insert(&entry).expect("insert history")); + } + + /// BUG-64: concurrent `mode=retry` calls for one history entry must + /// enqueue exactly one job; every caller gets that job's nzo_id. + #[tokio::test] + async fn concurrent_retries_of_one_history_entry_enqueue_one_job() { + let test_state = test_state(); + insert_retryable_history(&test_state, "failed-once"); + let barrier = std::sync::Barrier::new(5); + let runtime = tokio::runtime::Handle::current(); + let responses: Vec = std::thread::scope(|scope| { + let handles: Vec<_> = (0..5) + .map(|_| { + scope.spawn(|| { + let _runtime = runtime.enter(); + barrier.wait(); + handle_retry( + &test_state.state, + &SabApiRequest { + value: Some("failed-once".into()), + ..SabApiRequest::default() + }, + ) + .0 + }) + }) + .collect(); + handles.into_iter().map(|h| h.join().unwrap()).collect() + }); + + assert_eq!(test_state.state.queue_manager.get_jobs().len(), 1); + let first = &responses[0]["nzo_ids"][0]; + for response in &responses { + assert_eq!(response["status"], serde_json::json!(true), "{response}"); + assert_eq!(&response["nzo_ids"][0], first, "{response}"); + } + } + + /// Once the retried job has left the queue, the history entry can be + /// retried again. + #[tokio::test] + async fn retry_is_allowed_again_after_the_retried_job_leaves_the_queue() { + let test_state = test_state(); + insert_retryable_history(&test_state, "failed-twice"); + let req = SabApiRequest { + value: Some("failed-twice".into()), + ..SabApiRequest::default() + }; + let qm = &test_state.state.queue_manager; + let first = handle_retry(&test_state.state, &req).0; + assert_eq!(first["status"], serde_json::json!(true), "{first}"); + let job_id = qm.get_jobs()[0].id.clone(); + qm.remove_job(&job_id).expect("remove retried job"); + + let second = handle_retry(&test_state.state, &req).0; + assert_eq!(second["status"], serde_json::json!(true), "{second}"); + assert_ne!(second["nzo_ids"][0], first["nzo_ids"][0]); + assert_eq!(qm.get_jobs().len(), 1); + } + /// Without `del_files`, history delete only removes the DB record, as /// before. #[tokio::test] From 5beb82eee0cfe515f57d0b06b582df7da29254ac Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:05:11 -0500 Subject: [PATCH 06/33] docs(config): document max_active_downloads = 0 as unlimited The queue manager already treats 0 as no limit on simultaneous downloads, but neither the config field doc nor config.example.toml said so. Documentation only; no behaviour change. Co-Authored-By: Claude Opus 5.5 (cherry picked from commit b9d9024585f756b3e5a1cc7f3b63e92a7b72b58d) --- apps/rustnzb/config.example.toml | 1 + crates/nzb-core/src/config.rs | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/apps/rustnzb/config.example.toml b/apps/rustnzb/config.example.toml index 1c2f4791..37c806eb 100644 --- a/apps/rustnzb/config.example.toml +++ b/apps/rustnzb/config.example.toml @@ -10,6 +10,7 @@ cache_size = 524288000 # 500 MB log_level = "info" # log_file = "data/rustnzb.log" # history_retention = 100 # Number of NZBs to keep in history (omit or 0 = keep all) +max_active_downloads = 1 # NZBs downloading simultaneously (0 = unlimited) # auto_sort_remaining_pct = false # Keep queued items ordered by remaining work # rss_downloaded_item_expiry_days = 30 # Remove downloaded RSS records after this many days # Independent post-processing jobs overlap downloads. Separate repair and diff --git a/crates/nzb-core/src/config.rs b/crates/nzb-core/src/config.rs index d2dd1ab2..27d64335 100644 --- a/crates/nzb-core/src/config.rs +++ b/crates/nzb-core/src/config.rs @@ -57,7 +57,8 @@ pub struct GeneralConfig { /// History retention: how many NZBs to keep in history. /// `None` or `Some(0)` both mean keep all; see [`normalize_history_retention`]. pub history_retention: Option, - /// Max number of NZBs downloading simultaneously (default 1) + /// Max number of NZBs downloading simultaneously (default 1). + /// `0` means unlimited: every queued job may download at once. pub max_active_downloads: usize, /// Max number of independent jobs in post-processing at once. #[serde(default = "default_max_post_processing_jobs")] From 302b20e300df74b92a1ffc1ebbe5dbe36f7621ac Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:36:23 -0500 Subject: [PATCH 07/33] fix(web): wake the RSS monitor on config changes and poll feeds per interval RssMonitor::run checked every enabled feed, then slept for the smallest enabled poll interval, or 900 s when there were no feeds. Nothing interrupted that sleep. A feed added after a feed-less boot therefore waited up to 15 minutes for its first check, whatever its own interval. A shortened interval also waited out the old sleep. AppState now owns a Notify (rss_monitor_wake). commit_config signals it after every committed config change, which covers every handler that goes through update_config / update_config_with. Startup builds the state first and hands the signal to the monitor via RssMonitor::with_wake. The loop now tracks when each feed was last checked (FeedSchedule, keyed by name and URL). It checks only the feeds that are due, then sleeps until the earliest due time, the next 900 s maintenance pass, or a config-change wake, whichever comes first. The due time is computed from the feed's current poll_interval_secs on every pass, so an interval edit applies immediately. A new, renamed, re-enabled or re-pointed feed is due at once. A deleted or disabled feed is dropped. Each feed now runs on its own interval instead of the shortest one. Item pruning and expiry still run at least every 900 s. RSS rules live in the database and are re-read on every feed check. They do not affect when a feed is due, so rule edits do not wake the monitor. The config is held as a full Arc, not an arc-swap guard, across the feed-check awaits. No lock is held across an await. Adds tokio test-util as a dev-dependency feature. Bumps nzb-web to 0.4.24 for the additive public API (AppState::rss_monitor_wake, RssMonitor::with_wake). Fixes #107 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 3884543556b5b2385faf57c000c3d552b6cf2087) --- Cargo.lock | 2 +- Cargo.toml | 2 +- crates/nzb-web/Cargo.toml | 3 +- crates/nzb-web/src/rss_monitor.rs | 377 +++++++++++++++++++++++++++--- crates/nzb-web/src/startup.rs | 23 +- crates/nzb-web/src/state.rs | 6 + 6 files changed, 365 insertions(+), 48 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d5cfac32..43274716 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1894,7 +1894,7 @@ dependencies = [ [[package]] name = "nzb-web" -version = "0.4.23" +version = "0.4.24" dependencies = [ "anyhow", "arc-swap", diff --git a/Cargo.toml b/Cargo.toml index 419599e3..5582f1fc 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -88,7 +88,7 @@ feed-rs = "2" unicode-normalization = "0.1" # Shared NZB crates -nzb-web = { version = "0.4.23", path = "crates/nzb-web", features = ["groups-db"] } +nzb-web = { version = "0.4.24", path = "crates/nzb-web", features = ["groups-db"] } nzb-nntp = { version = "0.2.24", path = "crates/nzb-nntp" } nzb-core = { version = "0.2.18", path = "crates/nzb-core", features = ["groups-db"] } nzb-decode = { version = "0.1.5", path = "crates/nzb-decode" } diff --git a/crates/nzb-web/Cargo.toml b/crates/nzb-web/Cargo.toml index b71e168f..d67a66bb 100644 --- a/crates/nzb-web/Cargo.toml +++ b/crates/nzb-web/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nzb-web" -version = "0.4.23" +version = "0.4.24" edition = "2024" description = "Usenet download engine: queue management, download orchestration, and background services" license = "MIT" @@ -56,6 +56,7 @@ all = { level = "warn", priority = -1 } unused = "warn" [dev-dependencies] +tokio = { version = "1", features = ["full", "test-util"] } tempfile = "3.27.0" nzb-nntp = { version = "0.2.24", path = "../nzb-nntp", features = ["test-support"] } yenc-simd = { version = "0.1" } diff --git a/crates/nzb-web/src/rss_monitor.rs b/crates/nzb-web/src/rss_monitor.rs index aa0b81bf..31b12278 100644 --- a/crates/nzb-web/src/rss_monitor.rs +++ b/crates/nzb-web/src/rss_monitor.rs @@ -1,9 +1,11 @@ -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::sync::Arc; use arc_swap::ArcSwap; use chrono::Utc; +use tokio::sync::Notify; +use tokio::time::{Duration, Instant}; use tracing::{info, warn}; use crate::fetch_guard::{ @@ -22,6 +24,9 @@ pub struct RssMonitor { config: Arc>, queue_manager: Arc, data_dir: PathBuf, + /// Woken whenever the config changes, so feed additions, edits and + /// deletions are picked up without waiting for the current sleep. + wake: Arc, } impl RssMonitor { @@ -34,9 +39,17 @@ impl RssMonitor { config, queue_manager, data_dir, + wake: Arc::new(Notify::new()), } } + /// Use `wake` as the config-change signal (see + /// [`crate::state::AppState::rss_monitor_wake`]). + pub fn with_wake(mut self, wake: Arc) -> Self { + self.wake = wake; + self + } + /// Migrate legacy rss_seen.json entries into the database on first run. fn migrate_seen_json(&self) { let seen_file = self.data_dir.join("rss_seen.json"); @@ -80,58 +93,75 @@ impl RssMonitor { info!("Legacy rss_seen.json migrated and removed"); } - /// Run the monitor loop forever, polling feeds at their configured intervals. - /// Reads feed config from the shared ArcSwap on each iteration so that - /// feeds added/removed/toggled via the API take effect without a restart. + /// Run the monitor loop forever, polling each feed at its own configured + /// interval. Feed config is re-read from the shared ArcSwap on every pass, + /// and the loop sleeps only until the next feed is due or the config + /// changes (see [`RssMonitor::with_wake`]), so feeds added, edited, + /// re-enabled or removed via the API take effect at once. pub async fn run(self) { info!("RSS monitor started"); // Migrate legacy seen file on first run self.migrate_seen_json(); - loop { - let cfg = self.config.load(); - let feeds = &cfg.rss_feeds; + let mut schedule = FeedSchedule::default(); + let mut next_maintenance = Instant::now(); - for feed in feeds { - if !feed.enabled { - continue; - } + loop { + // A full Arc rather than an arc-swap guard: it is held across the + // feed checks below. + let cfg = self.config.load_full(); + schedule.sync(&cfg.rss_feeds); + + let now = Instant::now(); + let due: Vec<&RssFeedConfig> = cfg + .rss_feeds + .iter() + .filter(|feed| feed.enabled && schedule.is_due(feed, now)) + .collect(); + for feed in &due { if let Err(e) = self.check_feed(feed).await { warn!(feed = %feed.name, error = %e, "RSS feed check failed"); } + schedule.mark_checked(feed, Instant::now()); } - // Prune old items based on config - let limit = cfg.general.rss_history_limit.unwrap_or(500); - if let Ok(count) = self.queue_manager.rss_item_count() - && count > limit - && let Ok(pruned) = self.queue_manager.rss_items_prune(limit) - && pruned > 0 - { - info!(pruned, "Pruned old RSS items"); + if !due.is_empty() || Instant::now() >= next_maintenance { + self.prune_items(&cfg); + next_maintenance = Instant::now() + IDLE_INTERVAL; } - if let Some(days) = cfg.general.rss_downloaded_item_expiry_days { - let cutoff = (Utc::now() - chrono::Duration::days(days as i64)).to_rfc3339(); - if let Ok(expired) = self.queue_manager.rss_items_expire_downloaded(&cutoff) - && expired > 0 - { - info!(expired, "Expired downloaded RSS items"); - } + let wake_at = schedule + .next_due(&cfg.rss_feeds) + .map_or(next_maintenance, |due| due.min(next_maintenance)); + drop(cfg); + + tokio::select! { + _ = tokio::time::sleep_until(wake_at) => {} + _ = self.wake.notified() => {} } + } + } - // Use the minimum poll interval across all enabled feeds, defaulting to 15 min - let interval = feeds - .iter() - .filter(|f| f.enabled) - .map(|f| f.poll_interval_secs) - .min() - .unwrap_or(900); + /// Trim the stored RSS item history to the configured limits. + fn prune_items(&self, cfg: &AppConfig) { + let limit = cfg.general.rss_history_limit.unwrap_or(500); + if let Ok(count) = self.queue_manager.rss_item_count() + && count > limit + && let Ok(pruned) = self.queue_manager.rss_items_prune(limit) + && pruned > 0 + { + info!(pruned, "Pruned old RSS items"); + } - drop(cfg); - tokio::time::sleep(tokio::time::Duration::from_secs(interval)).await; + if let Some(days) = cfg.general.rss_downloaded_item_expiry_days { + let cutoff = (Utc::now() - chrono::Duration::days(days as i64)).to_rfc3339(); + if let Ok(expired) = self.queue_manager.rss_items_expire_downloaded(&cutoff) + && expired > 0 + { + info!(expired, "Expired downloaded RSS items"); + } } } @@ -405,6 +435,61 @@ impl RssMonitor { } } +/// How long the monitor sleeps when no feed is due sooner; also the cadence +/// of RSS item history maintenance. +const IDLE_INTERVAL: Duration = Duration::from_secs(900); + +/// When each enabled feed was last checked, keyed by feed name. +/// +/// A feed's next due time is derived from its *current* `poll_interval_secs` +/// on every pass, so an interval edit takes effect immediately. Feeds that are +/// unknown (newly added, renamed, re-enabled or re-pointed at a new URL) are +/// due at once; deleted or disabled feeds are forgotten. +#[derive(Debug, Default)] +struct FeedSchedule { + last_checked: HashMap, +} + +impl FeedSchedule { + /// Drop entries for feeds that are gone, disabled, or now point elsewhere. + fn sync(&mut self, feeds: &[RssFeedConfig]) { + self.last_checked.retain(|name, (url, _)| { + feeds + .iter() + .any(|f| f.enabled && &f.name == name && &f.url == url) + }); + } + + fn due_at(&self, feed: &RssFeedConfig) -> Option { + self.last_checked.get(&feed.name).map(|(_, at)| { + // An absurd interval must not overflow `Instant`; a year out + // is effectively "never" for a poller. + at.checked_add(Duration::from_secs(feed.poll_interval_secs)) + .unwrap_or(*at + Duration::from_secs(365 * 86_400)) + }) + } + + fn is_due(&self, feed: &RssFeedConfig, now: Instant) -> bool { + self.due_at(feed).is_none_or(|due| due <= now) + } + + fn mark_checked(&mut self, feed: &RssFeedConfig, at: Instant) { + self.last_checked + .insert(feed.name.clone(), (feed.url.clone(), at)); + } + + /// Earliest due time across enabled feeds; `None` when there are none. + /// A never-checked feed counts as due now. + fn next_due(&self, feeds: &[RssFeedConfig]) -> Option { + let now = Instant::now(); + feeds + .iter() + .filter(|f| f.enabled) + .map(|f| self.due_at(f).unwrap_or(now)) + .min() + } +} + #[cfg(test)] mod tests { use super::*; @@ -508,4 +593,226 @@ mod tests { .expect_err("local addresses must be rejected"); assert!(error.to_string().contains("private/reserved")); } + + const EMPTY_RSS: &str = + r#"t"#; + + /// Serve `EMPTY_RSS` on loopback and report the (tokio, so possibly + /// paused) time of every request that arrives. + async fn feed_server() -> ( + std::net::SocketAddr, + tokio::sync::mpsc::UnboundedReceiver, + ) { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind feed server"); + let addr = listener.local_addr().expect("feed server addr"); + let (tx, rx) = tokio::sync::mpsc::unbounded_channel(); + tokio::spawn(async move { + while let Ok((mut socket, _)) = listener.accept().await { + let mut request = Vec::new(); + let mut buf = [0u8; 1024]; + while !request.windows(4).any(|w| w == b"\r\n\r\n") { + match socket.read(&mut buf).await { + Ok(0) | Err(_) => break, + Ok(n) => request.extend_from_slice(&buf[..n]), + } + } + let _ = tx.send(tokio::time::Instant::now()); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/rss+xml\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{}", + EMPTY_RSS.len(), + EMPTY_RSS + ); + let _ = socket.write_all(response.as_bytes()).await; + let _ = socket.shutdown().await; + } + }); + (addr, rx) + } + + fn feed(name: &str, addr: std::net::SocketAddr, poll_interval_secs: u64) -> RssFeedConfig { + RssFeedConfig { + name: name.into(), + url: format!("http://{addr}/{name}.xml"), + poll_interval_secs, + category: None, + filter_regex: None, + enabled: true, + auto_download: false, + max_age_days: None, + } + } + + /// BUG-66: with no feeds at boot the monitor sleeps for the 900 s idle + /// interval, and a feed added through the API used to wait out that whole + /// sleep before its first check. Saving the config must wake the monitor. + /// Runs on a paused clock, so the 900 s sleep costs no wall-clock time and + /// the assertion is on virtual time. + #[tokio::test(start_paused = true)] + async fn feed_config_changes_take_effect_without_waiting_for_the_current_sleep() { + let temp = tempfile::tempdir().expect("tempdir"); + let (addr, mut requests) = feed_server().await; + + let mut config = AppConfig::default(); + config.general.fetch_allowed_hosts = vec!["127.0.0.1".into()]; + let (monitor, queue_manager) = monitor(temp.path().to_path_buf()); + let shared = Arc::clone(&monitor.config); + shared.store(Arc::new(config)); + let state = crate::state::AppState::new( + Arc::clone(&shared), + temp.path().join("config.toml"), + queue_manager, + LogBuffer::default(), + Arc::new(crate::auth::TokenStore::new()), + Arc::new(crate::auth::CredentialStore::new(temp.path().to_path_buf())), + ); + let monitor = monitor.with_wake(Arc::clone(&state.rss_monitor_wake)); + let task = tokio::spawn(monitor.run()); + + // Let the monitor run its empty first pass and go to sleep. + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + + let added_at = tokio::time::Instant::now(); + state + .update_config_with(|config| { + config.rss_feeds.push(feed("late", addr, 600)); + Ok::<_, anyhow::Error>(()) + }) + .expect("add feed"); + + let fetched_at = requests.recv().await.expect("feed was fetched"); + let waited = fetched_at - added_at; + assert!( + waited < std::time::Duration::from_secs(5), + "new feed waited {waited:?} for its first check" + ); + + // Shortening the interval takes effect without waiting out the old 600 s. + state + .update_config_with(|config| { + config.rss_feeds[0].poll_interval_secs = 30; + Ok::<_, anyhow::Error>(()) + }) + .expect("edit feed"); + let refetched_at = requests.recv().await.expect("feed was re-fetched"); + let gap = refetched_at - fetched_at; + assert!( + gap < std::time::Duration::from_secs(60), + "edited interval took {gap:?} to apply" + ); + + // A deleted feed is never fetched again. + state + .update_config_with(|config| { + config.rss_feeds.clear(); + Ok::<_, anyhow::Error>(()) + }) + .expect("delete feed"); + tokio::time::sleep(std::time::Duration::from_secs(3600)).await; + while let Ok(at) = requests.try_recv() { + assert!( + at <= refetched_at + std::time::Duration::from_secs(1), + "deleted feed was fetched again after {:?}", + at - refetched_at + ); + } + task.abort(); + } + + fn schedule_feed(name: &str, url: &str, poll_interval_secs: u64) -> RssFeedConfig { + RssFeedConfig { + name: name.into(), + url: url.into(), + poll_interval_secs, + category: None, + filter_regex: None, + enabled: true, + auto_download: false, + max_age_days: None, + } + } + + #[test] + fn schedule_checks_new_feeds_at_once_and_then_per_feed_interval() { + let t0 = Instant::now(); + let fast = schedule_feed("fast", "https://example.test/fast", 60); + let slow = schedule_feed("slow", "https://example.test/slow", 3600); + let feeds = vec![fast.clone(), slow.clone()]; + let mut schedule = FeedSchedule::default(); + + schedule.sync(&feeds); + assert!(schedule.is_due(&fast, t0) && schedule.is_due(&slow, t0)); + + schedule.mark_checked(&fast, t0); + schedule.mark_checked(&slow, t0); + let at_61 = t0 + Duration::from_secs(61); + assert!(schedule.is_due(&fast, at_61)); + assert!( + !schedule.is_due(&slow, at_61), + "slow feed is not over-polled" + ); + assert_eq!( + schedule.next_due(&feeds), + Some(t0 + Duration::from_secs(60)) + ); + } + + #[test] + fn schedule_applies_interval_edits_immediately() { + let t0 = Instant::now(); + let mut feed = schedule_feed("f", "https://example.test/f", 900); + let mut schedule = FeedSchedule::default(); + schedule.mark_checked(&feed, t0); + assert_eq!(schedule.due_at(&feed), Some(t0 + Duration::from_secs(900))); + + feed.poll_interval_secs = 15; + schedule.sync(std::slice::from_ref(&feed)); + assert_eq!(schedule.due_at(&feed), Some(t0 + Duration::from_secs(15))); + + feed.poll_interval_secs = u64::MAX; + assert!( + schedule.due_at(&feed).is_some(), + "huge interval must not overflow" + ); + } + + #[test] + fn schedule_forgets_deleted_disabled_and_repointed_feeds() { + let t0 = Instant::now(); + let kept = schedule_feed("kept", "https://example.test/kept", 900); + let deleted = schedule_feed("deleted", "https://example.test/deleted", 900); + let mut toggled = schedule_feed("toggled", "https://example.test/toggled", 900); + let mut moved = schedule_feed("moved", "https://example.test/old", 900); + let mut schedule = FeedSchedule::default(); + for feed in [&kept, &deleted, &toggled, &moved] { + schedule.mark_checked(feed, t0); + } + + toggled.enabled = false; + moved.url = "https://example.test/new".into(); + let feeds = vec![kept.clone(), toggled.clone(), moved.clone()]; + schedule.sync(&feeds); + + assert!(!schedule.is_due(&kept, t0)); + assert!(!schedule.last_checked.contains_key("deleted")); + assert!(!schedule.last_checked.contains_key("toggled")); + assert!( + schedule.is_due(&moved, t0), + "a re-pointed feed is checked at once" + ); + assert_eq!( + schedule.next_due(&[toggled.clone()]), + None, + "disabled feeds never wake the loop" + ); + + toggled.enabled = true; + schedule.sync(std::slice::from_ref(&toggled)); + assert!( + schedule.is_due(&toggled, t0), + "a re-enabled feed is checked at once" + ); + } } diff --git a/crates/nzb-web/src/startup.rs b/crates/nzb-web/src/startup.rs index 08ae0abf..e6e0283f 100644 --- a/crates/nzb-web/src/startup.rs +++ b/crates/nzb-web/src/startup.rs @@ -216,18 +216,9 @@ pub async fn initialize( // without a restart. let shared_config = Arc::new(ArcSwap::new(Arc::new(config))); - // Always start the RSS monitor so feeds added later via the API are polled. - let data_dir_for_rss = shared_config.load().general.data_dir.clone(); - let monitor = crate::rss_monitor::RssMonitor::new( - Arc::clone(&shared_config), - Arc::clone(&queue_manager), - data_dir_for_rss, - ); - tokio::spawn(async move { monitor.run().await }); - // Build shared application state let state = Arc::new(AppState::new( - shared_config, + Arc::clone(&shared_config), config_path, Arc::clone(&queue_manager), log_buffer.clone(), @@ -235,6 +226,18 @@ pub async fn initialize( credential_store, )); + // Always start the RSS monitor so feeds added later via the API are polled. + // Every committed config change wakes it, so a new or re-enabled feed is + // checked promptly instead of after the monitor's current sleep. + let data_dir_for_rss = shared_config.load().general.data_dir.clone(); + let monitor = crate::rss_monitor::RssMonitor::new( + shared_config, + Arc::clone(&queue_manager), + data_dir_for_rss, + ) + .with_wake(Arc::clone(&state.rss_monitor_wake)); + tokio::spawn(async move { monitor.run().await }); + Ok(StartupResult { state, queue_manager, diff --git a/crates/nzb-web/src/state.rs b/crates/nzb-web/src/state.rs index 02a5dee2..f3543035 100644 --- a/crates/nzb-web/src/state.rs +++ b/crates/nzb-web/src/state.rs @@ -23,6 +23,10 @@ pub struct AppState { /// Serialises config writers so a read-modify-write cycle (including /// persisting the TOML) cannot interleave with another one. config_write: Mutex<()>, + /// Signalled after every committed config change so the RSS monitor + /// re-reads its feeds at once instead of finishing its current sleep. + /// Single consumer: `notify_one` stores a permit if the monitor is busy. + pub rss_monitor_wake: Arc, } impl AppState { @@ -43,6 +47,7 @@ impl AppState { credential_store, started_at: std::time::Instant::now(), config_write: Mutex::new(()), + rss_monitor_wake: Arc::new(tokio::sync::Notify::new()), } } @@ -84,6 +89,7 @@ impl AppState { fn commit_config(&self, config: AppConfig) -> anyhow::Result<()> { config.save(&self.config_path)?; self.config.store(Arc::new(config)); + self.rss_monitor_wake.notify_one(); Ok(()) } } From e86b3e8a7380946880eb483d219d99c2a6a564b6 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 08:47:20 -0500 Subject: [PATCH 08/33] fix(web): enforce a 60 s floor on RSS feed poll intervals Nothing validated poll_interval_secs. A feed saved with 0, through the API, the UI or a hand-edited config.toml, was due again as soon as its check finished. The RSS monitor then polled the indexer in a tight loop, burning CPU and risking a rate-limit ban. - The feed add and update handlers now return 400 for poll_interval_secs below MIN_POLL_INTERVAL_SECS (60), before the URL check and before anything is written. - The monitor clamps lower values that are already in a config to the floor when it computes a feed's due time. FeedSchedule::sync logs a warning once per feed and warns again only if the interval is fixed and later broken again. - The RssFeedConfig doc comment records the floor. config.example.toml does not document feeds, so it is unchanged. Two tests from the parent change used sub-floor intervals (15 s and 30 s) to exercise interval edits. They now use 120 s and 60 s; what they assert is unchanged. MIN_POLL_INTERVAL_SECS is a new public const in nzb-web. It is covered by the unreleased 0.4.24 bump on the parent branch. Fixes #109 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit d894fd55fbe21cda97fedb3eb7d855a0d47f9e5c) --- apps/rustnzb/src/handlers.rs | 18 +++++++ apps/rustnzb/tests/api_contracts.rs | 52 +++++++++++++++++++ crates/nzb-core/src/config.rs | 3 +- crates/nzb-web/src/rss_monitor.rs | 79 ++++++++++++++++++++++++++--- 4 files changed, 144 insertions(+), 8 deletions(-) diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index c2f9edcc..c6370644 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -468,6 +468,22 @@ fn fetch_policy(state: &AppState) -> FetchPolicy { /// Reject an RSS feed whose URL the fetch guard would refuse, so a blocked /// feed is reported when it is saved instead of failing on every poll. +/// Reject a feed whose poll interval would hammer the indexer (and, at 0, +/// spin the RSS monitor). +fn validate_feed_poll_interval(feed: &RssFeedConfig) -> Result<(), ApiError> { + let min = nzb_web::rss_monitor::MIN_POLL_INTERVAL_SECS; + if feed.poll_interval_secs < min { + return Err(ApiError::from(( + StatusCode::BAD_REQUEST, + format!( + "poll_interval_secs must be at least {min} (got {})", + feed.poll_interval_secs + ), + ))); + } + Ok(()) +} + async fn validate_feed_url(state: &AppState, url: &str) -> Result<(), ApiError> { check_fetch_url_allowed(url, &fetch_policy(state)) .await @@ -1344,6 +1360,7 @@ pub async fn h_rss_feed_add( State(state): State>, Json(feed): Json, ) -> Result { + validate_feed_poll_interval(&feed)?; validate_feed_url(&state, &feed.url).await?; state.update_config_with(|config| { if config.rss_feeds.iter().any(|f| f.name == feed.name) { @@ -1364,6 +1381,7 @@ pub async fn h_rss_feed_update( Path(name): Path, Json(feed): Json, ) -> Result, ApiError> { + validate_feed_poll_interval(&feed)?; validate_feed_url(&state, &feed.url).await?; state.update_config_with(|config| { let idx = config diff --git a/apps/rustnzb/tests/api_contracts.rs b/apps/rustnzb/tests/api_contracts.rs index c576691b..093da1cd 100644 --- a/apps/rustnzb/tests/api_contracts.rs +++ b/apps/rustnzb/tests/api_contracts.rs @@ -461,6 +461,58 @@ async fn rss_feed_url_is_validated_when_saved() { ); } +#[tokio::test] +async fn rss_feed_poll_interval_below_floor_is_rejected_when_saved() { + let app = start_app(false).await; + let client = reqwest::Client::new(); + let access = setup_access(&app, &client).await; + let feed = |secs: u64| serde_json::json!({"name":"tight", "url":"https://example.test/feed", "poll_interval_secs": secs, "enabled": true}); + + for secs in [0, 1, 59] { + let rejected = client + .post(format!("{}/api/config/rss-feeds", app.base_url)) + .bearer_auth(&access) + .json(&feed(secs)) + .send() + .await + .unwrap(); + assert_eq!( + rejected.status(), + reqwest::StatusCode::BAD_REQUEST, + "{secs}s" + ); + assert!( + rejected + .text() + .await + .unwrap() + .contains("poll_interval_secs") + ); + } + assert!(app.state.config().rss_feeds.is_empty()); + + assert_eq!( + client + .post(format!("{}/api/config/rss-feeds", app.base_url)) + .bearer_auth(&access) + .json(&feed(60)) + .send() + .await + .unwrap() + .status(), + reqwest::StatusCode::OK + ); + let rejected = client + .put(format!("{}/api/config/rss-feeds/tight", app.base_url)) + .bearer_auth(&access) + .json(&feed(0)) + .send() + .await + .unwrap(); + assert_eq!(rejected.status(), reqwest::StatusCode::BAD_REQUEST); + assert_eq!(app.state.config().rss_feeds[0].poll_interval_secs, 60); +} + async fn login(app: &ContractApp, client: &reqwest::Client) -> (String, String) { let tokens = client .post(format!("{}/api/auth/login", app.base_url)) diff --git a/crates/nzb-core/src/config.rs b/crates/nzb-core/src/config.rs index 27d64335..0c2f9ab5 100644 --- a/crates/nzb-core/src/config.rs +++ b/crates/nzb-core/src/config.rs @@ -329,7 +329,8 @@ pub struct RssFeedConfig { pub name: String, /// Feed URL (RSS 2.0 or Atom) pub url: String, - /// How often to poll, in seconds (default 900 = 15 minutes) + /// How often to poll, in seconds (default 900 = 15 minutes, minimum 60; + /// lower values are rejected by the API and clamped by the RSS monitor) #[serde(default = "default_poll_interval")] pub poll_interval_secs: u64, /// Category to assign to downloaded NZBs diff --git a/crates/nzb-web/src/rss_monitor.rs b/crates/nzb-web/src/rss_monitor.rs index 31b12278..aa50853b 100644 --- a/crates/nzb-web/src/rss_monitor.rs +++ b/crates/nzb-web/src/rss_monitor.rs @@ -439,6 +439,16 @@ impl RssMonitor { /// of RSS item history maintenance. const IDLE_INTERVAL: Duration = Duration::from_secs(900); +/// Smallest accepted `poll_interval_secs`. The API rejects anything lower, +/// and the monitor clamps lower values from existing or hand-edited configs +/// so a `0` interval cannot spin the loop against the indexer. +pub const MIN_POLL_INTERVAL_SECS: u64 = 60; + +/// A feed's poll interval with [`MIN_POLL_INTERVAL_SECS`] applied. +fn effective_poll_interval(feed: &RssFeedConfig) -> Duration { + Duration::from_secs(feed.poll_interval_secs.max(MIN_POLL_INTERVAL_SECS)) +} + /// When each enabled feed was last checked, keyed by feed name. /// /// A feed's next due time is derived from its *current* `poll_interval_secs` @@ -448,23 +458,47 @@ const IDLE_INTERVAL: Duration = Duration::from_secs(900); #[derive(Debug, Default)] struct FeedSchedule { last_checked: HashMap, + /// Feeds already warned about for an interval below the floor. + clamped: HashSet, } impl FeedSchedule { - /// Drop entries for feeds that are gone, disabled, or now point elsewhere. - fn sync(&mut self, feeds: &[RssFeedConfig]) { + /// Drop entries for feeds that are gone, disabled, or now point elsewhere, + /// and warn (once per feed) about intervals below the floor. Returns the + /// names newly warned about. + fn sync(&mut self, feeds: &[RssFeedConfig]) -> Vec { self.last_checked.retain(|name, (url, _)| { feeds .iter() .any(|f| f.enabled && &f.name == name && &f.url == url) }); + + let too_fast: HashSet<&str> = feeds + .iter() + .filter(|f| f.enabled && f.poll_interval_secs < MIN_POLL_INTERVAL_SECS) + .map(|f| f.name.as_str()) + .collect(); + self.clamped.retain(|name| too_fast.contains(name.as_str())); + let mut warned = Vec::new(); + for feed in feeds { + if too_fast.contains(feed.name.as_str()) && self.clamped.insert(feed.name.clone()) { + warn!( + feed = %feed.name, + poll_interval_secs = feed.poll_interval_secs, + min_secs = MIN_POLL_INTERVAL_SECS, + "RSS feed poll interval is below the minimum; using the minimum" + ); + warned.push(feed.name.clone()); + } + } + warned } fn due_at(&self, feed: &RssFeedConfig) -> Option { self.last_checked.get(&feed.name).map(|(_, at)| { // An absurd interval must not overflow `Instant`; a year out // is effectively "never" for a poller. - at.checked_add(Duration::from_secs(feed.poll_interval_secs)) + at.checked_add(effective_poll_interval(feed)) .unwrap_or(*at + Duration::from_secs(365 * 86_400)) }) } @@ -692,14 +726,14 @@ mod tests { // Shortening the interval takes effect without waiting out the old 600 s. state .update_config_with(|config| { - config.rss_feeds[0].poll_interval_secs = 30; + config.rss_feeds[0].poll_interval_secs = 60; Ok::<_, anyhow::Error>(()) }) .expect("edit feed"); let refetched_at = requests.recv().await.expect("feed was re-fetched"); let gap = refetched_at - fetched_at; assert!( - gap < std::time::Duration::from_secs(60), + gap < std::time::Duration::from_secs(120), "edited interval took {gap:?} to apply" ); @@ -767,9 +801,9 @@ mod tests { schedule.mark_checked(&feed, t0); assert_eq!(schedule.due_at(&feed), Some(t0 + Duration::from_secs(900))); - feed.poll_interval_secs = 15; + feed.poll_interval_secs = 120; schedule.sync(std::slice::from_ref(&feed)); - assert_eq!(schedule.due_at(&feed), Some(t0 + Duration::from_secs(15))); + assert_eq!(schedule.due_at(&feed), Some(t0 + Duration::from_secs(120))); feed.poll_interval_secs = u64::MAX; assert!( @@ -815,4 +849,35 @@ mod tests { "a re-enabled feed is checked at once" ); } + + #[test] + fn schedule_clamps_intervals_below_the_floor_and_warns_once_per_feed() { + let t0 = Instant::now(); + let mut zero = schedule_feed("zero", "https://example.test/zero", 0); + let tiny = schedule_feed("tiny", "https://example.test/tiny", 5); + let ok = schedule_feed("ok", "https://example.test/ok", MIN_POLL_INTERVAL_SECS); + let mut schedule = FeedSchedule::default(); + + let feeds = vec![zero.clone(), tiny.clone(), ok.clone()]; + let mut warned = schedule.sync(&feeds); + warned.sort(); + assert_eq!(warned, vec!["tiny".to_string(), "zero".to_string()]); + assert!(schedule.sync(&feeds).is_empty(), "warns only once per feed"); + + let floor = Duration::from_secs(MIN_POLL_INTERVAL_SECS); + for feed in [&zero, &tiny, &ok] { + schedule.mark_checked(feed, t0); + assert_eq!(schedule.due_at(feed), Some(t0 + floor), "{}", feed.name); + assert!(!schedule.is_due(feed, t0), "{} must not spin", feed.name); + } + + // Fixing the interval and then breaking it again warns afresh. + zero.poll_interval_secs = 900; + assert!(schedule.sync(std::slice::from_ref(&zero)).is_empty()); + zero.poll_interval_secs = 0; + assert_eq!( + schedule.sync(std::slice::from_ref(&zero)), + vec!["zero".to_string()] + ); + } } From ae524c351d0047a5aa75eafe6a72de8d1a56875b Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:08:48 -0500 Subject: [PATCH 09/33] fix(web): treat an RSS rule with no feeds as applying to all feeds The web UI labels the rule's feed field "blank = all feeds" and lists such rules as applying to "all", but the RSS monitor selected rules for a feed with `feed_names.iter().any(..)`, which is false for an empty list. A rule saved with the field left blank therefore never matched any feed and auto-download silently never fired. An empty list now matches every feed; the unknown-feed save warning already stays quiet for it. Fixes #111 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit b83f9c321d749f66bc223c4d8b18cce2c0f27821) --- apps/rustnzb/tests/api_contracts.rs | 11 +++ crates/nzb-core/src/models.rs | 2 +- crates/nzb-web/src/rss_monitor.rs | 105 +++++++++++++++++++++++++++- 3 files changed, 115 insertions(+), 3 deletions(-) diff --git a/apps/rustnzb/tests/api_contracts.rs b/apps/rustnzb/tests/api_contracts.rs index 093da1cd..07bb3f1c 100644 --- a/apps/rustnzb/tests/api_contracts.rs +++ b/apps/rustnzb/tests/api_contracts.rs @@ -702,6 +702,17 @@ async fn unknown_ids_return_404_and_invalid_input_returns_400() { body["warnings"][0].as_str().unwrap().contains("nope"), "{body}" ); + + // An empty feed list means "all feeds", so it is saved without a warning. + let (status, body) = call( + client + .post(format!("{base}/api/rss/rules")) + .bearer_auth(&access) + .json(&serde_json::json!({"name": "all", "feed_names": [], "match_regex": ".*"})), + ) + .await; + assert_eq!(status, 200, "{body}"); + assert!(body.get("warnings").is_none(), "{body}"); } #[tokio::test] diff --git a/crates/nzb-core/src/models.rs b/crates/nzb-core/src/models.rs index b537e8da..deaab50f 100644 --- a/crates/nzb-core/src/models.rs +++ b/crates/nzb-core/src/models.rs @@ -379,7 +379,7 @@ pub struct RssRule { pub id: String, /// Human-readable name for the rule pub name: String, - /// Which feed(s) this rule applies to (one or more feed names) + /// Which feed(s) this rule applies to; empty means every feed pub feed_names: Vec, /// Category to assign to downloaded NZBs pub category: Option, diff --git a/crates/nzb-web/src/rss_monitor.rs b/crates/nzb-web/src/rss_monitor.rs index aa50853b..26351722 100644 --- a/crates/nzb-web/src/rss_monitor.rs +++ b/crates/nzb-web/src/rss_monitor.rs @@ -195,13 +195,16 @@ impl RssMonitor { ), }; - // Load download rules for this feed + // Load download rules for this feed (an empty feed list means all feeds) let rules = self .queue_manager .rss_rule_list() .unwrap_or_default() .into_iter() - .filter(|r| r.enabled && r.feed_names.iter().any(|n| n == &feed.name)) + .filter(|r| { + r.enabled + && (r.feed_names.is_empty() || r.feed_names.iter().any(|n| n == &feed.name)) + }) .collect::>(); // Collect all items for batch insert (single DB lock) @@ -606,6 +609,104 @@ mod tests { assert!(RssMonitor::compile_filter(&"x".repeat(513)).is_none()); } + /// Serve a one-item feed plus its NZB on loopback, and return a monitor + /// whose fetch policy admits that loopback address. + async fn monitor_with_local_feed( + data_dir: PathBuf, + ) -> (RssMonitor, Arc, RssFeedConfig) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0") + .await + .expect("bind loopback"); + let base = format!("http://{}", listener.local_addr().expect("local addr")); + let feed_xml = format!( + r#" + feedFeed2026-07-27T00:00:00Z + new-itemShow.S01E01.1080p + 2026-07-27T00:00:00Z + + + "# + ); + let nzb = r#" + + + alt.binaries.test + seg@example.test + +"#; + let app = axum::Router::new() + .route( + "/feed.xml", + axum::routing::get(move || async move { feed_xml }), + ) + .route( + "/release.nzb", + axum::routing::get(move || async move { nzb }), + ); + tokio::spawn(async move { axum::serve(listener, app).await }); + + let (mut monitor, queue_manager) = monitor(data_dir); + let mut config = AppConfig::default(); + config.general.fetch_allowed_hosts = vec!["127.0.0.1".into()]; + monitor.config = Arc::new(ArcSwap::from_pointee(config)); + let feed = RssFeedConfig { + name: "tv-feed".into(), + url: format!("{base}/feed.xml"), + poll_interval_secs: 900, + category: None, + filter_regex: None, + enabled: true, + auto_download: false, + max_age_days: None, + }; + (monitor, queue_manager, feed) + } + + fn rule(feed_names: Vec) -> crate::nzb_core::models::RssRule { + crate::nzb_core::models::RssRule { + id: "rule".into(), + name: "TV".into(), + feed_names, + category: None, + priority: 1, + match_regex: "1080p".into(), + enabled: true, + } + } + + #[tokio::test] + async fn rule_with_no_feed_names_applies_to_every_feed() { + let temp = tempfile::tempdir().expect("tempdir"); + let (monitor, queue_manager, feed) = monitor_with_local_feed(temp.path().into()).await; + queue_manager + .rss_rule_insert(&rule(Vec::new())) + .expect("insert rule"); + + monitor.check_feed(&feed).await.expect("feed check"); + + assert_eq!(queue_manager.get_jobs().len(), 1); + assert!( + queue_manager + .rss_item_get("new-item") + .expect("database query") + .expect("stored item") + .downloaded + ); + } + + #[tokio::test] + async fn rule_naming_another_feed_does_not_apply() { + let temp = tempfile::tempdir().expect("tempdir"); + let (monitor, queue_manager, feed) = monitor_with_local_feed(temp.path().into()).await; + queue_manager + .rss_rule_insert(&rule(vec!["other-feed".into()])) + .expect("insert rule"); + + monitor.check_feed(&feed).await.expect("feed check"); + + assert!(queue_manager.get_jobs().is_empty()); + } + #[tokio::test] async fn feed_checks_reject_local_targets_before_network_access() { let temp = tempfile::tempdir().expect("tempdir"); From dcf046444ce6837cad77cbbfc509fe179055a686 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:30:26 -0500 Subject: [PATCH 10/33] fix(server): layer the DAV extension in build_router so tests match prod (cherry picked from commit ea5de5ef2b6cf22e9d0a87a324333749b3e92af2) Adapted: the test harness keeps its Arc clone of the app state, since it returns the state to callers after handing one to build_router. --- apps/rustnzb/src/main.rs | 32 +----------- apps/rustnzb/src/server.rs | 51 ++++++++++++++++++++ apps/rustnzb/tests/sab_api_key_api.rs | 2 - apps/rustnzb/tests/server_bind_error_test.rs | 4 -- apps/rustnzb/tests/support/mod.rs | 2 - 5 files changed, 52 insertions(+), 39 deletions(-) diff --git a/apps/rustnzb/src/main.rs b/apps/rustnzb/src/main.rs index 2dc037ed..cfef8554 100644 --- a/apps/rustnzb/src/main.rs +++ b/apps/rustnzb/src/main.rs @@ -314,7 +314,6 @@ async fn main() -> anyhow::Result<()> { #[cfg(feature = "webdav")] { - use axum::Extension; use std::sync::Arc; let servers = result.queue_manager.get_servers(); @@ -378,36 +377,7 @@ async fn main() -> anyhow::Result<()> { }); } - let router = { - let mut r = rustnzb::server::build_router(result.state.clone()); - if let Some(ref dav) = dav_handle { - let auth_state = result.state.clone(); - let dav_auth = axum::middleware::from_fn( - move |headers: axum::http::HeaderMap, - req: axum::extract::Request, - next: axum::middleware::Next| { - let st = auth_state.clone(); - async move { rustnzb::dav::auth::dav_auth(st, headers, req, next).await } - }, - ); - let dav_router = nzbdav_dav::dav_router(Arc::clone(&dav.store)).layer(dav_auth); - r = r.nest("/dav", dav_router); - let cfg = result.state.config(); - if cfg.dav.username.is_none() - && cfg.dav.password.is_none() - && cfg.dav.api_key.is_none() - { - tracing::warn!( - "WebDAV media library mounted at /dav (UNAUTHENTICATED — \ - set dav.username/password or dav.api_key in config)" - ); - } else { - info!("WebDAV media library mounted at /dav (auth enabled)"); - } - } - // Always layer Option> so h_status and h_dav_add can extract it. - r.layer(Extension(dav_handle)) - }; + let router = rustnzb::server::build_router_with_dav(result.state.clone(), dav_handle); rustnzb::server::serve(result.state, router).await?; } diff --git a/apps/rustnzb/src/server.rs b/apps/rustnzb/src/server.rs index 45e5ab88..48c71689 100644 --- a/apps/rustnzb/src/server.rs +++ b/apps/rustnzb/src/server.rs @@ -115,7 +115,58 @@ fn is_hashed_asset(path: &str) -> bool { } /// Build the axum Router with all API routes. +/// +/// With the `webdav` feature this is [`build_router_with_dav`] with no DAV +/// handle, so the router always carries the `Option>` extension +/// that `/api/status` and `/api/dav/add` extract. Every caller — the binary and +/// the integration tests — gets the same wiring. pub fn build_router(state: Arc) -> Router { + #[cfg(feature = "webdav")] + { + build_router_with_dav(state, None) + } + #[cfg(not(feature = "webdav"))] + { + build_api_router(state) + } +} + +/// Build the router with an optional WebDAV media library. +/// +/// When `dav` is `Some`, the DAV store is mounted at `/dav` behind +/// [`crate::dav::auth::dav_auth`]. The `Option>` extension is +/// layered either way, because handlers extract it unconditionally when the +/// feature is compiled in and would otherwise fail with a 500. +#[cfg(feature = "webdav")] +pub fn build_router_with_dav( + state: Arc, + dav: Option>, +) -> Router { + let mut router = build_api_router(state.clone()); + if let Some(ref dav) = dav { + let auth_state = state.clone(); + let dav_auth = axum::middleware::from_fn( + move |headers: HeaderMap, req: axum::extract::Request, next: Next| { + let st = auth_state.clone(); + async move { crate::dav::auth::dav_auth(st, headers, req, next).await } + }, + ); + let dav_router = nzbdav_dav::dav_router(Arc::clone(&dav.store)).layer(dav_auth); + router = router.nest("/dav", dav_router); + let cfg = state.config(); + if cfg.dav.username.is_none() && cfg.dav.password.is_none() && cfg.dav.api_key.is_none() { + tracing::warn!( + "WebDAV media library mounted at /dav (UNAUTHENTICATED — \ + set dav.username/password or dav.api_key in config)" + ); + } else { + info!("WebDAV media library mounted at /dav (auth enabled)"); + } + } + router.layer(axum::Extension(dav)) +} + +fn build_api_router(state: Arc) -> Router { let cors = CorsLayer::default() .allow_origin(AllowOrigin::any()) .allow_headers(AllowHeaders::any()); diff --git a/apps/rustnzb/tests/sab_api_key_api.rs b/apps/rustnzb/tests/sab_api_key_api.rs index 6710e092..b2a11b46 100644 --- a/apps/rustnzb/tests/sab_api_key_api.rs +++ b/apps/rustnzb/tests/sab_api_key_api.rs @@ -51,8 +51,6 @@ async fn start_test_server() -> ( credential_store, )); let router = build_router(state.clone()); - #[cfg(feature = "webdav")] - let router = router.layer(axum::Extension(None::>)); let listener = tokio::net::TcpListener::bind("127.0.0.1:0") .await diff --git a/apps/rustnzb/tests/server_bind_error_test.rs b/apps/rustnzb/tests/server_bind_error_test.rs index 6d261f06..6595b46c 100644 --- a/apps/rustnzb/tests/server_bind_error_test.rs +++ b/apps/rustnzb/tests/server_bind_error_test.rs @@ -69,8 +69,6 @@ async fn serve_reports_context_on_bind_failure() { let state = build_state(config).await; let router = build_router(state.clone()); - #[cfg(feature = "webdav")] - let router = router.layer(axum::Extension(None::>)); let result = rustnzb::server::serve(state, router).await; let err = match result { @@ -103,8 +101,6 @@ async fn serve_succeeds_on_free_port() { let state = build_state(config).await; let router = build_router(state.clone()); - #[cfg(feature = "webdav")] - let router = router.layer(axum::Extension(None::>)); let handle = tokio::spawn(async move { rustnzb::server::serve(state, router).await }); diff --git a/apps/rustnzb/tests/support/mod.rs b/apps/rustnzb/tests/support/mod.rs index 312aac60..86502bb5 100644 --- a/apps/rustnzb/tests/support/mod.rs +++ b/apps/rustnzb/tests/support/mod.rs @@ -126,8 +126,6 @@ async fn start_test_server_inner( )); let router = build_router(state.clone()); - #[cfg(feature = "webdav")] - let router = router.layer(axum::Extension(None::>)); let listener = tokio::net::TcpListener::bind("127.0.0.1:0") .await .expect("Failed to bind random port"); From 36b2087823cfd3193d58ef81baa759fc72fe3800 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 09:58:23 -0500 Subject: [PATCH 11/33] fix(api): 404/409 for unknown and duplicate config entries; validate categories on save Config handlers wrapped "not found" and "already exists" in anyhow!, which carries no status, so the central mapping from #79 never saw them and clients got 500 internal_error for their own mistakes. Unknown and duplicate entries (BUG-73): - Categories: PUT/DELETE of an unknown name return CategoryNotFound (404); a duplicate POST is 409. - Servers: PUT/DELETE/test of an unknown id return ServerNotFound (404, error_kind server_not_found); POST of an id that already exists is 409 instead of silently adding a second server with the same id. - RSS feeds: PUT/DELETE unknown -> 404, duplicate POST -> 409; downloading an unknown RSS item -> 404. - Groups: get, status, headers/fetch and headers/download of an unknown group id -> 404. - nzb-web gains ApiError::conflict(msg) so a 409 with a message goes through the same type rather than an ad hoc status. Category validation (BUG-74): - POST/PUT category now refuse, with 400, a name that is not a single safe path component and an output_dir that enqueue would reject. Before, these saved with 200 and every later job in the category failed at enqueue with "category output path is unsafe". - The rule lives in one place: nzb_core::path::category_output_root, used by both CategoryConfig::validate and QueueManager::output_dir_for. A relative output_dir must stay below the complete dir; an absolute one is still allowed, but no longer with a ".." component or control characters. An empty output_dir is refused (it already failed at enqueue). - Startup logs a warning for each unsafe category in a hand-edited config.toml instead of failing. The existing contract test asserted 500 for a duplicate category; it now asserts 409. Bumps nzb-core to 0.2.19 and nzb-web to 0.4.24. Fixes #115 Fixes #116 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit ed1f13dbb260c3e771ae65b36c885c730fdaf8fd) --- Cargo.lock | 2 +- Cargo.toml | 2 +- apps/rustnzb/src/group_handlers.rs | 8 +- apps/rustnzb/src/handlers.rs | 29 ++-- apps/rustnzb/tests/api_contracts.rs | 207 +++++++++++++++++++++++++++- crates/nzb-core/Cargo.toml | 2 +- crates/nzb-core/src/config.rs | 49 +++++++ crates/nzb-core/src/path.rs | 42 ++++++ crates/nzb-web/src/error.rs | 12 ++ crates/nzb-web/src/queue_manager.rs | 12 +- crates/nzb-web/src/startup.rs | 20 ++- desktop/src-tauri/Cargo.lock | 4 +- 12 files changed, 360 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 43274716..200f5a35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1773,7 +1773,7 @@ dependencies = [ [[package]] name = "nzb-core" -version = "0.2.18" +version = "0.2.19" dependencies = [ "anyhow", "chrono", diff --git a/Cargo.toml b/Cargo.toml index 5582f1fc..9df3672d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -90,7 +90,7 @@ unicode-normalization = "0.1" # Shared NZB crates nzb-web = { version = "0.4.24", path = "crates/nzb-web", features = ["groups-db"] } nzb-nntp = { version = "0.2.24", path = "crates/nzb-nntp" } -nzb-core = { version = "0.2.18", path = "crates/nzb-core", features = ["groups-db"] } +nzb-core = { version = "0.2.19", path = "crates/nzb-core", features = ["groups-db"] } nzb-decode = { version = "0.1.5", path = "crates/nzb-decode" } nzb-news = { version = "0.1.14", path = "crates/nzb-news" } nzb-dispatch = { version = "0.2.8", path = "crates/nzb-dispatch" } diff --git a/apps/rustnzb/src/group_handlers.rs b/apps/rustnzb/src/group_handlers.rs index 4edac6dc..888f3fac 100644 --- a/apps/rustnzb/src/group_handlers.rs +++ b/apps/rustnzb/src/group_handlers.rs @@ -125,7 +125,7 @@ pub async fn h_group_get( .queue_manager .with_db(|db| db.group_get(id)) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Group not found")))?; + .ok_or_else(|| ApiError::not_found("Group not found"))?; Ok(Json(serde_json::to_value(group).map_err(|e| { ApiError::from(anyhow::anyhow!("Serialisation error: {e}")) })?)) @@ -140,7 +140,7 @@ pub async fn h_group_status( let group = qm .with_db(|db| db.group_get(id)) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Group not found")))?; + .ok_or_else(|| ApiError::not_found("Group not found"))?; let total_headers = qm .with_db(|db| db.header_count(id, None)) @@ -215,7 +215,7 @@ pub async fn h_header_fetch( .queue_manager .with_db(|db| db.group_get(group_id)) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Group not found")))?; + .ok_or_else(|| ApiError::not_found("Group not found"))?; let servers = state.queue_manager.get_servers(); let server = servers @@ -422,7 +422,7 @@ pub async fn h_header_download( .queue_manager .with_db(|db| db.group_get(group_id)) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Group not found")))?; + .ok_or_else(|| ApiError::not_found("Group not found"))?; let name = input .name diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index c6370644..277c1ff1 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -19,6 +19,7 @@ static HTTP_CLIENT: std::sync::LazyLock = std::sync::LazyLock:: .expect("Failed to build shared HTTP client") }); +use nzb_web::nzb_core::NzbError; #[cfg(feature = "webdav")] use nzb_web::nzb_core::config::DavConfig; use nzb_web::nzb_core::config::{ @@ -900,6 +901,12 @@ pub async fn h_server_add( } state.update_config_with(|config| { + if config.servers.iter().any(|s| s.id == server.id) { + return Err(ApiError::conflict(format!( + "Server '{}' already exists", + server.id + ))); + } config.servers.push(server); Ok::<_, ApiError>(()) })?; @@ -943,7 +950,7 @@ pub async fn h_server_update( .servers .iter() .position(|s| s.id == id) - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Server not found: {id}")))?; + .ok_or_else(|| ApiError::from(NzbError::ServerNotFound(id.clone())))?; let mut server = merge_server_update(&config.servers[idx], patch)?; keep_unchanged_password(&mut server, &config.servers[idx]); @@ -988,7 +995,7 @@ pub async fn h_server_delete( config.servers.retain(|s| s.id != id); if config.servers.len() == before { - return Err(ApiError::from(anyhow::anyhow!("Server not found: {id}"))); + return Err(ApiError::from(NzbError::ServerNotFound(id.clone()))); } Ok::<_, ApiError>(()) @@ -1011,7 +1018,7 @@ pub async fn h_server_test( .servers .iter() .find(|s| s.id == id) - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Server not found: {id}")))? + .ok_or_else(|| ApiError::from(NzbError::ServerNotFound(id.clone())))? .clone(); // Test connection in a spawned task with timeout @@ -1162,9 +1169,10 @@ pub async fn h_category_add( State(state): State>, Json(cat): Json, ) -> Result { + cat.validate().map_err(ApiError::bad_request)?; state.update_config_with(|config| { if config.categories.iter().any(|c| c.name == cat.name) { - return Err(ApiError::from(anyhow::anyhow!( + return Err(ApiError::conflict(format!( "Category '{}' already exists", cat.name ))); @@ -1184,12 +1192,13 @@ pub async fn h_category_update( Path(name): Path, Json(cat): Json, ) -> Result, ApiError> { + cat.validate().map_err(ApiError::bad_request)?; state.update_config_with(|config| { let idx = config .categories .iter() .position(|c| c.name == name) - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Category not found")))?; + .ok_or_else(|| ApiError::from(NzbError::CategoryNotFound(name.clone())))?; config.categories[idx] = cat; state .queue_manager @@ -1208,7 +1217,7 @@ pub async fn h_category_delete( let initial_len = config.categories.len(); config.categories.retain(|c| c.name != name); if config.categories.len() == initial_len { - return Err(ApiError::from(anyhow::anyhow!("Category not found"))); + return Err(ApiError::from(NzbError::CategoryNotFound(name.clone()))); } state .queue_manager @@ -1364,7 +1373,7 @@ pub async fn h_rss_feed_add( validate_feed_url(&state, &feed.url).await?; state.update_config_with(|config| { if config.rss_feeds.iter().any(|f| f.name == feed.name) { - return Err(ApiError::from(anyhow::anyhow!( + return Err(ApiError::conflict(format!( "Feed '{}' already exists", feed.name ))); @@ -1388,7 +1397,7 @@ pub async fn h_rss_feed_update( .rss_feeds .iter() .position(|f| f.name == name) - .ok_or_else(|| ApiError::from(anyhow::anyhow!("Feed not found")))?; + .ok_or_else(|| ApiError::not_found("Feed not found"))?; config.rss_feeds[idx] = feed; Ok::<_, ApiError>(()) })?; @@ -1404,7 +1413,7 @@ pub async fn h_rss_feed_delete( let len = config.rss_feeds.len(); config.rss_feeds.retain(|f| f.name != name); if config.rss_feeds.len() == len { - return Err(ApiError::from(anyhow::anyhow!("Feed not found"))); + return Err(ApiError::not_found("Feed not found")); } Ok::<_, ApiError>(()) })?; @@ -1443,7 +1452,7 @@ pub async fn h_rss_item_download( .queue_manager .rss_item_get(&id) .map_err(ApiError::from)? - .ok_or_else(|| ApiError::from(anyhow::anyhow!("RSS item not found")))?; + .ok_or_else(|| ApiError::not_found("RSS item not found"))?; let url = item .url diff --git a/apps/rustnzb/tests/api_contracts.rs b/apps/rustnzb/tests/api_contracts.rs index 07bb3f1c..84e2e4c4 100644 --- a/apps/rustnzb/tests/api_contracts.rs +++ b/apps/rustnzb/tests/api_contracts.rs @@ -283,7 +283,9 @@ async fn config_routes_validate_duplicates_and_persist_successful_updates() { .await .unwrap() .status(), - reqwest::StatusCode::INTERNAL_SERVER_ERROR + // A duplicate name is a client-side conflict. This asserted 500 + // before BUG-73 routed it through the central 409 mapping. + reqwest::StatusCode::CONFLICT ); let feed = serde_json::json!({"name":"daily", "url":"https://example.test/feed", "poll_interval_secs":60, "category":"tv", "filter_regex":null, "enabled":true, "auto_download":false}); @@ -947,3 +949,206 @@ async fn missing_article_returns_404() { .await; assert_eq!(status, 404, "{body}"); } + +/// BUG-73: config endpoints answer an unknown name or id with 404 and a +/// duplicate add with 409, instead of a 500 that reads as a server fault. +#[tokio::test] +async fn unknown_config_entries_return_404_and_duplicates_return_409() { + let app = start_app(true).await; + let client = reqwest::Client::new(); + let (access, _) = login(&app, &client).await; + let base = &app.base_url; + + // Categories. + let category = serde_json::json!({"name": "tv", "output_dir": null, "post_processing": 3}); + let (status, body) = call( + client + .put(format!("{base}/api/config/categories/no-such-cat")) + .bearer_auth(&access) + .json(&category), + ) + .await; + assert_eq!(status, 404, "{body}"); + assert_eq!(body["error_kind"], "not_found"); + let (status, body) = call( + client + .delete(format!("{base}/api/config/categories/no-such-cat")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); + assert_eq!(body["error_kind"], "not_found"); + for expected in [200, 409] { + let (status, body) = call( + client + .post(format!("{base}/api/config/categories")) + .bearer_auth(&access) + .json(&category), + ) + .await; + assert_eq!(status, expected, "{body}"); + } + + // Servers. + let (status, body) = call( + client + .put(format!("{base}/api/config/servers/no-such-server")) + .bearer_auth(&access) + .json(&serde_json::json!({"port": 563})), + ) + .await; + assert_eq!(status, 404, "{body}"); + assert_eq!(body["error_kind"], "server_not_found"); + let (status, body) = call( + client + .delete(format!("{base}/api/config/servers/no-such-server")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); + assert_eq!(body["error_kind"], "server_not_found"); + let (status, body) = call( + client + .post(format!("{base}/api/config/servers/no-such-server/test")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); + let server = serde_json::json!({ + "id": "primary", "name": "Primary", "host": "news.example.test", + "port": 563, "ssl": true, "connections": 4, + }); + for expected in [200, 409] { + let (status, body) = call( + client + .post(format!("{base}/api/config/servers")) + .bearer_auth(&access) + .json(&server), + ) + .await; + assert_eq!(status, expected, "{body}"); + } + assert_eq!(app.state.config().servers.len(), 1); + + // RSS feeds and items. + let feed = serde_json::json!({"name":"daily", "url":"https://example.test/feed", "poll_interval_secs":60, "category":null, "filter_regex":null, "enabled":true, "auto_download":false}); + let (status, body) = call( + client + .put(format!("{base}/api/config/rss-feeds/no-such-feed")) + .bearer_auth(&access) + .json(&feed), + ) + .await; + assert_eq!(status, 404, "{body}"); + let (status, body) = call( + client + .delete(format!("{base}/api/config/rss-feeds/no-such-feed")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); + for expected in [200, 409] { + let (status, body) = call( + client + .post(format!("{base}/api/config/rss-feeds")) + .bearer_auth(&access) + .json(&feed), + ) + .await; + assert_eq!(status, expected, "{body}"); + } + let (status, body) = call( + client + .post(format!("{base}/api/rss/items/no-such-item/download")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); + + // Groups. + for path in ["", "/status"] { + let (status, body) = call( + client + .get(format!("{base}/api/groups/987654{path}")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{path}: {body}"); + assert_eq!(body["error_kind"], "not_found"); + } + let (status, body) = call( + client + .post(format!("{base}/api/groups/987654/headers/fetch")) + .bearer_auth(&access), + ) + .await; + assert_eq!(status, 404, "{body}"); +} + +/// BUG-74: a category whose name or output_dir would make every later +/// enqueue fail ("category output path is unsafe") is refused at save time. +#[tokio::test] +async fn category_save_rejects_unsafe_name_and_output_dir() { + let app = start_app(true).await; + let client = reqwest::Client::new(); + let (access, _) = login(&app, &client).await; + let base = &app.base_url; + let category = |name: &str, output_dir: serde_json::Value| serde_json::json!({"name": name, "output_dir": output_dir, "post_processing": 3}); + + let unsafe_bodies = [ + category("../escape", serde_json::Value::Null), + category("a/b", serde_json::Value::Null), + category("..", serde_json::Value::Null), + category("", serde_json::Value::Null), + category("tab\tname", serde_json::Value::Null), + category("tv", serde_json::json!("../../../etc")), + category("tv", serde_json::json!("shows/../../etc")), + category("tv", serde_json::json!("")), + category("tv", serde_json::json!("/srv/media/../../etc")), + ]; + for body in &unsafe_bodies { + let (status, response) = call( + client + .post(format!("{base}/api/config/categories")) + .bearer_auth(&access) + .json(body), + ) + .await; + assert_eq!(status, 400, "POST {body}: {response}"); + assert_eq!(response["error_kind"], "bad_request"); + let (status, response) = call( + client + .put(format!("{base}/api/config/categories/Default")) + .bearer_auth(&access) + .json(body), + ) + .await; + assert_eq!(status, 400, "PUT {body}: {response}"); + } + let saved = AppConfig::load(&app.config_path).unwrap(); + assert_eq!(saved.categories.len(), 1); + assert_eq!(saved.categories[0].name, "Default"); + assert!(saved.categories[0].output_dir.is_none()); + + // What enqueue accepts is still accepted: relative subdirectories and + // absolute roots. + for body in [ + category("tv", serde_json::json!("shows/tv")), + category("movies", serde_json::json!("/srv/media/movies")), + ] { + let (status, response) = call( + client + .post(format!("{base}/api/config/categories")) + .bearer_auth(&access) + .json(&body), + ) + .await; + assert_eq!(status, 200, "{body}: {response}"); + } + let output = app + .state + .queue_manager + .output_dir_for("tv", "Some.Job") + .unwrap(); + assert!(output.ends_with("complete/shows/tv/Some.Job"), "{output:?}"); +} diff --git a/crates/nzb-core/Cargo.toml b/crates/nzb-core/Cargo.toml index 41f52697..8a7e5016 100644 --- a/crates/nzb-core/Cargo.toml +++ b/crates/nzb-core/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "nzb-core" -version = "0.2.18" +version = "0.2.19" edition = "2024" description = "Shared models, config, NZB parser, and SQLite database for NZB clients" license = "MIT" diff --git a/crates/nzb-core/src/config.rs b/crates/nzb-core/src/config.rs index 0c2f9ab5..433c478e 100644 --- a/crates/nzb-core/src/config.rs +++ b/crates/nzb-core/src/config.rs @@ -322,6 +322,30 @@ impl Default for CategoryConfig { } } +impl CategoryConfig { + /// Check the name and `output_dir` against the rules + /// `QueueManager::output_dir_for` applies when a job is enqueued, so an + /// entry that would make every job in the category fail can be refused + /// when it is saved. Returns a message suitable for an API client. + pub fn validate(&self) -> std::result::Result<(), &'static str> { + if crate::path::safe_component(&self.name).is_none() { + return Err("category name must be a single non-empty path component \ + without '/', '\\', '..' or control characters"); + } + if let Some(output_dir) = &self.output_dir { + // Only the shape of the override matters here, not where the + // complete directory lives. + if crate::path::category_output_root(std::path::Path::new("/"), output_dir).is_none() { + return Err( + "category output_dir must be an absolute path, or a relative \ + path below the complete directory, without '..' or control characters", + ); + } + } + Ok(()) + } +} + /// RSS feed configuration for automatic NZB downloading. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct RssFeedConfig { @@ -697,6 +721,31 @@ ssl = false assert!(cfg.server("nonexistent").is_none()); } + #[test] + fn category_validate_matches_enqueue_rules() { + let category = |name: &str, output_dir: Option<&str>| CategoryConfig { + name: name.into(), + output_dir: output_dir.map(PathBuf::from), + ..CategoryConfig::default() + }; + assert!(CategoryConfig::default().validate().is_ok()); + assert!(category("*", None).validate().is_ok()); + assert!(category("tv", Some("shows/tv")).validate().is_ok()); + assert!(category("tv", Some("/srv/media/tv")).validate().is_ok()); + for bad in [ + category("", None), + category("..", None), + category("a/b", None), + category("a\\b", None), + category("a\tb", None), + category("tv", Some("")), + category("tv", Some("../../../etc")), + category("tv", Some("/srv/../etc")), + ] { + assert!(bad.validate().is_err(), "{bad:?}"); + } + } + #[test] fn test_find_category_or_default() { let mut cfg = AppConfig::default(); diff --git a/crates/nzb-core/src/path.rs b/crates/nzb-core/src/path.rs index a30a7ea2..3bcfa618 100644 --- a/crates/nzb-core/src/path.rs +++ b/crates/nzb-core/src/path.rs @@ -40,6 +40,23 @@ pub fn safe_component(value: &str) -> Option<&str> { Some(value) } +/// Resolve a category `output_dir` override to the root its jobs land in. +/// +/// A relative override is joined beneath `complete_dir` with [`safe_join`]. +/// An absolute override is used as is, provided it has no `..` component +/// and no control characters. Returns `None` for anything else, including +/// an empty override. +pub fn category_output_root(complete_dir: &Path, output_dir: &Path) -> Option { + if output_dir.is_absolute() { + let clean = !output_dir + .components() + .any(|component| matches!(component, std::path::Component::ParentDir)) + && !output_dir.to_string_lossy().chars().any(char::is_control); + return clean.then(|| output_dir.to_path_buf()); + } + safe_join(complete_dir, &output_dir.to_string_lossy()) +} + /// Maximum length, in bytes, of a sanitized job name. Leaves headroom under /// the common 255-byte `NAME_MAX` for suffixes such as `.1` or `_UNPACK_`. pub const MAX_JOB_NAME_BYTES: usize = 240; @@ -140,6 +157,31 @@ mod tests { assert!(safe_component(r"movies\tv").is_none()); } + #[test] + fn category_output_root_accepts_subdirs_and_clean_absolute_roots() { + let complete = Path::new("/data/complete"); + assert_eq!( + category_output_root(complete, Path::new("shows/tv")), + Some(PathBuf::from("/data/complete/shows/tv")) + ); + assert_eq!( + category_output_root(complete, Path::new("/srv/media")), + Some(PathBuf::from("/srv/media")) + ); + for unsafe_dir in [ + "", + "../../../etc", + "shows/../../etc", + "/srv/../etc", + "/srv/a\nb", + ] { + assert!( + category_output_root(complete, Path::new(unsafe_dir)).is_none(), + "{unsafe_dir:?}" + ); + } + } + #[test] fn sanitize_job_name_replaces_windows_illegal_characters() { assert_eq!(sanitize_job_name("a:b*c?d"), "a-b_c_d"); diff --git a/crates/nzb-web/src/error.rs b/crates/nzb-web/src/error.rs index ef077b36..fde00411 100644 --- a/crates/nzb-web/src/error.rs +++ b/crates/nzb-web/src/error.rs @@ -67,6 +67,15 @@ impl ApiError { } } + /// 409 for a create that collides with an existing entry, such as a + /// category, server or feed that is already configured. + pub fn conflict(msg: impl Into) -> Self { + Self { + status: Some(StatusCode::CONFLICT), + kind: ApiErrorKind::Message(msg.into()), + } + } + pub const fn admission_conflict() -> Self { Self { status: Some(StatusCode::CONFLICT), @@ -252,5 +261,8 @@ mod tests { assert_eq!(json["error_kind"], "not_found"); let json = serde_json::to_value(ApiError::bad_request("bad")).unwrap(); assert_eq!(json["error_kind"], "bad_request"); + let json = serde_json::to_value(ApiError::conflict("taken")).unwrap(); + assert_eq!(json["error_kind"], "conflict"); + assert_eq!(json["status"], 409); } } diff --git a/crates/nzb-web/src/queue_manager.rs b/crates/nzb-web/src/queue_manager.rs index 94697ed5..01bcb04c 100644 --- a/crates/nzb-web/src/queue_manager.rs +++ b/crates/nzb-web/src/queue_manager.rs @@ -4005,14 +4005,10 @@ impl QueueManager { .iter() .find(|configured| configured.name == category); if let Some(base) = category_config.and_then(|configured| configured.output_dir.as_ref()) { - let root = if base.is_absolute() { - base.clone() - } else { - crate::nzb_core::path::safe_join(&self.complete_dir(), &base.to_string_lossy()) - .ok_or_else(|| { - crate::nzb_core::NzbError::Other("category output path is unsafe".into()) - })? - }; + let root = crate::nzb_core::path::category_output_root(&self.complete_dir(), base) + .ok_or_else(|| { + crate::nzb_core::NzbError::Other("category output path is unsafe".into()) + })?; return crate::nzb_core::path::safe_join(&root, name).ok_or_else(|| { crate::nzb_core::NzbError::Other("category output path is unsafe".into()) }); diff --git a/crates/nzb-web/src/startup.rs b/crates/nzb-web/src/startup.rs index e6e0283f..b47fc7c1 100644 --- a/crates/nzb-web/src/startup.rs +++ b/crates/nzb-web/src/startup.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use anyhow::Context; use arc_swap::ArcSwap; -use tracing::info; +use tracing::{info, warn}; use crate::nzb_core::config::AppConfig; use crate::nzb_core::db::Database; @@ -49,6 +49,22 @@ fn env_flag_enabled(name: &str) -> Option { /// Create a data directory (e.g. `data_dir`/`incomplete_dir`/`complete_dir`), attaching /// the failing path and a permission hint to any error so failures are actionable /// instead of a bare `Permission denied (os error 13)`. +/// A hand-edited config.toml can carry a category the API would refuse. +/// Starting anyway keeps the rest of the service usable; the warning +/// explains why jobs in that category fail to enqueue. +fn warn_unsafe_categories(config: &AppConfig) { + for category in &config.categories { + if let Err(reason) = category.validate() { + warn!( + category = %category.name, + output_dir = ?category.output_dir, + reason, + "Unsafe category in config; jobs in it will fail to enqueue until it is fixed" + ); + } + } +} + fn create_data_dir(path: &Path) -> anyhow::Result<()> { std::fs::create_dir_all(path).with_context(|| { format!( @@ -135,6 +151,8 @@ pub async fn initialize( config.otel.service_name = val; } + warn_unsafe_categories(&config); + // Ensure directories exist create_data_dir(&config.general.data_dir)?; create_data_dir(&config.general.incomplete_dir)?; diff --git a/desktop/src-tauri/Cargo.lock b/desktop/src-tauri/Cargo.lock index 31063d98..eb5ea881 100644 --- a/desktop/src-tauri/Cargo.lock +++ b/desktop/src-tauri/Cargo.lock @@ -3015,7 +3015,7 @@ dependencies = [ [[package]] name = "nzb-core" -version = "0.2.18" +version = "0.2.19" dependencies = [ "anyhow", "chrono", @@ -3109,7 +3109,7 @@ dependencies = [ [[package]] name = "nzb-web" -version = "0.4.23" +version = "0.4.24" dependencies = [ "anyhow", "arc-swap", From cbd4272939af602abd17b2a64919d38c9408db97 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:18:03 -0500 Subject: [PATCH 12/33] fix(api): 400/409 instead of 500 for bad input on add, browse and RSS download (cherry picked from commit 4311ba500ce65c4293c1b05df87385d56ad862ac) Adapted: history retry already goes through retry_history_entry, so the missing-NZB-data case maps HistoryRetryOutcome::NoNzbData to 409 instead of re-introducing the inline retry path this commit replaced. --- apps/rustnzb/src/handlers.rs | 47 +++++++------- apps/rustnzb/tests/api_contracts.rs | 94 ++++++++++++++++++++++++++++ crates/nzb-web/src/error.rs | 11 ++++ crates/nzb-web/src/sabnzbd_compat.rs | 16 +---- 4 files changed, 131 insertions(+), 37 deletions(-) diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 277c1ff1..1cfe8896 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -299,24 +299,23 @@ pub async fn h_queue_list( async fn next_uploaded_file( multipart: &mut Multipart, ) -> Result)>, ApiError> { - let Some(field) = multipart - .next_field() - .await - .map_err(|error| ApiError::from(anyhow::anyhow!("Multipart error: {error}")))? - else { + let Some(field) = multipart.next_field().await.map_err(ApiError::from)? else { return Ok(None); }; let file_name = field .file_name() .map(str::to_string) .unwrap_or_else(|| "unknown.nzb".to_string()); - let data = field - .bytes() - .await - .map_err(|error| ApiError::from(anyhow::anyhow!("Read error: {error}")))?; + let data = field.bytes().await.map_err(ApiError::from)?; Ok(Some((file_name, data.to_vec()))) } +/// An archive that cannot be unpacked into NZBs (corrupt, empty, over the +/// size or count limits) is bad input, not a server fault. +fn bad_upload(error: anyhow::Error) -> ApiError { + ApiError::from((StatusCode::BAD_REQUEST, error)) +} + fn enqueue_nzb( state: &AppState, q: &AddNzbQuery, @@ -381,7 +380,7 @@ pub async fn h_queue_add( // exactly one NZB — a multi-NZB upload has no single job to replay. let mut uploaded_nzbs = Vec::new(); while let Some((file_name, data)) = next_uploaded_file(&mut multipart).await? { - uploaded_nzbs.extend(extract_nzbs(&file_name, &data).map_err(ApiError::from)?); + uploaded_nzbs.extend(extract_nzbs(&file_name, &data).map_err(bad_upload)?); } if uploaded_nzbs.len() != 1 { return Err(ApiError::bad_request( @@ -399,7 +398,7 @@ pub async fn h_queue_add( } else { while let Some((file_name, data)) = next_uploaded_file(&mut multipart).await? { // Extract NZBs (handles zip/gz/bz2 archives or plain .nzb) - for (nzb_name, nzb_data) in extract_nzbs(&file_name, &data).map_err(ApiError::from)? { + for (nzb_name, nzb_data) in extract_nzbs(&file_name, &data).map_err(bad_upload)? { nzo_ids.push(enqueue_nzb(&state, &q, &nzb_name, nzb_data, None)?); } } @@ -505,7 +504,7 @@ pub async fn h_queue_add_url( Json(body): Json, ) -> Result { if body.url.is_empty() { - return Err(ApiError::from(anyhow::anyhow!("No URL provided"))); + return Err(ApiError::bad_request("No URL provided")); } let fetch_plan = validate_fetch_url_with(&body.url, &fetch_policy(&state)).await?; @@ -547,7 +546,7 @@ pub async fn h_queue_add_url( priority: body.priority, }; - let nzbs = extract_nzbs(&file_name, &data).map_err(ApiError::from)?; + let nzbs = extract_nzbs(&file_name, &data).map_err(bad_upload)?; let mut nzo_ids = Vec::new(); for (nzb_name, nzb_data) in nzbs { let id = enqueue_nzb(&state, &q, &nzb_name, nzb_data, None)?; @@ -727,8 +726,9 @@ pub async fn h_history_retry( job_id } HistoryRetryOutcome::NoNzbData => { - return Err(ApiError::from(anyhow::anyhow!( - "No NZB data stored for this entry" + return Err(ApiError::from(( + StatusCode::CONFLICT, + "No NZB data stored for this entry", ))); } }; @@ -1457,7 +1457,7 @@ pub async fn h_rss_item_download( let url = item .url .as_ref() - .ok_or_else(|| ApiError::from(anyhow::anyhow!("No download URL for this item")))?; + .ok_or_else(|| ApiError::from((StatusCode::CONFLICT, "No download URL for this item")))?; // Fetch the NZB let fetch_plan = validate_fetch_url_with(url, &fetch_policy(&state)).await?; @@ -1481,8 +1481,7 @@ pub async fn h_rss_item_download( let data = read_response_bytes_limited(response, MAX_FETCH_BODY_BYTES).await?; - let mut job = nzb_parser::parse_nzb(&item.title, &data) - .map_err(|e| ApiError::from(anyhow::anyhow!("Failed to parse NZB: {e}")))?; + let mut job = nzb_parser::parse_nzb(&item.title, &data).map_err(ApiError::from)?; // Use the item's category or feed category if let Some(ref cat) = item.category { @@ -1968,7 +1967,10 @@ pub async fn h_browse_directory( let dir = std::path::Path::new(&path); if !dir.is_dir() { - return Err(ApiError::from(anyhow::anyhow!("Not a directory: {path}"))); + return Err(ApiError::from(( + StatusCode::BAD_REQUEST, + format!("Not a directory: {path}"), + ))); } let parent = dir.parent().map(|p| p.to_string_lossy().to_string()); @@ -2116,13 +2118,10 @@ pub async fn h_import_sabnzbd_ini(mut multipart: Multipart) -> Result for ApiError { } } +impl From for ApiError { + /// A multipart body that cannot be read is the client's fault: axum + /// reports 400 for a malformed body and 413 for one over the limit. + fn from(e: axum::extract::multipart::MultipartError) -> Self { + Self { + status: Some(e.status()), + kind: ApiErrorKind::Message(format!("Multipart error: {}", e.body_text())), + } + } +} + impl From for ApiError { /// Map domain errors to the HTTP status that describes them: missing /// resources are 404, malformed input 400, conflicts 409, and only diff --git a/crates/nzb-web/src/sabnzbd_compat.rs b/crates/nzb-web/src/sabnzbd_compat.rs index 67006ade..b4325c58 100644 --- a/crates/nzb-web/src/sabnzbd_compat.rs +++ b/crates/nzb-web/src/sabnzbd_compat.rs @@ -574,11 +574,7 @@ async fn read_multipart_fields( nzbname: &mut Option, pp: &mut Option, ) -> Result<(), ApiError> { - while let Some(field) = multipart - .next_field() - .await - .map_err(|e| ApiError::from(anyhow::anyhow!("Multipart error: {e}")))? - { + while let Some(field) = multipart.next_field().await.map_err(ApiError::from)? { let field_name = field.name().unwrap_or("").to_string(); match field_name.as_str() { "mode" => { @@ -612,10 +608,7 @@ async fn read_multipart_fields( .file_name() .map(|s| s.to_string()) .unwrap_or_else(|| "unknown.nzb".into()); - let data = field - .bytes() - .await - .map_err(|e| ApiError::from(anyhow::anyhow!("Read error: {e}")))?; + let data = field.bytes().await.map_err(ApiError::from)?; *nzb_data = Some((file_name, data.to_vec())); } else if let Ok(text) = field.text().await { *name = Some(text); @@ -626,10 +619,7 @@ async fn read_multipart_fields( .file_name() .map(|s| s.to_string()) .unwrap_or_else(|| "unknown.nzb".into()); - let data = field - .bytes() - .await - .map_err(|e| ApiError::from(anyhow::anyhow!("Read error: {e}")))?; + let data = field.bytes().await.map_err(ApiError::from)?; *nzb_data = Some((file_name, data.to_vec())); } "value" | "url" => { From 9d8609e1c4eee93f34cd7aca8b95309c39c197f8 Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:18:05 -0500 Subject: [PATCH 13/33] fix(api): validate imported categories on SABnzbd import apply /api/setup/apply replaced the configured categories with the imported ones without the check a category save gets, so an unsafe name or output_dir was persisted and every later job in that category failed at enqueue with "category output path is unsafe". The apply now runs CategoryConfig::validate on every imported category first and, if any fail, returns 400 naming each offending entry and its reason without applying anything. Fixes #119 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit af2157f9bc1d2c15a7cc188efdd74d9144d7a704) --- apps/rustnzb/src/handlers.rs | 20 ++++++ .../tests/sabnzbd_import_apply_test.rs | 70 +++++++++++++++++++ 2 files changed, 90 insertions(+) diff --git a/apps/rustnzb/src/handlers.rs b/apps/rustnzb/src/handlers.rs index 1cfe8896..05eb1ce2 100644 --- a/apps/rustnzb/src/handlers.rs +++ b/apps/rustnzb/src/handlers.rs @@ -2233,6 +2233,26 @@ pub async fn h_setup_apply( )); } + // Imported categories replace the configured ones wholesale, so they get + // the same check as a category save. Otherwise an unsafe name or + // output_dir is persisted and every later job in it fails at enqueue. + let invalid: Vec = preview + .categories + .iter() + .filter_map(|category| { + category + .validate() + .err() + .map(|reason| format!("'{}': {reason}", category.name)) + }) + .collect(); + if !invalid.is_empty() { + return Err(ApiError::from(( + StatusCode::BAD_REQUEST, + format!("cannot apply: invalid categories: {}", invalid.join("; ")), + ))); + } + let config = state .update_config_with(|config| { // Convert imported servers → ServerConfig with fresh UUIDs diff --git a/apps/rustnzb/tests/sabnzbd_import_apply_test.rs b/apps/rustnzb/tests/sabnzbd_import_apply_test.rs index 6cce1645..4b00a278 100644 --- a/apps/rustnzb/tests/sabnzbd_import_apply_test.rs +++ b/apps/rustnzb/tests/sabnzbd_import_apply_test.rs @@ -125,3 +125,73 @@ async fn apply_accepts_absolute_dirs() { std::path::PathBuf::from("/downloads/incomplete") ); } + +fn preview_with_categories(categories: serde_json::Value) -> SabnzbdImportPreview { + serde_json::from_value(serde_json::json!({ + "servers": [], + "categories": categories, + "general": { + "api_key": null, + "complete_dir": null, + "incomplete_dir": null, + "speed_limit_bps": 0 + }, + "rss_feeds": [], + "warnings": [], + "skipped_fields": [] + })) + .expect("preview JSON should deserialize") +} + +/// Negative: imported categories get the same validation as a category +/// save; an unsafe name or output_dir rejects the whole apply with 400, +/// names every offending entry, and changes nothing. +#[tokio::test] +async fn apply_rejects_unsafe_imported_categories() { + let (state, _tempdir) = test_state(); + let before = state.config().categories.clone(); + let preview = preview_with_categories(serde_json::json!([ + {"name": "tv", "output_dir": "shows/tv", "post_processing": 3}, + {"name": "../escape", "output_dir": null, "post_processing": 3}, + {"name": "movies", "output_dir": "../../etc", "post_processing": 3}, + ])); + + let error = match h_setup_apply(State(state.clone()), Json(preview)).await { + Ok(_) => panic!("expected unsafe categories to be rejected"), + Err(e) => e, + }; + + assert_eq!(error.status(), StatusCode::BAD_REQUEST); + let message = error.to_string(); + assert!(message.contains("../escape"), "{message}"); + assert!(message.contains("movies"), "{message}"); + assert!(!message.contains("'tv'"), "{message}"); + let after = state.config().categories.clone(); + assert_eq!( + after.iter().map(|c| &c.name).collect::>(), + before.iter().map(|c| &c.name).collect::>(), + "rejected apply must not have replaced the categories" + ); +} + +/// Positive: safe imported categories replace the configured ones. +#[tokio::test] +async fn apply_accepts_safe_imported_categories() { + let (state, _tempdir) = test_state(); + let preview = preview_with_categories(serde_json::json!([ + {"name": "tv", "output_dir": "shows/tv", "post_processing": 3}, + {"name": "movies", "output_dir": "/srv/media/movies", "post_processing": 3}, + ])); + + h_setup_apply(State(state.clone()), Json(preview)) + .await + .expect("safe categories should be accepted"); + + let names: Vec<_> = state + .config() + .categories + .iter() + .map(|c| c.name.clone()) + .collect(); + assert_eq!(names, ["tv", "movies"]); +} From 41c13bc0e00ee02116275d7ca6fb2d975f3bbd9c Mon Sep 17 00:00:00 2001 From: xbmc4lyfe <273732874+xbmc4lyfe@users.noreply.github.com> Date: Fri, 9 Oct 2026 10:24:21 -0500 Subject: [PATCH 14/33] fix(queue): report disk-space holds and lift them when space frees (cherry picked from commit aac8b3ebdaca22445d5f9a2ece9ea9bf012ed85f) Adapted: the new PauseReason enum was merged alongside our QueueSortField from the queue-sort fix; neither replaced the other. --- .../src/app/core/models/queue.model.ts | 6 + .../queue/queue-view.component.spec.ts | 42 +++ .../features/queue/queue-view.component.ts | 37 ++- apps/rustnzb/src/handlers.rs | 29 +- apps/rustnzb/tests/disk_space_pause_api.rs | 174 ++++++++++ crates/nzb-web/src/queue_manager.rs | 310 +++++++++++++++++- crates/nzb-web/src/sabnzbd_compat.rs | 32 +- 7 files changed, 602 insertions(+), 28 deletions(-) create mode 100644 apps/rustnzb/tests/disk_space_pause_api.rs diff --git a/apps/rustnzb/frontend/src/app/core/models/queue.model.ts b/apps/rustnzb/frontend/src/app/core/models/queue.model.ts index bd8a1456..73bd5cda 100644 --- a/apps/rustnzb/frontend/src/app/core/models/queue.model.ts +++ b/apps/rustnzb/frontend/src/app/core/models/queue.model.ts @@ -6,6 +6,9 @@ export interface ServerArticleStats { bytes_downloaded: number; } +/** Why a job or the whole queue is paused; `disk_space` lifts automatically. */ +export type PauseReason = 'manual' | 'global' | 'disk_space' | 'server'; + export interface NzbJob { id: string; name: string; @@ -24,6 +27,7 @@ export interface NzbJob { speed_bps: number; error_message: string | null; server_stats: ServerArticleStats[]; + pause_reason?: PauseReason | null; } export interface QueueResponse { @@ -31,6 +35,7 @@ export interface QueueResponse { total: number; speed_bps: number; paused: boolean; + pause_reason?: PauseReason | null; } export interface StatusResponse { @@ -43,6 +48,7 @@ export interface StatusResponse { disk_space_total: number; min_free_space_bytes: number; paused: boolean; + pause_reason?: PauseReason | null; pause_remaining_secs: number | null; webdav_available: boolean; webdav_enabled: boolean; diff --git a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts index 172de47e..8d5e222f 100644 --- a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts +++ b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.spec.ts @@ -175,6 +175,48 @@ describe('QueueViewComponent', () => { expect(component.displayConnectionCount('backup', 2_000)).toEqual({ count: 3, recent: false }); }); + it('labels a job held by the disk-space guard', () => { + const { component } = makeComponent(); + + expect(component.statusLabel(makeJob({ status: 'paused', pause_reason: 'disk_space' }))).toBe( + 'paused · low disk', + ); + expect(component.statusLabel(makeJob({ status: 'paused', pause_reason: 'manual' }))).toBe( + 'paused', + ); + expect(component.statusLabel(makeJob({ status: 'downloading' }))).toBe('downloading'); + }); + + it('shows the disk-space reason for a global pause applied by the guard', () => { + const { component } = makeComponent({ + get: vi.fn((url: string) => + url === '/queue' + ? of({ jobs: [], total: 0, speed_bps: 0, paused: true, pause_reason: 'disk_space' }) + : of({}), + ), + }); + + component.loadQueue(); + + expect(component.pausedLabel()).toBe('Paused · low disk space'); + // A queued job caught by the global hold before the next refresh. + expect(component.statusLabel(makeJob({ status: 'queued' }))).toBe('paused · low disk'); + }); + + it('keeps the plain paused label for a user pause', () => { + const { component } = makeComponent({ + get: vi.fn((url: string) => + url === '/queue' + ? of({ jobs: [], total: 0, speed_bps: 0, paused: true, pause_reason: 'global' }) + : of({}), + ), + }); + + component.loadQueue(); + + expect(component.pausedLabel()).toBe('Paused'); + }); + it('ignores a duplicate row action while the first request is pending', () => { const action$ = new Subject(); const { component, api } = makeComponent({ diff --git a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts index 77ffd57d..1b13ad4a 100644 --- a/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts +++ b/apps/rustnzb/frontend/src/app/features/queue/queue-view.component.ts @@ -8,7 +8,7 @@ import { Observable, Subscription, catchError, finalize, forkJoin, map, of } fro import { ApiService } from '../../core/services/api.service'; import { AddNzbService } from '../../core/services/add-nzb.service'; import { PauseStateService } from '../../core/services/pause-state.service'; -import { NzbJob, QueueResponse, StatusResponse } from '../../core/models/queue.model'; +import { NzbJob, PauseReason, QueueResponse, StatusResponse } from '../../core/models/queue.model'; import { HistoryViewComponent } from '../history/history-view.component'; import { ConfirmService } from '../../shared/confirm.service'; import { IconComponent } from '../../shared/icon.component'; @@ -67,7 +67,7 @@ interface PipelineStep {
{{ speedValue() }} {{ speedUnit() }}
-
{{ paused() ? 'Paused' : 'Active · limit off' }}
+
{{ paused() ? pausedLabel() : 'Active · limit off' }}
NNTP connections
@@ -430,9 +430,16 @@ interface PipelineStep { {{ job.speed_bps > 0 ? formatSpeed(job.speed_bps) : '—' }} {{ job.speed_bps > 0 ? eta(job) : '—' }} - {{ - displayStatus(effectiveStatus(job.status)) - }} + {{ statusLabel(job) }}