From 923fd3273f723143d3ff8bef627b6f85047a83d3 Mon Sep 17 00:00:00 2001 From: vincent Date: Sat, 29 Aug 2026 14:29:14 +0200 Subject: [PATCH] fix: a failed subtitle no longer destroys the download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults behind the 429 in the report. The subtitle languages were a wildcard: 'en.*' also matches every machine-translated variant YouTube offers — en-en-US and dozens more — so each download fired a burst of subtitle requests and earned an HTTP 429. Languages are now named exactly, as ',-orig'. Turning subtitles off now really does request none; it previously still asked for English. Worse, yt-dlp exits non-zero if anything at all failed, and a caption it could not fetch was enough to mark a fully downloaded video as failed and leave the file orphaned. yt-dlp now runs with --ignore-errors, and success is judged by whether the media file actually landed rather than by the exit code. 429 also gets its own message pointing at the sign-in setting, which raises the limit. --- src-tauri/src/commands.rs | 23 ++++++---- src-tauri/src/downloader.rs | 88 ++++++++++++++++++++++++++++++------- src/App.tsx | 7 +-- 3 files changed, 89 insertions(+), 29 deletions(-) diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index 86dcfaa..98b0f2d 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -121,6 +121,11 @@ fn explain_yt_dlp_error(stderr: &str, signed_in: bool) -> String { .into() }; } + if stderr.contains("429") || stderr.contains("Too Many Requests") { + return "YouTube is rate-limiting this machine. Wait a few minutes, or sign in \ + under Settings → Sign in to YouTube, which raises the limit." + .into(); + } if stderr.contains("Operation not permitted") && stderr.contains("Safari") { return "macOS blocked access to Safari's cookies. Give FlightTube Full Disk Access in System Settings → Privacy & Security, or pick a different browser." .into(); @@ -1038,15 +1043,15 @@ pub async fn download_video( let stderr_lines = stderr_task.await.unwrap_or_default(); drop(permit); - if status.success() { - // yt-dlp normally reports the path via `--print after_move:`; if that - // line went missing, find the file it wrote by its embedded video id. - let path = match final_path { - Some(p) => p, - None => find_by_video_id(&library, &video_id) - .await - .ok_or("Download finished but the file could not be located.")?, - }; + // yt-dlp exits non-zero if *anything* failed, including a subtitle it could + // not fetch. If the video itself landed, the download succeeded — throwing + // away a finished file over a missing caption would be absurd. + let landed = match final_path { + Some(p) if tokio::fs::metadata(&p).await.is_ok() => Some(p), + _ => find_by_video_id(&library, &video_id).await, + }; + + if let Some(path) = landed { // YouTube's captions arrive pinned to the left edge and full of // karaoke timing tags; clean them before they reach the player. tidy_subtitles(&library, &video_id).await; diff --git a/src-tauri/src/downloader.rs b/src-tauri/src/downloader.rs index ab099ce..58e5b9d 100644 --- a/src-tauri/src/downloader.rs +++ b/src-tauri/src/downloader.rs @@ -79,13 +79,15 @@ pub fn parse_progress_line(line: &str) -> Option { /// Arguments for downloading one video. Kept separate from process spawning so /// the argument construction is assertable in tests. +/// `sub_langs` is empty when subtitles are switched off, in which case none +/// are requested at all. pub fn build_args( video_id: &str, out_template: &str, quality: &str, sub_langs: &str, ) -> Vec { - vec![ + let mut args = vec![ "-f".into(), format_selector(quality), "--merge-output-format".into(), @@ -99,24 +101,50 @@ pub fn build_args( "180".into(), "--progress-template".into(), PROGRESS_TEMPLATE.into(), - // Subtitles come along for offline use, including YouTube's - // auto-generated ones. They are written beside the video as WebVTT - // rather than muxed in: WebKit reads a reliably, whereas - // subtitle streams inside an MP4 it largely ignores. - "--write-subs".into(), - "--write-auto-subs".into(), - "--sub-format".into(), - "vtt".into(), - "--convert-subs".into(), - "vtt".into(), - "--sub-langs".into(), - sub_langs.into(), + // A failing subtitle must never take the video with it. Subtitles are + // a bonus; the file is the point. + "--ignore-errors".into(), "--print".into(), "after_move:FTPATH %(filepath)s".into(), "-o".into(), out_template.into(), - format!("https://www.youtube.com/watch?v={video_id}"), - ] + ]; + + if !sub_langs.is_empty() { + // Subtitles come along for offline use, including YouTube's + // auto-generated ones. WebVTT beside the video rather than muxed in: + // WebKit reads a reliably and largely ignores subtitle streams + // inside an MP4. + // + // The languages are named exactly. A wildcard like "en.*" also matches + // every machine-translated variant YouTube offers — en-en-US, en-de and + // dozens more — and asking for all of them earns an HTTP 429. + args.extend([ + "--write-subs".into(), + "--write-auto-subs".into(), + "--sub-format".into(), + "vtt".into(), + "--convert-subs".into(), + "vtt".into(), + "--sub-langs".into(), + sub_langs.to_string(), + ]); + } + + args.push(format!("https://www.youtube.com/watch?v={video_id}")); + args +} + +/// The subtitle languages to request for a preference, or empty for none. +/// +/// Only the language itself and YouTube's "-orig" variant; anything broader +/// pulls in machine translations by the dozen. +pub fn sub_langs_for(pref: &str) -> String { + if pref.is_empty() || pref == "off" { + String::new() + } else { + format!("{pref},{pref}-orig") + } } /// yt-dlp reports the final path via `--print after_move:`, which is more @@ -194,15 +222,41 @@ mod tests { #[test] fn subtitles_are_requested_including_auto_generated() { - let args = build_args("abc", "/tmp/o.%(ext)s", "best", "en.*,nl.*"); + let args = build_args("abc", "/tmp/o.%(ext)s", "best", "en,en-orig"); assert!(args.contains(&"--write-subs".to_string())); // The auto-generated track is the only one many videos have. assert!(args.contains(&"--write-auto-subs".to_string())); - assert!(args.contains(&"en.*,nl.*".to_string())); + assert!(args.contains(&"en,en-orig".to_string())); // WebVTT, because that is what a element can load. assert!(args.contains(&"vtt".to_string())); } + #[test] + fn no_preference_means_no_subtitle_requests_at_all() { + let args = build_args("abc", "/tmp/o.%(ext)s", "best", ""); + assert!(!args.contains(&"--write-subs".to_string())); + assert!(!args.contains(&"--write-auto-subs".to_string())); + assert!(!args.contains(&"--sub-langs".to_string())); + } + + #[test] + fn a_subtitle_failure_must_not_abort_the_video() { + // yt-dlp aborts the whole job on the first error without this. + assert!(build_args("abc", "/tmp/o.%(ext)s", "best", "en,en-orig") + .contains(&"--ignore-errors".to_string())); + } + + #[test] + fn languages_are_named_exactly_never_as_a_wildcard() { + // "en.*" also matches en-en-US and dozens of machine translations, + // and requesting them all earns an HTTP 429. + let langs = sub_langs_for("nl"); + assert_eq!(langs, "nl,nl-orig"); + assert!(!langs.contains('*')); + assert_eq!(sub_langs_for("off"), ""); + assert_eq!(sub_langs_for(""), ""); + } + #[test] fn best_quality_takes_the_highest_available() { let args = build_args("abc123", "/tmp/out.%(ext)s", "best", "en.*"); diff --git a/src/App.tsx b/src/App.tsx index a28a74d..218a255 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -168,9 +168,10 @@ export default function App() { [channels], ); - // yt-dlp wants a pattern; "en.*" catches "en" and "en-orig" and the - // auto-generated "en" track alike. English is always fetched as a fallback. - const subLangArg = subLang === "off" ? "en.*" : `${subLang}.*,en.*`; + // Named exactly, never as a wildcard: "en.*" also matches every + // machine-translated variant YouTube offers, and asking for all of them + // earns an HTTP 429. Off means no subtitle requests at all. + const subLangArg = subLang === "off" ? "" : `${subLang},${subLang}-orig`; const doRefresh = useCallback(async () => { setRefreshing(true);