fix: stabilize /browser and /diff (#522)

* fix: stabilize /browser and /diff

Fixes #508

Fixes #473

* test(browser): add proxy-env regression for CDP discovery

Author: zemaj

code-rs/browser/src/manager.rs

@@ -141,6 +141,16 @@ fn should_stop_handler<E: std::fmt::Display>(
             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<String> {
 
     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<u16> {
         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<AtomicBool>, 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);
+    }
+
 }

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");
+    }
 }