Honor fetch timeout for navigation deadlines

crates/obscura-browser/src/page.rs

@@ -240,6 +240,11 @@ pub struct Page {
     /// It is reset once author styles are installed, so stylesheet download
     /// latency does not incorrectly advance newly-created animations.
     document_timeline_origin: std::time::Instant,
+    /// Optional page-scoped ceiling for an end-to-end navigation. Automation
+    /// frontends set this from their request timeout so a caller asking for a
+    /// 50-second navigation is not silently cut off by the process default.
+    /// Pages without an override retain the environment-configurable default.
+    navigation_timeout: Option<std::time::Duration>,
     /// Navigation history for Page.getNavigationHistory / navigateToHistoryEntry.
     /// Entries are URLs in visit order; `history_index` is the current position.
     /// Pushed on every successful navigation; truncated on goBack -> new nav.
@@ -272,6 +277,22 @@ pub struct Page {
 
 const MAX_STYLESHEET_IMPORT_DEPTH: u8 = 4;
 const MAX_STYLESHEET_RESOURCES: usize = 128;
+const DEFAULT_NAVIGATION_TIMEOUT_MS: u64 = 30_000;
+
+fn default_navigation_timeout() -> std::time::Duration {
+    navigation_timeout_from_env_value(std::env::var("OBSCURA_NAV_TIMEOUT_MS").ok().as_deref())
+}
+
+fn navigation_timeout_from_env_value(value: Option<&str>) -> std::time::Duration {
+    let milliseconds = value
+        .and_then(|value| value.parse().ok())
+        .unwrap_or(DEFAULT_NAVIGATION_TIMEOUT_MS);
+    std::time::Duration::from_millis(milliseconds)
+}
+
+fn duration_millis_u64(duration: std::time::Duration) -> u64 {
+    duration.as_millis().min(u128::from(u64::MAX)) as u64
+}
 
 #[derive(Clone)]
 struct LoadedStylesheet {
@@ -863,6 +884,7 @@ impl Page {
             default_background_color_override: None,
             encoding: "UTF-8".to_string(),
             document_timeline_origin: std::time::Instant::now(),
+            navigation_timeout: None,
             history: Vec::new(),
             history_index: 0,
             network_events: Vec::new(),
@@ -881,6 +903,19 @@ impl Page {
         }
     }
 
+    /// Set the end-to-end navigation deadline for this page. This page-scoped
+    /// value takes precedence over `OBSCURA_NAV_TIMEOUT_MS`; callers that do
+    /// not set it retain the existing environment-configurable 30s default.
+    pub fn set_navigation_timeout(&mut self, timeout: std::time::Duration) {
+        self.navigation_timeout = Some(timeout);
+    }
+
+    /// Return the effective end-to-end navigation deadline for this page.
+    pub fn navigation_timeout(&self) -> std::time::Duration {
+        self.navigation_timeout
+            .unwrap_or_else(default_navigation_timeout)
+    }
+
     fn should_block_url(&self, url: &str) -> bool {
         for pattern in &self.blocked_url_patterns {
             if url_matches_cdp_pattern(pattern, url) {
@@ -2125,12 +2160,10 @@ impl Page {
         // dispatcher holds the lock across the entire handler. 30 seconds
         // matches reqwest's default per-request timeout — the worst case is
         // one slow primary GET plus one slow JS-redirect chain step. Override
-        // with `OBSCURA_NAV_TIMEOUT_MS=NN`.
-        let nav_timeout_ms: u64 = std::env::var("OBSCURA_NAV_TIMEOUT_MS")
-            .ok()
-            .and_then(|s| s.parse().ok())
-            .unwrap_or(30_000);
-        let nav_timeout = tokio::time::Duration::from_millis(nav_timeout_ms);
+        // with `OBSCURA_NAV_TIMEOUT_MS=NN`, or set a page-scoped deadline when
+        // the automation request already has an explicit timeout.
+        let nav_timeout = self.navigation_timeout();
+        let nav_timeout_ms = duration_millis_u64(nav_timeout);
 
         let result = match tokio::time::timeout(
             nav_timeout,
@@ -3482,12 +3515,10 @@ impl Page {
                         .map(|target| navigation_referrer(source, &target))
                 })
                 .unwrap_or_default();
-            let nav_timeout_ms: u64 = std::env::var("OBSCURA_NAV_TIMEOUT_MS")
-                .ok()
-                .and_then(|s| s.parse().ok())
-                .unwrap_or(30_000);
+            let nav_timeout = self.navigation_timeout();
+            let nav_timeout_ms = duration_millis_u64(nav_timeout);
             let result = tokio::time::timeout(
-                tokio::time::Duration::from_millis(nav_timeout_ms),
+                nav_timeout,
                 self.navigate_with_wait_post_inner(
                     &url,
                     crate::lifecycle::WaitUntil::Load,
@@ -3562,15 +3593,35 @@ fn url_matches_cdp_pattern(pattern: &str, url: &str) -> bool {
 mod tests {
     use super::{
         css_resource_urls, linked_stylesheet_requests, materialize_linked_stylesheet_script,
-        materialize_stylesheet_graph, navigation_referrer, parse_import_url, rebase_css_urls,
-        script_response_is_executable, split_css_imports, truncate_on_char_boundary,
-        url_matches_cdp_pattern, LoadedStylesheet, StylesheetImport,
+        materialize_stylesheet_graph, navigation_referrer, navigation_timeout_from_env_value,
+        parse_import_url, rebase_css_urls, script_response_is_executable, split_css_imports,
+        truncate_on_char_boundary, url_matches_cdp_pattern, LoadedStylesheet, StylesheetImport,
     };
     #[cfg(feature = "render")]
     use super::remaining_settle_resource_warmup_ms;
     use base64::Engine as _;
     use obscura_dom::parse_html;
 
+    #[test]
+    fn navigation_timeout_environment_default_remains_thirty_seconds() {
+        assert_eq!(
+            navigation_timeout_from_env_value(None),
+            std::time::Duration::from_secs(30)
+        );
+        assert_eq!(
+            navigation_timeout_from_env_value(Some("not-a-timeout")),
+            std::time::Duration::from_secs(30)
+        );
+    }
+
+    #[test]
+    fn navigation_timeout_environment_override_remains_available() {
+        assert_eq!(
+            navigation_timeout_from_env_value(Some("42000")),
+            std::time::Duration::from_secs(42)
+        );
+    }
+
     #[test]
     fn css_resource_discovery_ignores_strings_comments_data_and_fragments() {
         let base = url::Url::parse("https://example.test/css/app/main.css").unwrap();

crates/obscura-cli/src/main.rs

@@ -667,6 +667,10 @@ async fn settle_page(page: &mut Page, wait_secs: u64, fixed: bool) {
     }
 }
 
+fn configure_fetch_navigation_timeout(page: &mut Page, timeout_secs: u64) {
+    page.set_navigation_timeout(Duration::from_secs(timeout_secs));
+}
+
 async fn run_fetch(
     url_str: &str,
     dump: Option<DumpFormat>,
@@ -710,6 +714,10 @@ async fn run_fetch(
         allow_private_network,
     ));
     let mut page = Page::new("fetch-page".to_string(), context.clone());
+    // Keep the browser's end-to-end navigation ceiling aligned with the CLI
+    // request deadline. Previously Page retained its independent 30s default,
+    // so `fetch --timeout 50` could still fail after 30 seconds.
+    configure_fetch_navigation_timeout(&mut page, timeout_secs);
     // A screenshot viewport is also the navigation viewport: responsive
     // frameworks must build the DOM for the same dimensions we later paint.
     // Previously page JS saw a randomized screen-sized innerWidth while the
@@ -1868,10 +1876,10 @@ fn dump_assets(page: &Page) -> String {
 #[cfg(test)]
 mod tests {
     use super::{
-        effective_v8_flags, extract_assets, extract_readable_text, fetch_original_bytes,
-        is_quiet_command, link_kind_from_rel, merge_proxy, normalize_v8_flags, read_urls_from_file,
-        resolve_asset_url, select_log_filter, write_or_print, write_or_print_bytes, Args, Command,
-        DumpFormat, DEFAULT_V8_FLAGS,
+        configure_fetch_navigation_timeout, effective_v8_flags, extract_assets,
+        extract_readable_text, fetch_original_bytes, is_quiet_command, link_kind_from_rel,
+        merge_proxy, normalize_v8_flags, read_urls_from_file, resolve_asset_url, select_log_filter,
+        write_or_print, write_or_print_bytes, Args, Command, DumpFormat, DEFAULT_V8_FLAGS,
     };
     use clap::Parser;
     use obscura_dom::parse_html;
@@ -2226,6 +2234,51 @@ mod tests {
         }
     }
 
+    fn configured_fetch_timeout(args: Args) -> std::time::Duration {
+        let timeout = match args.command {
+            Some(Command::Fetch { timeout, .. }) => timeout,
+            _ => panic!("expected Fetch command"),
+        };
+        let context = std::sync::Arc::new(
+            obscura_browser::BrowserContext::with_storage_and_network(
+                "cli-timeout-test".to_string(),
+                None,
+                false,
+                None,
+                None,
+                true,
+            ),
+        );
+        let mut page = obscura_browser::Page::new("cli-timeout-test".to_string(), context);
+        configure_fetch_navigation_timeout(&mut page, timeout);
+        page.navigation_timeout()
+    }
+
+    #[test]
+    fn fetch_timeout_sets_the_page_navigation_budget() {
+        let args = Args::try_parse_from([
+            "obscura",
+            "fetch",
+            "https://example.com",
+            "--timeout",
+            "50",
+        ])
+        .unwrap();
+        assert_eq!(
+            configured_fetch_timeout(args),
+            std::time::Duration::from_secs(50)
+        );
+    }
+
+    #[test]
+    fn fetch_default_navigation_budget_remains_thirty_seconds() {
+        let args = Args::try_parse_from(["obscura", "fetch", "https://example.com"]).unwrap();
+        assert_eq!(
+            configured_fetch_timeout(args),
+            std::time::Duration::from_secs(30)
+        );
+    }
+
     #[test]
     fn matcher_still_uses_fetch_variant() {
         let cmd = Some(Command::Fetch {