diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 58a375b..8e305fc 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1175,10 +1175,9 @@ pub(crate) async fn start_media_download_internal( let path_env = format!("{}:/usr/bin:/bin", bin_dir_str); let mut strike = 0_usize; - let mut terminal_failure = false; let mut processing_started = false; - 'retry: while strike <= MAX_RETRIES { + while strike <= MAX_RETRIES { let mut cmd = app_handle.shell().sidecar("yt-dlp").map_err(|e| e.to_string())? .arg("--newline") .arg("--no-check-formats") @@ -1252,9 +1251,7 @@ pub(crate) async fn start_media_download_internal( log::info!("yt-dlp spawned for id: {} (strike {})", id, strike); let mut stderr_tail = String::new(); - let mut failure_reason: Option = None; - - loop { + let failure_reason = loop { tokio::select! { _ = cancel_rx.changed() => { let _ = child.kill(); @@ -1336,49 +1333,37 @@ pub(crate) async fn start_media_download_internal( } Some(tauri_plugin_shell::process::CommandEvent::Error(err)) => { log::error!("yt-dlp shell error [{}]: {}", id, err); - failure_reason = Some(err); - break; + break err; } Some(tauri_plugin_shell::process::CommandEvent::Terminated(payload)) => { if payload.code == Some(0) { log::info!("yt-dlp completed successfully for id: {}", id); - let _ = app_handle.emit("download-complete", id.to_string()); - use tauri_plugin_notification::NotificationExt; - let _ = app_handle.notification().builder().title("Download Complete").body(&safe_filename).show(); return Ok(()); } log::error!("yt-dlp exited with non-zero code {:?} for id: {}", payload.code, id); - failure_reason = Some(if stderr_tail.is_empty() { + break if stderr_tail.is_empty() { format!("yt-dlp exited with code {:?}", payload.code) } else { stderr_tail.clone() - }); - break; + }; } Some(_) => {} None => { - failure_reason = Some(if stderr_tail.is_empty() { + break if stderr_tail.is_empty() { "yt-dlp process ended unexpectedly".to_string() } else { stderr_tail.clone() - }); - break; + }; } } } } - } - - let failure_reason = match failure_reason { - Some(reason) => reason, - None => return Ok(()), }; let transient = is_transient_network_error(&failure_reason); let strikes_left = strike < MAX_RETRIES; if !(transient && strikes_left) { - terminal_failure = true; - break 'retry; + return Err(failure_reason); } let reason = failure_reason.clone(); @@ -1396,17 +1381,13 @@ pub(crate) async fn start_media_download_internal( .await; if outcome == BackoffOutcome::Aborted { - return Ok(()); + return Err(crate::queue::MEDIA_RUN_CANCELLED.to_string()); } strike += 1; } - if terminal_failure { - let _ = app_handle.emit("download-failed", id.to_string()); - } - - Ok(()) + Err("yt-dlp retry loop exhausted".to_string()) } #[tauri::command] diff --git a/src-tauri/tests/queue_manager.rs b/src-tauri/tests/queue_manager.rs index 6140144..36489b8 100644 --- a/src-tauri/tests/queue_manager.rs +++ b/src-tauri/tests/queue_manager.rs @@ -1,8 +1,11 @@ -use firelink_lib::queue::{QueueManager, SpawnPayload, TaskKind, QueuedTask}; +use firelink_lib::queue::{ + QueueManager, QueuedTask, SidecarSpawner, SpawnPayload, TaskKind, MEDIA_RUN_CANCELLED, +}; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; use std::time::Duration; use tauri::test::{mock_builder, mock_context, noop_assets}; +use tauri::Listener; use tokio::time::timeout; /// A fake spawner that records calls and lets tests gate sidecar lifetime. @@ -211,6 +214,98 @@ fn aria2_task(id: &str) -> QueuedTask { } } +fn media_task(id: &str) -> QueuedTask { + QueuedTask { + id: id.to_string(), + kind: TaskKind::Media, + payload: SpawnPayload::default(), + } +} + +struct FixedMediaSpawner { + outcome: Result<(), String>, +} + +#[async_trait::async_trait] +impl SidecarSpawner for FixedMediaSpawner { + async fn add_uri(&self, _id: &str, _payload: &SpawnPayload) -> Result { + unreachable!("aria2 is not used by media terminal-state tests") + } + + async fn run_media(&self, _id: &str, _payload: &SpawnPayload) -> Result<(), String> { + self.outcome.clone() + } + + async fn run_native(&self, _id: &str, _payload: &SpawnPayload) -> Result<(), String> { + unreachable!("native downloads are not used by media terminal-state tests") + } +} + +fn make_media_manager( + outcome: Result<(), String>, +) -> ( + QueueManager, + std::sync::mpsc::Receiver, +) { + let app = mock_builder() + .build(mock_context(noop_assets())) + .expect("mock app"); + let (event_tx, event_rx) = std::sync::mpsc::channel(); + app.handle().listen("download-state", move |event| { + let _ = event_tx.send(event.payload().to_string()); + }); + let spawner: Arc = Arc::new(FixedMediaSpawner { outcome }); + let manager = QueueManager::test_new(app.handle().clone(), 1, spawner); + (manager, event_rx) +} + +fn emitted_statuses(event_rx: &std::sync::mpsc::Receiver) -> Vec { + event_rx + .try_iter() + .filter_map(|payload| serde_json::from_str::(&payload).ok()) + .filter_map(|payload| payload.get("status")?.as_str().map(str::to_string)) + .collect() +} + +#[tokio::test] +async fn media_terminal_error_emits_failed_without_completed() { + let (manager, event_rx) = make_media_manager(Err("terminal media failure".to_string())); + let manager = Arc::new(manager); + manager.push(media_task("media-failed")).await; + let dispatcher = { + let manager = Arc::clone(&manager); + tokio::spawn(async move { manager.run_dispatcher().await }) + }; + + tokio::time::sleep(Duration::from_millis(100)).await; + let statuses = emitted_statuses(&event_rx); + assert!(statuses.iter().any(|status| status == "failed")); + assert!(!statuses.iter().any(|status| status == "completed")); + assert_eq!(manager.available_permits(), 1); + + dispatcher.abort(); +} + +#[tokio::test] +async fn media_cancellation_does_not_emit_completed() { + let (manager, event_rx) = + make_media_manager(Err(MEDIA_RUN_CANCELLED.to_string())); + let manager = Arc::new(manager); + manager.push(media_task("media-cancelled")).await; + let dispatcher = { + let manager = Arc::clone(&manager); + tokio::spawn(async move { manager.run_dispatcher().await }) + }; + + tokio::time::sleep(Duration::from_millis(100)).await; + let statuses = emitted_statuses(&event_rx); + assert!(!statuses.iter().any(|status| status == "failed")); + assert!(!statuses.iter().any(|status| status == "completed")); + assert_eq!(manager.available_permits(), 1); + + dispatcher.abort(); +} + #[tokio::test] async fn aria2_permit_survives_rpc_return() { let (mgr, spawner) = make_manager(1); diff --git a/src/App.tsx b/src/App.tsx index a742997..cd995e0 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -1,4 +1,4 @@ -import { initMediaDomains } from './utils/downloads'; +import { initMediaDomains, isActiveDownloadStatus } from './utils/downloads'; import { useEffect, useRef, useState } from "react"; import { Sidebar, SidebarFilter } from "./components/Sidebar"; import { DownloadTable } from "./components/DownloadTable"; @@ -134,7 +134,7 @@ function App() { useEffect(() => { if (!schedulerRunning) return; const hasPendingScheduledWork = downloads.some(download => - download.status === 'queued' || download.status === 'downloading' + isActiveDownloadStatus(download.status) ); if (hasPendingScheduledWork) return; @@ -209,26 +209,20 @@ function App() { useEffect(() => { initDownloadListener(); - const unlistenComplete = listen('download-complete', (event) => { + const unlistenTerminalState = listen('download-state', (event) => { + if (event.payload.status !== 'completed' && event.payload.status !== 'failed') return; const settings = useSettingsStore.getState(); - if (settings.showNotifications) { - const item = useDownloadStore.getState().downloads.find(d => d.id === event.payload); - const fileName = item?.fileName || 'A file'; - + if (!settings.showNotifications) return; + + const item = useDownloadStore.getState().downloads.find(d => d.id === event.payload.id); + const fileName = item?.fileName || 'A file'; + if (event.payload.status === 'completed') { sendNotification({ title: 'Download Complete', body: `${fileName} has finished downloading.`, sound: settings.playCompletionSound ? 'default' : undefined }); - } - }); - - const unlistenFailed = listen('download-failed', (event) => { - const settings = useSettingsStore.getState(); - if (settings.showNotifications) { - const item = useDownloadStore.getState().downloads.find(d => d.id === event.payload); - const fileName = item?.fileName || 'A file'; - + } else { sendNotification({ title: 'Download Failed', body: `${fileName} failed to download.`, @@ -265,8 +259,7 @@ function App() { return () => { invoke('set_extension_frontend_ready', { ready: false }).catch(() => {}); - unlistenComplete.then(f => f()); - unlistenFailed.then(f => f()); + unlistenTerminalState.then(f => f()); unlistenExtension.then(f => f()); unlistenExtensionQueued.then(f => f()); unlistenDeepLink.then(f => f()); diff --git a/src/ipc.ts b/src/ipc.ts index 2ba5f6c..a1ff9f3 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -4,6 +4,7 @@ import { listen as tauriListen, type Event, type EventCallback, type UnlistenFn import type { DownloadCategory } from './bindings/DownloadCategory'; import type { DownloadItem } from './bindings/DownloadItem'; import type { DownloadProgressEvent } from './bindings/DownloadProgressEvent'; +import type { DownloadStateEvent } from './bindings/DownloadStateEvent'; import type { DownloadStatus } from './bindings/DownloadStatus'; import type { ExtensionDownload } from './bindings/ExtensionDownload'; import type { MediaCookieSource } from './bindings/MediaCookieSource'; @@ -123,6 +124,7 @@ export function invokeCommand( type EventMap = { 'schedule-trigger': 'start' | 'stop'; 'download-progress': DownloadProgressEvent; + 'download-state': DownloadStateEvent; 'download-complete': string; 'download-failed': string; 'extension-add-download': ExtensionDownload; diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index e1ff984..35190d3 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -10,6 +10,7 @@ import type { ExtensionDownload } from '../bindings/ExtensionDownload'; import type { Queue } from '../bindings/Queue'; import type { MediaMetadata } from '../bindings/MediaMetadata'; import { useSettingsStore } from './useSettingsStore'; +import { isActiveDownloadStatus } from '../utils/downloads'; export type { DownloadCategory } from '../utils/downloads'; @@ -533,10 +534,10 @@ export const useDownloadStore = create((set, get) => ({ downloads: downloads.length > 0 ? downloads : state.downloads })); - // Reset interrupted downloads (crashed while downloading/processing) to queued + // Reset interrupted active downloads to queued. set((state) => ({ downloads: state.downloads.map(d => - d.status === 'downloading' || d.status === 'processing' + isActiveDownloadStatus(d.status) && d.status !== 'queued' ? { ...d, status: 'queued' as const } : d ) diff --git a/src/utils/downloads.ts b/src/utils/downloads.ts index 8f732d2..3fee6ae 100644 --- a/src/utils/downloads.ts +++ b/src/utils/downloads.ts @@ -1,4 +1,5 @@ import type { DownloadCategory } from '../bindings/DownloadCategory'; +import type { DownloadStatus } from '../bindings/DownloadStatus'; export type { DownloadCategory } from '../bindings/DownloadCategory'; import { invoke } from '@tauri-apps/api/core'; @@ -19,6 +20,16 @@ let MEDIA_DOMAINS = [ 'fb.watch' ]; +const ACTIVE_DOWNLOAD_STATUSES: ReadonlySet = new Set([ + 'queued', + 'downloading', + 'processing', + 'retrying', +]); + +export const isActiveDownloadStatus = (status: DownloadStatus): boolean => + ACTIVE_DOWNLOAD_STATUSES.has(status); + export const initMediaDomains = async () => { try { const domains = await invoke('get_supported_media_domains');