Retry without browser cookies after keyring extraction failure

yt-dlp walks the default keyring collection and aborts with
`ERROR: Item does not exist!` when it reaches a stale item path before
the valid Brave "Safe Storage" entry. The profile, cookie DB, and Brave
key item are readable; the same failure reproduces outside the Flatpak.
A Secret Service backend switch (for example oo7 and gnome-keyring) can
leave such ghost paths in the collection.

Retry the probe once without --cookies-from-browser when extraction
fails and no staged handoff jar exists, and latch that state for the
session so later previews/downloads do not repeat the broken keyring
walk. Extension handoff jars still win; changing the browser preference
or restarting clears the latch. If the retry also fails, the error says
that browser cookies could not be read and were skipped.

Author: houssemko

src/extension_server.rs

@@ -474,6 +474,7 @@ fn handoff_entry(url: &str) -> Option<HandoffCookies> {
 /// Browser UA staged with a handoff's cookies, if any. Same lookup as the
 /// cookies (exact URL, then same-host fallback).
 pub(crate) fn handoff_user_agent(url: &str, browser: &str) -> Option<String> {
+    let browser = crate::video_tools::effective_cookies_browser(browser);
     let live_browser = crate::video_tools::cookies_browser_spec(browser).is_some();
     let now = SystemTime::now();
     handoff_entry(url)
@@ -530,6 +531,7 @@ fn ytdlp_spawn_identity_at(cookies_browser: &str, page_url: &str, now: SystemTim
 /// `--cookies-from-browser` matters: yt-dlp aborts the whole spawn when its
 /// keyring extraction fails, even when the jar would have worked.
 pub(crate) fn ytdlp_spawn_identity(cookies_browser: &str, page_url: &str) -> Vec<String> {
+    let cookies_browser = crate::video_tools::effective_cookies_browser(cookies_browser);
     ytdlp_spawn_identity_at(cookies_browser, page_url, SystemTime::now())
 }
 
@@ -548,6 +550,7 @@ pub(crate) fn handoff_cookie_args(url: &str) -> Vec<String> {
 /// browser source. A stale snapshot would override rotated cookies exported
 /// from the live browser, so withhold it and let gallery-dl use the export.
 pub(crate) fn live_handoff_cookie_path(url: &str, browser: &str) -> Option<PathBuf> {
+    let browser = crate::video_tools::effective_cookies_browser(browser);
     live_handoff_cookie_path_at(url, browser, SystemTime::now())
 }
 

src/preferences.rs

@@ -864,6 +864,8 @@ pub fn show(
         let grant = cookies_override_row.downgrade();
         let restart = cookies_restart_row.downgrade();
         settings.connect_changed(Some(crate::settings::key::COOKIES_BROWSER), move |s, _| {
+            // A new browser choice may have a working cookie source again.
+            crate::video_tools::clear_browser_cookies_failed();
             if let (Some(grant), Some(restart)) = (grant.upgrade(), restart.upgrade()) {
                 sync_cookies_override_row(&grant, &restart, s);
             }

src/video_spawn.rs

@@ -12,6 +12,7 @@ use crate::video_tools::{VideoError, ensure_tool_versions};
 use crate::video_types::{FetchedVideo, ProbeResult, VideoInfo};
 use gettextrs::gettext;
 use std::path::{Path, PathBuf};
+use std::process::ExitStatus;
 use std::time::Duration;
 use yt_dlp::client::deps::Libraries;
 use yt_dlp::model::Video;
@@ -45,38 +46,28 @@ pub(crate) async fn read_bounded(
     ))
 }
 
-/// Fetch one page's raw `--dump-single-json`, then parse leniently. `flat_playlist` lists collection stubs; false runs full extraction.
-pub(crate) async fn fetch_raw_dump_json(
+/// One yt-dlp spawn's captured output. Callers decide whether a failed run is
+/// retryable.
+struct SpawnOutput {
+    status: ExitStatus,
+    stdout: Vec<u8>,
+    stderr: Vec<u8>,
+}
+
+/// Run one yt-dlp process with bounded concurrent drains and timeout kill.
+async fn run_ytdlp_once(
     youtube_bin: &Path,
-    url: &str,
-    cookies_browser: &str,
-    timeout: Duration,
+    args: &[String],
     fetch_proxy: Option<&crate::net_types::ResolvedProxy>,
-    flat_playlist: bool,
-) -> Result<serde_json::Value, VideoError> {
-    let mut args = vec!["--ignore-config".to_string(), "--no-progress".to_string()];
-    if flat_playlist {
-        args.push("--flat-playlist".to_string());
-    }
-    args.push("--dump-single-json".to_string());
-    args.extend(proxy_cli_args(fetch_proxy));
-    // Extension handoff (Cloudflare bypass): staged by the handoff receiver,
-    // peeked here. The entry outlives the probe — the subtitle probe and the
-    // media download spawn yt-dlp separately and need it too; the finished
-    // download discards it. The tail ends with `-- <url>`, past which yt-dlp
-    // reads positionals.
-    args.extend(crate::extension_server::ytdlp_spawn_identity(
-        cookies_browser,
-        url,
-    ));
-    // Direct spawn with the same semantics (concurrent drain, timeout kill, stderr detail on failure).
+    timeout: Duration,
+) -> Result<SpawnOutput, VideoError> {
     let mut cmd = ytdlp_command(youtube_bin);
     // Belt-and-braces with the ProcessGroupGuard below (which SIGKILLs the
     // process group on drop): if the drain is dropped, the child dies with
     // it either way. Download/capture spawns keep their guard-only shutdown
     // behavior; never set this via the shared ytdlp_command() builder.
     cmd.kill_on_drop(true);
-    cmd.args(&args);
+    cmd.args(args);
     apply_proxy_env(&mut cmd, fetch_proxy);
     // Debug: log the full argv before spawning. as_std() captures everything
     // including ytdlp_command()'s --impersonate flag.
@@ -131,13 +122,104 @@ pub(crate) async fn fetch_raw_dump_json(
     // Debug: echo the probe's stderr (not live — the probe is short-lived,
     // so post-hoc is fine).
     crate::backend_debug::echo_stderr(&stderr);
-    if !status.success() {
-        let detail = last_log_line(&String::from_utf8_lossy(&stderr), "yt-dlp reported failure");
+    Ok(SpawnOutput {
+        status,
+        stdout,
+        stderr,
+    })
+}
+
+/// yt-dlp reports a missing keyring item as the bare secretstorage error;
+/// these are cookie-source failures a no-cookie retry can recover from.
+fn is_browser_cookie_error(stderr: &str) -> bool {
+    let stderr = stderr.to_ascii_lowercase();
+    [
+        "item does not exist",
+        "failed to extract cookies from browser",
+        "cookies database",
+        "secretstorage",
+        "failed to decrypt",
+        "could not decrypt",
+        "keyring",
+    ]
+    .iter()
+    .any(|needle| stderr.contains(needle))
+}
+
+/// True when a failed spawn has a direct browser cookie source and no staged
+/// jar to preserve, and its stderr points at cookie extraction.
+pub(crate) fn should_retry_without_cookies(args: &[String], stderr: &str) -> bool {
+    args.iter()
+        .any(|a| a.starts_with("--cookies-from-browser="))
+        && !args.iter().any(|a| a == "--cookies")
+        && is_browser_cookie_error(stderr)
+}
+
+/// Remove only the direct browser cookie flag; a staged jar, identity flags,
+/// proxy flags and the `-- <url>` tail stay intact.
+pub(crate) fn without_browser_cookies(args: &[String]) -> Vec<String> {
+    args.iter()
+        .filter(|a| !a.starts_with("--cookies-from-browser="))
+        .cloned()
+        .collect()
+}
+
+/// Fetch one page's raw `--dump-single-json`, then parse leniently. `flat_playlist` lists collection stubs; false runs full extraction.
+pub(crate) async fn fetch_raw_dump_json(
+    youtube_bin: &Path,
+    url: &str,
+    cookies_browser: &str,
+    timeout: Duration,
+    fetch_proxy: Option<&crate::net_types::ResolvedProxy>,
+    flat_playlist: bool,
+) -> Result<serde_json::Value, VideoError> {
+    let mut args = vec!["--ignore-config".to_string(), "--no-progress".to_string()];
+    if flat_playlist {
+        args.push("--flat-playlist".to_string());
+    }
+    args.push("--dump-single-json".to_string());
+    args.extend(proxy_cli_args(fetch_proxy));
+    // Extension handoff (Cloudflare bypass): staged by the handoff receiver,
+    // peeked here. The entry outlives the probe — the subtitle probe and the
+    // media download spawn yt-dlp separately and need it too; the finished
+    // download discards it. The tail ends with `-- <url>`, past which yt-dlp
+    // reads positionals.
+    args.extend(crate::extension_server::ytdlp_spawn_identity(
+        cookies_browser,
+        url,
+    ));
+    let mut retried_without_cookies = false;
+    loop {
+        let output = run_ytdlp_once(youtube_bin, &args, fetch_proxy, timeout).await?;
+        if output.status.success() {
+            let value: serde_json::Value =
+                serde_json::from_str(&String::from_utf8_lossy(&output.stdout))
+                    .map_err(VideoError::fetch)?;
+            return Ok(value);
+        }
+        let stderr = String::from_utf8_lossy(&output.stderr);
+        let detail = last_log_line(&stderr, "yt-dlp reported failure");
+        if !retried_without_cookies && should_retry_without_cookies(&args, &stderr) {
+            retried_without_cookies = true;
+            crate::video_tools::mark_browser_cookies_failed();
+            tracing::warn!(
+                host = %page_host(url),
+                error = %detail,
+                "browser cookie extraction failed; retrying without cookies"
+            );
+            args = without_browser_cookies(&args);
+            continue;
+        }
+        let detail = if retried_without_cookies {
+            format!(
+                "{detail} ({})",
+                gettext("browser cookies couldn't be read; retried without cookies")
+            )
+        } else {
+            detail
+        };
         return Err(VideoError::fetch(detail));
     }
-    let value: serde_json::Value =
-        serde_json::from_str(&String::from_utf8_lossy(&stdout)).map_err(VideoError::fetch)?;
-    Ok(value)
 }
 
 /// Strict single-video model for the download worker (full extraction; playlist output selects the picked entry or expands worker-side).

src/video_tests.rs

@@ -41,7 +41,7 @@ use crate::video_runner::{
 };
 use crate::video_spawn::{
     ProcessGroupGuard, await_group_quiescence, fetch_raw_dump_json, fetch_video_page, reap_child,
-    ytdlp_command,
+    should_retry_without_cookies, without_browser_cookies, ytdlp_command,
 };
 use crate::video_staging::{
     ResumePlan, ResumeQuery, VideoManifest, clean_dest_parts, clean_staging, collect_sidecar,
@@ -6769,6 +6769,132 @@ fn fetch_video_page_surfaces_stderr_tail() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+#[test]
+fn browser_cookie_retry_only_recovers_extraction_failures() {
+    let args = vec![
+        "--cookies-from-browser=brave:/profile".to_string(),
+        "--".to_string(),
+        "https://example.com/".to_string(),
+    ];
+    for stderr in [
+        "ERROR: Item does not exist!",
+        "ERROR: could not find brave cookies database in \"/home/u/.config\"",
+        "ERROR: Failed to extract cookies from browser. Check '--cookies-from-browser'",
+        "ERROR: secretstorage is not installed",
+        "ERROR: Failed to decrypt with DPAPI",
+        "ERROR: Could not decrypt cookie file",
+        "ERROR: keyring is locked",
+    ] {
+        assert!(
+            should_retry_without_cookies(&args, stderr),
+            "expected a retry for {stderr}"
+        );
+    }
+    for stderr in [
+        "ERROR: [youtube] Sign in to confirm you're not a bot. Use --cookies-from-browser",
+        "ERROR: Video unavailable",
+    ] {
+        assert!(
+            !should_retry_without_cookies(&args, stderr),
+            "unexpected retry for {stderr}"
+        );
+    }
+}
+
+#[test]
+fn browser_cookie_retry_preserves_a_staged_jar() {
+    let args = vec![
+        "--cookies-from-browser=brave:/profile".to_string(),
+        "--cookies".to_string(),
+        "/tmp/grab-cookies.jar".to_string(),
+        "--".to_string(),
+        "https://example.com/".to_string(),
+    ];
+    assert!(!should_retry_without_cookies(
+        &args,
+        "ERROR: Item does not exist!"
+    ));
+}
+
+#[test]
+fn browser_cookie_retry_drops_only_the_browser_flag() {
+    let args = vec![
+        "--ignore-config".to_string(),
+        "--cookies-from-browser=brave:/profile".to_string(),
+        "--user-agent".to_string(),
+        "Grab/1".to_string(),
+        "--".to_string(),
+        "https://example.com/".to_string(),
+    ];
+    assert_eq!(
+        without_browser_cookies(&args),
+        vec![
+            "--ignore-config".to_string(),
+            "--user-agent".to_string(),
+            "Grab/1".to_string(),
+            "--".to_string(),
+            "https://example.com/".to_string(),
+        ]
+    );
+}
+
+#[test]
+fn browser_cookie_keyring_failure_retries_without_cookies() {
+    crate::video_tools::clear_browser_cookies_failed();
+    let dir = std::env::temp_dir().join(format!("grab-cookie-retry-{}", std::process::id()));
+    let _ = std::fs::remove_dir_all(&dir);
+    std::fs::create_dir_all(&dir).unwrap();
+    let log = dir.join("args.log");
+    let bin = dir.join("fake-ytdlp");
+    std::fs::write(
+        &bin,
+        probe_guard(&format!(
+            "#!/bin/sh\n\
+             for a in \"$@\"; do printf '%s\\n' \"$a\" >> \"{log}\"; done\n\
+             printf '%s\\n' '---' >> \"{log}\"\n\
+             for a in \"$@\"; do\n\
+               case \"$a\" in --cookies-from-browser=*) echo 'ERROR: Item does not exist!' >&2; exit 1;; esac\n\
+             done\n\
+             echo '{{\"title\":\"ok\"}}'\n",
+            log = log.display()
+        )),
+    )
+    .unwrap();
+    #[cfg(unix)]
+    {
+        use std::os::unix::fs::PermissionsExt as _;
+        std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).unwrap();
+    }
+    let res = crate::runtime::tokio_rt().block_on(fetch_raw_dump_json(
+        &bin,
+        "https://cookie-retry.invalid/v",
+        "brave",
+        std::time::Duration::from_secs(5),
+        None,
+        false,
+    ));
+    assert!(res.is_ok(), "retry should recover: {res:?}");
+    let calls = std::fs::read_to_string(&log).unwrap();
+    let mut attempts = calls.split("---").filter(|part| !part.trim().is_empty());
+    let first = attempts.next().expect("first attempt");
+    let retry = attempts.next().expect("retry attempt");
+    assert!(
+        first.contains("--cookies-from-browser="),
+        "first attempt must use browser cookies: {first}"
+    );
+    assert!(
+        !retry.contains("--cookies-from-browser="),
+        "retry must drop browser cookies: {retry}"
+    );
+    assert_eq!(
+        crate::video_tools::effective_cookies_browser("brave"),
+        "none",
+        "the session latch must keep later spawns cookie-free"
+    );
+    crate::video_tools::clear_browser_cookies_failed();
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn fetch_video_page_rejects_garbage_stdout() {
     // Zero exit with non-JSON stdout is a parse error, not a phantom video.

src/video_tools.rs

@@ -6,6 +6,7 @@ use crate::download_net::tool_client_builder_proxied;
 use gettextrs::gettext;
 use std::collections::HashMap;
 use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Mutex, OnceLock};
 use thiserror::Error;
 use yt_dlp::client::deps::Libraries;
@@ -1611,6 +1612,32 @@ pub(crate) fn cookies_browser_spec(value: &str) -> Option<String> {
     Some(ytdlp_browser.to_string())
 }
 
+/// One-way per-session latch: once direct browser-cookie extraction fails
+/// (e.g. a dangling keyring item), later spawns skip browser cookies so a
+/// preview or download can still run. A staged handoff jar still wins.
+/// Cleared when the browser preference changes or the app restarts.
+static BROWSER_COOKIES_FAILED: AtomicBool = AtomicBool::new(false);
+
+/// Mark direct browser-cookie extraction as failed for the rest of the session.
+pub(crate) fn mark_browser_cookies_failed() {
+    BROWSER_COOKIES_FAILED.store(true, Ordering::Relaxed);
+}
+
+/// Allow direct browser cookies again after the user picks a browser.
+pub(crate) fn clear_browser_cookies_failed() {
+    BROWSER_COOKIES_FAILED.store(false, Ordering::Relaxed);
+}
+
+/// Browser value for spawns: `none` once this session has seen a direct
+/// cookie-extraction failure, so the broken source is not retried per spawn.
+pub(crate) fn effective_cookies_browser(browser: &str) -> &str {
+    if BROWSER_COOKIES_FAILED.load(Ordering::Relaxed) && browser != "none" {
+        "none"
+    } else {
+        browser
+    }
+}
+
 /// Shared trailing argv for every yt-dlp spawn: player-client workaround, JS
 /// runtime pin for YouTube, cookies, user agent, then the page URL
 /// behind `--`. One helper so these flags cannot drift between spawns (or let