diff --git a/code-rs/browser/src/manager.rs b/code-rs/browser/src/manager.rs index a27decd8b60..d4c6e4e65b3 100644 --- a/code-rs/browser/src/manager.rs +++ b/code-rs/browser/src/manager.rs @@ -141,6 +141,16 @@ fn should_stop_handler( false } Err(err) => { + let message = err.to_string(); + let message_lower = message.to_ascii_lowercase(); + // Chromiumoxide surfaces "oneshot canceled" when a request future is dropped + // (e.g., due to our own timeouts). These are expected and should not + // be treated as connection failures. + if message_lower.contains("oneshot canceled") || message_lower.contains("oneshot cancelled") { + *consecutive_errors = 0; + debug!("{label} event handler error ignored: {message}"); + return false; + } *consecutive_errors = consecutive_errors.saturating_add(1); let count = *consecutive_errors; if count <= HANDLER_ERROR_LIMIT { @@ -161,6 +171,7 @@ async fn discover_ws_via_host_port(host: &str, port: u16) -> Result { let client_start = tokio::time::Instant::now(); let client = Client::builder() + .no_proxy() .timeout(Duration::from_secs(5)) // Allow Chrome time to bring up /json/version on fresh launch .build() .map_err(|e| BrowserError::CdpError(format!("Failed to build HTTP client: {}", e)))?; @@ -248,6 +259,7 @@ async fn scan_for_chrome_debug_port() -> Option { let test_future = async move { let url = format!("http://127.0.0.1:{}/json/version", port); let client = Client::builder() + .no_proxy() .timeout(Duration::from_millis(200)) // Shorter timeout for parallel tests .build() .ok()?; @@ -2502,7 +2514,28 @@ pub struct BrowserStatus { #[cfg(test)] mod tests { + use super::discover_ws_via_host_port; use super::should_restart_handler; + use super::should_stop_handler; + use std::io::Read; + use std::io::Write; + use std::net::TcpListener; + use std::process::Command; + use std::sync::Arc; + use std::sync::atomic::AtomicBool; + use std::sync::atomic::Ordering; + use std::thread; + use std::time::Duration; + use std::time::Instant; + + #[derive(Debug)] + struct TestError(&'static str); + + impl std::fmt::Display for TestError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.0) + } + } #[test] fn handler_restarts_after_repeated_errors() { @@ -2511,4 +2544,132 @@ mod tests { assert!(!should_restart_handler(2)); assert!(should_restart_handler(3)); } + + #[test] + fn handler_ignores_oneshot_cancellations() { + let mut consecutive_errors = 0u32; + for _ in 0..10 { + let should_stop = should_stop_handler( + "[test]", + Err(TestError("oneshot canceled")), + &mut consecutive_errors, + ); + assert!(!should_stop); + assert_eq!(consecutive_errors, 0); + } + } + + const TEST_PROXY_WS_URL: &str = "ws://proxy.invalid/devtools/browser/proxy"; + const TEST_TARGET_WS_URL: &str = "ws://target.invalid/devtools/browser/target"; + + fn spawn_json_version_server( + ws_url: &str, + ) -> (u16, Arc, thread::JoinHandle<()>) { + let listener = TcpListener::bind("127.0.0.1:0").expect("bind server"); + listener + .set_nonblocking(true) + .expect("set non-blocking"); + let port = listener.local_addr().expect("server addr").port(); + let stop = Arc::new(AtomicBool::new(false)); + let stop_thread = Arc::clone(&stop); + let ws_url = ws_url.to_string(); + + let handle = thread::spawn(move || { + let deadline = Instant::now() + Duration::from_secs(10); + while !stop_thread.load(Ordering::Relaxed) && Instant::now() < deadline { + match listener.accept() { + Ok((mut stream, _)) => { + let mut buffer = [0u8; 1024]; + let _ = stream.read(&mut buffer); + + let body = format!(r#"{{"webSocketDebuggerUrl":"{ws_url}"}}"#); + let response = format!( + "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", + body.len(), + body + ); + let _ = stream.write_all(response.as_bytes()); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(10)); + } + Err(_) => break, + } + } + }); + + (port, stop, handle) + } + + #[test] + fn cdp_discovery_ignores_proxy_env_vars() { + let (proxy_port, proxy_stop, proxy_handle) = + spawn_json_version_server(TEST_PROXY_WS_URL); + let (target_port, target_stop, target_handle) = + spawn_json_version_server(TEST_TARGET_WS_URL); + + let exe = std::env::current_exe().expect("current exe"); + let proxy_url = format!("http://127.0.0.1:{proxy_port}"); + + let output = Command::new(exe) + .arg("--exact") + .arg("manager::tests::cdp_discovery_ignores_proxy_env_vars_child") + .arg("--ignored") + .arg("--nocapture") + .env("CODE_BROWSER_TEST_TARGET_PORT", target_port.to_string()) + .env("HTTP_PROXY", &proxy_url) + .env("HTTPS_PROXY", &proxy_url) + .env("ALL_PROXY", &proxy_url) + .env("http_proxy", &proxy_url) + .env("https_proxy", &proxy_url) + .env("all_proxy", &proxy_url) + .env("NO_PROXY", "") + .env("no_proxy", "") + .output() + .expect("spawn child test"); + + proxy_stop.store(true, Ordering::Relaxed); + target_stop.store(true, Ordering::Relaxed); + let _ = proxy_handle.join(); + let _ = target_handle.join(); + + if !output.status.success() { + panic!( + "child test failed: status={:?}\nstdout:\n{}\nstderr:\n{}", + output.status, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr), + ); + } + } + + #[ignore] + #[tokio::test] + async fn cdp_discovery_ignores_proxy_env_vars_child() { + let target_port: u16 = std::env::var("CODE_BROWSER_TEST_TARGET_PORT") + .expect("CODE_BROWSER_TEST_TARGET_PORT") + .parse() + .expect("valid port"); + + let url = format!("http://127.0.0.1:{target_port}/json/version"); + let default_client = reqwest::Client::builder() + .timeout(Duration::from_secs(2)) + .build() + .expect("build default client"); + let resp = default_client + .get(&url) + .send() + .await + .expect("default request"); + + let proxy_version: super::JsonVersion = + resp.json().await.expect("parse proxy json"); + assert_eq!(proxy_version.web_socket_debugger_url, TEST_PROXY_WS_URL); + + let discovered = discover_ws_via_host_port("127.0.0.1", target_port) + .await + .expect("discover ws url"); + assert_eq!(discovered, TEST_TARGET_WS_URL); + } + } diff --git a/code-rs/tui/src/history_cell/diff.rs b/code-rs/tui/src/history_cell/diff.rs index d51a5ec83d8..8bf4a721895 100644 --- a/code-rs/tui/src/history_cell/diff.rs +++ b/code-rs/tui/src/history_cell/diff.rs @@ -113,6 +113,8 @@ pub(crate) fn diff_record_from_string(title: String, diff: &str) -> DiffRecord { (DiffLineKind::Addition, rest.to_string()) } else if let Some(rest) = line.strip_prefix('-') { (DiffLineKind::Removal, rest.to_string()) + } else if let Some(rest) = line.strip_prefix(' ') { + (DiffLineKind::Context, rest.to_string()) } else { (DiffLineKind::Context, line) }; @@ -159,4 +161,21 @@ mod tests { assert!(!lines[0].content.contains('\u{001B}')); assert!(!lines[1].content.contains('\u{001B}')); } + + #[test] + fn diff_record_strips_context_prefix_space() { + let diff = concat!( + "@@ -1,3 +1,3 @@\n", + " unchanged\n", + "-old\n", + "+new\n", + ); + let record = diff_record_from_string(String::new(), diff); + assert_eq!(record.hunks.len(), 1); + let lines = &record.hunks[0].lines; + assert_eq!(lines.len(), 3); + assert_eq!(lines[0].content, "unchanged"); + assert_eq!(lines[1].content, "old"); + assert_eq!(lines[2].content, "new"); + } }