diff --git a/cli/src/commands.rs b/cli/src/commands.rs index 6e75b11..4535f7a 100644 --- a/cli/src/commands.rs +++ b/cli/src/commands.rs @@ -320,7 +320,27 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result *u, None if cmd == "open" => { @@ -350,6 +370,23 @@ fn parse_command_inner(args: &[String], flags: &Flags) -> Result --wait-until ", + })?; + if !["load", "domcontentloaded", "networkidle", "none"].contains(val) { + return Err(ParseError::InvalidValue { + message: format!("Unknown --wait-until value: {}", val), + usage: "open --wait-until ", + }); + } + nav_cmd["waitUntil"] = json!(val); + } if let Some(ref headers_json) = flags.headers { let headers = serde_json::from_str::(headers_json).map_err(|_| { @@ -3643,6 +3680,44 @@ mod tests { } } + #[test] + fn test_open_wait_until_after_url() { + let cmd = parse_command( + &args("open https://x.com --wait-until domcontentloaded"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["action"], "navigate"); + assert_eq!(cmd["url"], "https://x.com"); + assert_eq!(cmd["waitUntil"], "domcontentloaded"); + } + + #[test] + fn test_open_wait_until_before_url_not_mistaken_for_url() { + // The --wait-until value must not be picked up as the URL. + let cmd = parse_command( + &args("open --wait-until domcontentloaded https://x.com"), + &default_flags(), + ) + .unwrap(); + assert_eq!(cmd["url"], "https://x.com"); + assert_eq!(cmd["waitUntil"], "domcontentloaded"); + } + + #[test] + fn test_open_wait_until_rejects_bogus_value() { + let err = parse_command( + &args("open https://x.com --wait-until wat"), + &default_flags(), + ) + .unwrap_err(); + assert!( + err.format().contains("--wait-until"), + "got: {}", + err.format() + ); + } + #[test] fn test_find_bare_value_suggests_text_locator() { // `find "I'm not a robot" click` — value where a locator keyword was diff --git a/cli/src/native/browser.rs b/cli/src/native/browser.rs index b6be9e0..2540a68 100644 --- a/cli/src/native/browser.rs +++ b/cli/src/native/browser.rs @@ -265,6 +265,15 @@ impl WaitUntil { _ => Self::Load, } } + + pub fn as_str(&self) -> &'static str { + match self { + Self::Load => "load", + Self::DomContentLoaded => "domcontentloaded", + Self::NetworkIdle => "networkidle", + Self::None => "none", + } + } } pub enum BrowserProcess { @@ -800,9 +809,38 @@ impl BrowserManager { // Only wait for lifecycle events if Chrome created a new loader (full navigation). // If loader_id is None, it was a same-document navigation (e.g., hash routing) // which does not fire Page.loadEventFired or Page.domContentEventFired. + let mut nav_warning: Option = None; if nav_result.loader_id.is_some() && wait_until != WaitUntil::None { - self.wait_for_lifecycle(wait_until, &session_id, &mut lifecycle_rx) - .await?; + if let Err(e) = self + .wait_for_lifecycle(wait_until, &session_id, &mut lifecycle_rx) + .await + { + // The lifecycle event (e.g. `load`) didn't fire within the + // timeout. On SPAs this is common — a long-pending XHR or a stuck + // sub-resource holds `load` open long after the DOM is interactive + // and the page is usable, so `open` would hard-fail even though + // eval/screenshot work immediately (issue #10). If the DOM is + // already ready, treat navigation as done (with a warning, carried + // in the response so the CLI can surface it) instead of failing. + // Only a still-loading document is a real failure. + let ready = self + .evaluate_simple("document.readyState") + .await + .ok() + .and_then(|v| v.as_str().map(str::to_string)) + .unwrap_or_default(); + if ready == "interactive" || ready == "complete" { + nav_warning = Some(format!( + "`{}` didn't complete within the timeout, but the DOM is ready ({}) — \ + continuing. Pass `--wait-until domcontentloaded` to skip this wait on \ + SPAs with long-lived requests.", + wait_until.as_str(), + ready + )); + } else { + return Err(e); + } + } } let page_url = self.get_url().await.unwrap_or_else(|_| url.to_string()); @@ -821,7 +859,11 @@ impl BrowserManager { page.title = title.clone(); } - Ok(json!({ "url": page_url, "title": title })) + let mut out = json!({ "url": page_url, "title": title }); + if let Some(w) = nav_warning { + out["warning"] = json!(w); + } + Ok(out) } async fn wait_for_lifecycle( diff --git a/cli/src/output.rs b/cli/src/output.rs index db073b6..badccc3 100644 --- a/cli/src/output.rs +++ b/cli/src/output.rs @@ -256,6 +256,12 @@ pub fn print_response_with_opts(resp: &Response, action: Option<&str>, opts: &Ou // empty title line. None => println!("{} {}", color::success_indicator(), color::dim(url)), } + // Soft warning carried in the response (e.g. the load event timed out + // but the DOM was ready — issue #10). Goes to stderr so it doesn't + // pollute the stdout url/title that scripts parse. + if let Some(w) = data.get("warning").and_then(|v| v.as_str()) { + eprintln!("⚠ navigation: {w}"); + } return; } if let Some(cdp_url) = data.get("cdpUrl").and_then(|v| v.as_str()) {