From 68a6eecfbe3583f076d08cf38ddaa8b3ed784d18 Mon Sep 17 00:00:00 2001 From: dailz Date: Sun, 21 Jun 2026 10:44:15 +0800 Subject: [PATCH] fix(cap_portal): phased timeouts + token-aware retry for Portal setup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 2 of Portal resilience. When xdg-desktop-portal is stuck, setup_portal now fails fast with actionable diagnostics instead of hanging indefinitely. Auto-recovers from stale restore token case. Phased timeouts (per Oracle review): Phase 1: Screencast proxy creation 5s (no user interaction) Phase 2: create_session 5s (no user interaction) Phase 3: select_sources 5s with token / 30s without Phase 4: start + response 5s with token / 30s without Phase 5: open_pipe_wire_remote 5s (no user interaction) Phase 3/4 timeout depends on whether restore token was loaded: - With valid token: no permission dialog expected, 5s - Without token: user must click Allow in dialog, allow 30s Token-aware retry (Oracle B'): On timeout in phase 3 or 4 IF restore token was in use: 1. Log warning explaining auto-recovery 2. Delete cached token (~/.cache/wl-webrtc/portal-restore-token) 3. Retry whole setup_portal once with no_persist=true behavior 4. On second failure: exit with diagnostic Retry is whole-flow (new Screencast proxy, new session). Does NOT reuse half-created objects — Oracle warned this can leak state. Diagnostic messages: Service-side timeout (phases 1, 2, 5, or phase 3/4 without token): 'Portal service did not respond within timeout while . Try: systemctl --user restart xdg-desktop-portal xdg-desktop-portal-kde, then re-run wl-webrtc.' Token-side timeout (phase 3/4 with token, after auto-retry exhausted): Same message + ' If this recurs, try: wl-webrtc --no-persist' Implementation: - PortalPhaseTimeout enum distinguishes Service vs TokenDependent failures (only TokenDependent triggers retry) - _setup_portal_inner does the actual phased work with timeouts - setup_portal wraps inner, handles retry on TokenDependent - log_portal_phase_timeout helper for consistent diagnostics - delete_restore_token for safe token removal (concurrent-instance safe) Tests: - cargo build --release: 0 new warnings (19 baseline preserved) - cargo test: 97 lib + 3 integration, 0 failed - All 7 existing token tests pass unchanged - SAFETY comments preserved verbatim - 1 file changed, +199/-22 lines Out of scope (Oracle deferred): - --doctor diagnostic CLI subcommand - Runtime watchdog (Portal going bad mid-session) - systemd auto-restart (disrupts other Portal clients) - Phased diagnostics for the optional PipeWire first-frame wait Combined with Phase 1 (backend_detect.rs, commit 6ccb225), Portal service issues now fail fast with clear recovery instructions instead of hanging indefinitely. --- src/cap_portal.rs | 221 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 199 insertions(+), 22 deletions(-) diff --git a/src/cap_portal.rs b/src/cap_portal.rs index 5e1b0c0..176a94b 100644 --- a/src/cap_portal.rs +++ b/src/cap_portal.rs @@ -23,6 +23,69 @@ use tokio::runtime::Runtime; use crate::args::Args; +/// Timeout for Portal phases that do not require user interaction. +/// Applies to: Screencast proxy creation, session creation, source selection +/// (when a restore token is used), session start (token path), and PipeWire +/// remote fd acquisition. +/// +/// 5 seconds is generous for healthy xdg-desktop-portal (typically <500ms) +/// but bounded enough that a stuck service fails fast. +const PORTAL_SERVICE_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(5); + +/// Timeout for Portal phases that may require a user to click "Allow" in a +/// desktop dialog. Applies to: source selection and session start when NO +/// restore token is available (fresh authorization). +/// +/// 30 seconds gives the user time to find and click the dialog without +/// causing a spurious timeout failure. +const PORTAL_USER_DIALOG_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(30); + +/// Classification of Portal phase timeouts to drive retry behavior. +/// +/// `TokenDependent` failures are eligible for a single automatic retry that +/// clears the cached restore token and re-runs the entire Portal setup with +/// fresh authorization. `Service` failures are not retried automatically +/// (the user should restart the Portal service instead). +#[derive(Debug)] +enum PortalPhaseTimeout { + /// Portal service unresponsive in a phase not related to restore token. + /// No automatic retry; user should restart Portal service. + Service, + /// Portal timed out in a token-dependent phase. Retry eligible: clear + /// cached token and try once more with fresh authorization. + TokenDependent, +} + +impl std::fmt::Display for PortalPhaseTimeout { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Service => write!(f, "Portal phase timed out (service)"), + Self::TokenDependent => write!(f, "Portal phase timed out (token-dependent)"), + } + } +} + +impl std::error::Error for PortalPhaseTimeout {} + +/// Log an actionable diagnostic when a Portal phase times out. +/// +/// Mirrors the message format from `backend_detect.rs::log_portal_unresponsive` +/// but additionally suggests `--no-persist` when the timeout occurred in a +/// phase that was using a restore token. +fn log_portal_phase_timeout(phase: &str, used_restore_token: bool) { + let persist_hint = if used_restore_token { + " If this recurs, try: wl-webrtc --no-persist" + } else { + "" + }; + tracing::error!( + "Portal service did not respond within timeout while {phase}. \ + This usually means xdg-desktop-portal or xdg-desktop-portal-kde is stuck. \ + Try: systemctl --user restart xdg-desktop-portal xdg-desktop-portal-kde, \ + then re-run wl-webrtc.{persist_hint}" + ); +} + /// PipeWire DMA-BUF 帧数据 /// /// 表示从 PipeWire 流中接收到的一帧视频数据。 @@ -185,27 +248,80 @@ impl CapPortal { /// 5. 打开 PipeWire 远程连接,获取文件描述符 /// /// 返回 (PipeWire fd, node_id),供 PipeWire 线程连接使用 + /// + /// Wraps `_setup_portal_inner` with token-aware retry: on a `TokenDependent` + /// timeout (phases 3 or 4 with a restore token in use) AND `no_persist == + /// false`, clears the cached restore token and retries once with + /// `no_persist = true`. async fn setup_portal(no_persist: bool) -> Result<(OwnedFd, u32)> { + match Self::_setup_portal_inner(no_persist, false).await { + Ok(result) => Ok(result), + Err(e) if e.is::() => { + let inner_err = e.downcast_ref::().unwrap(); + match inner_err { + PortalPhaseTimeout::TokenDependent if !no_persist => { + tracing::warn!( + "Portal timed out during token-using phase. \ + Clearing cached restore token and retrying with fresh authorization." + ); + delete_restore_token(); + Self::_setup_portal_inner(true, true).await + } + _ => Err(e), + } + } + Err(e) => Err(e), + } + } + + /// Inner Portal setup with phased timeouts. See `setup_portal` for the + /// retry wrapper. + /// + /// `is_retry == true` disables further retry attempts (max 1 retry). + async fn _setup_portal_inner( + no_persist: bool, + is_retry: bool, + ) -> Result<(OwnedFd, u32)> { use ashpd::desktop::screencast::{ CursorMode, Screencast, SelectSourcesOptions, SourceType, }; use ashpd::desktop::PersistMode; - let proxy = Screencast::new() - .await - .map_err(|e| anyhow::anyhow!("Failed to create Screencast proxy: {e}"))?; + // Phase 1: Screencast proxy (no user interaction). + let proxy = match tokio::time::timeout(PORTAL_SERVICE_TIMEOUT, Screencast::new()).await { + Ok(Ok(p)) => p, + Ok(Err(e)) => return Err(anyhow::anyhow!("Failed to create Screencast proxy: {e}")), + Err(_) => { + log_portal_phase_timeout("creating Screencast proxy", false); + return Err(PortalPhaseTimeout::Service.into()); + } + }; - let session = proxy - .create_session(Default::default()) - .await - .map_err(|e| anyhow::anyhow!("Failed to create ScreenCast session: {e}"))?; + // Phase 2: create_session (no user interaction). + let session = match tokio::time::timeout( + PORTAL_SERVICE_TIMEOUT, + proxy.create_session(Default::default()), + ) + .await + { + Ok(Ok(s)) => s, + Ok(Err(e)) => return Err(anyhow::anyhow!("Failed to create ScreenCast session: {e}")), + Err(_) => { + log_portal_phase_timeout("creating session", false); + return Err(PortalPhaseTimeout::Service.into()); + } + }; let version_supported = proxy.version() >= 4; let (persist_mode, saved_token) = if !no_persist && version_supported { let token = load_restore_token(); if token.is_some() { - tracing::info!("Attempting to restore portal session with saved token"); + if is_retry { + tracing::info!("Re-attempting portal session after token clear"); + } else { + tracing::info!("Attempting to restore portal session with saved token"); + } } (PersistMode::ExplicitlyRevoked, token) } else { @@ -222,17 +338,57 @@ impl CapPortal { options = options.set_restore_token(token.as_str()); } - proxy - .select_sources(&session, options) - .await - .map_err(|e| anyhow::anyhow!("Screen sharing permission denied: {e}"))?; + // Phase 3: select_sources — token path is fast (no dialog); fresh + // authorization may pop a dialog. + let token_in_use = saved_token.is_some(); + let phase3_timeout = if token_in_use { + PORTAL_SERVICE_TIMEOUT + } else { + PORTAL_USER_DIALOG_TIMEOUT + }; + match tokio::time::timeout(phase3_timeout, proxy.select_sources(&session, options)).await { + Ok(Ok(_)) => {} + Ok(Err(e)) => return Err(anyhow::anyhow!("Screen sharing permission denied: {e}")), + Err(_) => { + log_portal_phase_timeout("selecting sources", token_in_use); + return Err( + if token_in_use { + PortalPhaseTimeout::TokenDependent + } else { + PortalPhaseTimeout::Service + } + .into(), + ); + } + } - let response = proxy - .start(&session, None, Default::default()) - .await - .map_err(|e| anyhow::anyhow!("ScreenCast start failed: {e}"))? - .response() - .map_err(|e| anyhow::anyhow!("ScreenCast response error: {e}"))?; + // Phase 4: start + response — same dialog-vs-token reasoning as phase 3. + let phase4_timeout = if token_in_use { + PORTAL_SERVICE_TIMEOUT + } else { + PORTAL_USER_DIALOG_TIMEOUT + }; + let start_fut = async { + proxy + .start(&session, None, Default::default()) + .await? + .response() + }; + let response = match tokio::time::timeout(phase4_timeout, start_fut).await { + Ok(Ok(r)) => r, + Ok(Err(e)) => return Err(anyhow::anyhow!("ScreenCast start/response error: {e}")), + Err(_) => { + log_portal_phase_timeout("starting session", token_in_use); + return Err( + if token_in_use { + PortalPhaseTimeout::TokenDependent + } else { + PortalPhaseTimeout::Service + } + .into(), + ); + } + }; if !no_persist && version_supported { if let Some(new_token) = response.restore_token() { @@ -247,10 +403,20 @@ impl CapPortal { let node_id = stream.pipe_wire_node_id(); - let fd = proxy - .open_pipe_wire_remote(&session, Default::default()) - .await - .map_err(|e| anyhow::anyhow!("Failed to open PipeWire remote: {e}"))?; + // Phase 5: open_pipe_wire_remote (no user interaction). + let fd = match tokio::time::timeout( + PORTAL_SERVICE_TIMEOUT, + proxy.open_pipe_wire_remote(&session, Default::default()), + ) + .await + { + Ok(Ok(f)) => f, + Ok(Err(e)) => return Err(anyhow::anyhow!("Failed to open PipeWire remote: {e}")), + Err(_) => { + log_portal_phase_timeout("opening PipeWire remote", false); + return Err(PortalPhaseTimeout::Service.into()); + } + }; tracing::info!("Portal session established: node_id={node_id}"); @@ -391,6 +557,17 @@ fn save_restore_token(token: &str) { save_restore_token_to(token, &path); } +fn delete_restore_token() { + let Some(path) = token_path() else { + return; + }; + match std::fs::remove_file(&path) { + Ok(()) => tracing::info!("Deleted stale portal restore token at {}", path.display()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => tracing::warn!("Failed to delete stale restore token at {}: {e}", path.display()), + } +} + fn save_restore_token_to(token: &str, path: &std::path::Path) { use std::fs::OpenOptions; use std::io::Write;