refactor: better camoufox instance tracking

src-tauri/src/browser_runner.rs

@@ -212,6 +212,10 @@ impl BrowserRunner {
         };
 
         // Use the nodecar camoufox launcher
+        println!(
+          "Launching Camoufox via nodecar for profile: {}",
+          profile.name
+        );
         let camoufox_result = crate::camoufox::launch_camoufox_profile_nodecar(
           app_handle.clone(),
           profile.clone(),
@@ -223,21 +227,27 @@ impl BrowserRunner {
           format!("Failed to launch camoufox via nodecar: {e}").into()
         })?;
 
-        // For server-based Camoufox, we don't have a PID but we have a port
-        // We'll use the port as a unique identifier for the running instance
-        let process_id = camoufox_result.port;
+        // For server-based Camoufox, we use the port as a unique identifier (which is actually the PID)
+        let process_id = camoufox_result.port.unwrap_or(0);
+        println!("Camoufox launched successfully with PID: {process_id}");
 
         // Update profile with the process info from camoufox result
         let mut updated_profile = profile.clone();
-        updated_profile.process_id = process_id;
+        updated_profile.process_id = Some(process_id);
         updated_profile.last_launch = Some(SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs());
 
         // Save the updated profile
         self.save_process_info(&updated_profile)?;
+        println!(
+          "Updated profile with process info: {}",
+          updated_profile.name
+        );
 
         // Emit profile update event to frontend
         if let Err(e) = app_handle.emit("profile-updated", &updated_profile) {
           println!("Warning: Failed to emit profile update event: {e}");
+        } else {
+          println!("Emitted profile update event for: {}", updated_profile.name);
         }
 
         return Ok(updated_profile);
@@ -769,69 +779,62 @@ impl BrowserRunner {
     if profile.browser == "camoufox" {
       let camoufox_launcher = crate::camoufox::CamoufoxNodecarLauncher::new(app_handle.clone());
 
-      // Try to stop by PID first (faster)
-      if let Some(stored_pid) = profile.process_id {
-        match camoufox_launcher
-          .stop_camoufox(&app_handle, &stored_pid.to_string())
-          .await
-        {
-          Ok(stopped) => {
-            if stopped {
-              println!("Successfully stopped Camoufox process by PID: {stored_pid}");
-            } else {
-              println!("Failed to stop Camoufox process by PID: {stored_pid}");
-            }
-          }
-          Err(e) => {
-            println!("Error stopping Camoufox process by PID: {e}");
-          }
-        }
-      } else {
-        // Fallback: search by profile path
-        let profiles_dir = self.get_profiles_dir();
-        let profile_data_path = profile.get_profile_data_path(&profiles_dir);
-        let profile_path_str = profile_data_path.to_string_lossy();
+      // Search by profile path to find the running Camoufox instance
+      let profiles_dir = self.get_profiles_dir();
+      let profile_data_path = profile.get_profile_data_path(&profiles_dir);
+      let profile_path_str = profile_data_path.to_string_lossy();
 
-        match camoufox_launcher
-          .find_camoufox_by_profile(&profile_path_str)
-          .await
-        {
-          Ok(Some(camoufox_process)) => {
-            match camoufox_launcher
-              .stop_camoufox(&app_handle, &camoufox_process.id)
-              .await
-            {
-              Ok(stopped) => {
-                if stopped {
-                  println!(
-                    "Successfully stopped Camoufox process: {}",
-                    camoufox_process.id
-                  );
-                } else {
-                  println!("Failed to stop Camoufox process: {}", camoufox_process.id);
-                }
-              }
-              Err(e) => {
-                println!("Error stopping Camoufox process: {e}");
+      println!(
+        "Attempting to kill Camoufox process for profile: {}",
+        profile.name
+      );
+
+      match camoufox_launcher
+        .find_camoufox_by_profile(&profile_path_str)
+        .await
+      {
+        Ok(Some(camoufox_process)) => {
+          println!(
+            "Found Camoufox process: {} (PID: {:?})",
+            camoufox_process.id, camoufox_process.port
+          );
+
+          match camoufox_launcher
+            .stop_camoufox(&app_handle, &camoufox_process.id)
+            .await
+          {
+            Ok(stopped) => {
+              if stopped {
+                println!(
+                  "Successfully stopped Camoufox process: {} (PID: {:?})",
+                  camoufox_process.id, camoufox_process.port
+                );
+              } else {
+                println!(
+                  "Failed to stop Camoufox process: {} (PID: {:?})",
+                  camoufox_process.id, camoufox_process.port
+                );
               }
             }
-          }
-          Ok(None) => {
-            println!(
-              "No running Camoufox process found for profile: {}",
-              profile.name
-            );
-          }
-          Err(e) => {
-            println!("Error finding Camoufox process: {e}");
+            Err(e) => {
+              println!(
+                "Error stopping Camoufox process {}: {}",
+                camoufox_process.id, e
+              );
+            }
           }
         }
-      }
-
-      // Stop proxy if one was running for this profile
-      if let Some(pid) = profile.process_id {
-        if let Err(e) = PROXY_MANAGER.stop_proxy(app_handle.clone(), pid).await {
-          println!("Warning: Failed to stop proxy for Camoufox profile: {e}");
+        Ok(None) => {
+          println!(
+            "No running Camoufox process found for profile: {}",
+            profile.name
+          );
+        }
+        Err(e) => {
+          println!(
+            "Error finding Camoufox process for profile {}: {}",
+            profile.name, e
+          );
         }
       }
 
@@ -847,6 +850,10 @@ impl BrowserRunner {
         println!("Warning: Failed to emit profile update event: {e}");
       }
 
+      println!(
+        "Camoufox process cleanup completed for profile: {}",
+        profile.name
+      );
       return Ok(());
     }
 
@@ -877,13 +884,7 @@ impl BrowserRunner {
             "zen" => exe_name.contains("zen"),
             "chromium" => exe_name.contains("chromium"),
             "brave" => exe_name.contains("brave"),
-            "camoufox" => {
-              exe_name.contains("camoufox")
-                || (exe_name.contains("firefox")
-                  && cmd
-                    .iter()
-                    .any(|arg| arg.to_str().unwrap_or("").contains("camoufox")))
-            }
+            // Camoufox is handled via nodecar, not PID-based checking
             _ => false,
           };
 
@@ -1620,12 +1621,14 @@ pub fn create_browser_profile_new(
 
 #[tauri::command]
 pub async fn update_camoufox_config(
+  app_handle: tauri::AppHandle,
   profile_name: String,
   config: CamoufoxConfig,
 ) -> Result<(), String> {
   let profile_manager = ProfileManager::new();
   profile_manager
-    .update_camoufox_config(&profile_name, config)
+    .update_camoufox_config(app_handle, &profile_name, config)
+    .await
     .map_err(|e| format!("Failed to update Camoufox config: {e}"))
 }
 

src-tauri/src/camoufox.rs

@@ -437,18 +437,22 @@ impl CamoufoxNodecarLauncher {
     }
 
     // Execute nodecar sidecar command
+    println!("Executing nodecar command with args: {args:?}");
     let output = sidecar_command.output().await?;
 
     if !output.status.success() {
       let stderr = String::from_utf8_lossy(&output.stderr);
+      let stdout = String::from_utf8_lossy(&output.stdout);
+      println!("nodecar camoufox failed - stdout: {stdout}, stderr: {stderr}");
       return Err(format!("nodecar camoufox failed: {stderr}").into());
     }
 
     let stdout = String::from_utf8_lossy(&output.stdout);
+    println!("nodecar camoufox output: {stdout}");
 
     // Parse the JSON output
     let launch_result: CamoufoxLaunchResult = serde_json::from_str(&stdout)
-      .map_err(|e| format!("Failed to parse nodecar output as JSON: {e}"))?;
+      .map_err(|e| format!("Failed to parse nodecar output as JSON: {e}\nOutput was: {stdout}"))?;
 
     // Store the instance
     let instance = CamoufoxInstance {
@@ -529,9 +533,10 @@ impl CamoufoxNodecarLauncher {
           .unwrap_or_else(|_| std::path::Path::new(instance_profile_path).to_path_buf());
 
         if instance_path == target_path {
-          // Verify the server is actually running by checking the port
+          // Verify the server is actually running by checking the process
           if let Some(port) = instance.port {
             if self.is_server_running(port).await {
+              println!("Found running Camoufox instance for profile: {profile_path}");
               return Ok(Some(CamoufoxLaunchResult {
                 id: id.clone(),
                 port: instance.port,
@@ -539,12 +544,15 @@ impl CamoufoxNodecarLauncher {
                 profilePath: instance.profile_path.clone(),
                 url: instance.url.clone(),
               }));
+            } else {
+              println!("Camoufox instance found but process is not running: {id}");
             }
           }
         }
       }
     }
 
+    println!("No running Camoufox instance found for profile: {profile_path}");
     Ok(None)
   }
 
@@ -560,14 +568,16 @@ impl CamoufoxNodecarLauncher {
 
       for (id, instance) in inner.instances.iter() {
         if let Some(port) = instance.port {
-          // Check if the server is still alive
+          // Check if the process is still alive (port is actually PID)
           if !self.is_server_running(port).await {
-            // Server is dead
+            // Process is dead
+            println!("Camoufox instance {id} (PID: {port}) is no longer running");
             dead_instances.push(id.clone());
             instances_to_remove.push(id.clone());
           }
         } else {
-          // No port means it's likely a dead instance
+          // No port/PID means it's likely a dead instance
+          println!("Camoufox instance {id} has no PID, marking as dead");
           dead_instances.push(id.clone());
           instances_to_remove.push(id.clone());
         }
@@ -579,26 +589,35 @@ impl CamoufoxNodecarLauncher {
       let mut inner = self.inner.lock().await;
       for id in &instances_to_remove {
         inner.instances.remove(id);
+        println!("Removed dead Camoufox instance: {id}");
       }
     }
 
     Ok(dead_instances)
   }
 
-  /// Check if a Camoufox server is running on the given port
+  /// Check if a Camoufox server is running on the given port (which is actually a PID)
   async fn is_server_running(&self, port: u32) -> bool {
-    let client = reqwest::Client::new();
-    let url = format!("http://localhost:{port}/json/version");
+    // For Camoufox, the "port" is actually the process PID
+    // Check if the process is still running
+    use sysinfo::{Pid, System};
 
-    match client
-      .get(&url)
-      .timeout(std::time::Duration::from_secs(1))
-      .send()
-      .await
-    {
-      Ok(response) => response.status().is_success(),
-      Err(_) => false,
+    let system = System::new_all();
+    if let Some(process) = system.process(Pid::from(port as usize)) {
+      // Check if this is actually a Camoufox process by looking at the command line
+      let cmd = process.cmd();
+      let is_camoufox = cmd.iter().any(|arg| {
+        let arg_str = arg.to_str().unwrap_or("");
+        arg_str.contains("camoufox-worker") || arg_str.contains("camoufox")
+      });
+
+      if is_camoufox {
+        println!("Found running Camoufox process with PID: {port}");
+        return true;
+      }
     }
+
+    false
   }
 }
 

src-tauri/src/geoip_downloader.rs

@@ -27,6 +27,12 @@ impl GeoIPDownloader {
     }
   }
 
+  /// Create a new downloader with custom client (for testing)
+  #[cfg(test)]
+  pub fn new_with_client(client: Client) -> Self {
+    Self { client }
+  }
+
   fn get_cache_dir() -> Result<PathBuf, Box<dyn std::error::Error + Send + Sync>> {
     let base_dirs = BaseDirs::new().ok_or("Failed to determine base directories")?;
 
@@ -169,3 +175,125 @@ impl GeoIPDownloader {
     Ok(releases)
   }
 }
+
+#[cfg(test)]
+mod tests {
+  use super::*;
+  use crate::browser::GithubRelease;
+  use wiremock::matchers::{method, path};
+  use wiremock::{Mock, MockServer, ResponseTemplate};
+
+  fn create_mock_release() -> GithubRelease {
+    GithubRelease {
+      tag_name: "v1.0.0".to_string(),
+      name: "Test Release".to_string(),
+      body: Some("Test release body".to_string()),
+      published_at: "2023-01-01T00:00:00Z".to_string(),
+      created_at: Some("2023-01-01T00:00:00Z".to_string()),
+      html_url: Some("https://example.com/release".to_string()),
+      tarball_url: Some("https://example.com/tarball".to_string()),
+      zipball_url: Some("https://example.com/zipball".to_string()),
+      draft: false,
+      prerelease: false,
+      is_nightly: false,
+      id: Some(1),
+      node_id: Some("test_node_id".to_string()),
+      target_commitish: None,
+      assets: vec![crate::browser::GithubAsset {
+        id: Some(1),
+        node_id: Some("test_asset_node_id".to_string()),
+        name: "GeoLite2-City.mmdb".to_string(),
+        label: None,
+        content_type: Some("application/octet-stream".to_string()),
+        state: Some("uploaded".to_string()),
+        size: 1024,
+        download_count: Some(0),
+        created_at: Some("2023-01-01T00:00:00Z".to_string()),
+        updated_at: Some("2023-01-01T00:00:00Z".to_string()),
+        browser_download_url: "https://example.com/GeoLite2-City.mmdb".to_string(),
+      }],
+    }
+  }
+
+  #[tokio::test]
+  async fn test_fetch_geoip_releases_success() {
+    let mock_server = MockServer::start().await;
+    let releases = vec![create_mock_release()];
+
+    Mock::given(method("GET"))
+      .and(path(format!("/repos/{MMDB_REPO}/releases")))
+      .respond_with(ResponseTemplate::new(200).set_body_json(&releases))
+      .mount(&mock_server)
+      .await;
+
+    let client = Client::builder()
+      .build()
+      .expect("Failed to create HTTP client");
+
+    let downloader = GeoIPDownloader::new_with_client(client);
+
+    // Override the URL for testing
+    let url = format!("{}/repos/{}/releases", mock_server.uri(), MMDB_REPO);
+    let response = downloader
+      .client
+      .get(&url)
+      .header("User-Agent", "Mozilla/5.0 (compatible; donutbrowser)")
+      .send()
+      .await
+      .expect("Request should succeed");
+
+    assert!(response.status().is_success());
+
+    let fetched_releases: Vec<GithubRelease> = response.json().await.expect("Should parse JSON");
+    assert_eq!(fetched_releases.len(), 1);
+    assert_eq!(fetched_releases[0].tag_name, "v1.0.0");
+  }
+
+  #[tokio::test]
+  async fn test_find_city_mmdb_asset() {
+    let downloader = GeoIPDownloader::new();
+    let release = create_mock_release();
+
+    let asset_url = downloader.find_city_mmdb_asset(&release);
+    assert!(asset_url.is_some());
+    assert_eq!(asset_url.unwrap(), "https://example.com/GeoLite2-City.mmdb");
+  }
+
+  #[tokio::test]
+  async fn test_find_city_mmdb_asset_not_found() {
+    let downloader = GeoIPDownloader::new();
+    let mut release = create_mock_release();
+    release.assets[0].name = "wrong-file.txt".to_string();
+
+    let asset_url = downloader.find_city_mmdb_asset(&release);
+    assert!(asset_url.is_none());
+  }
+
+  #[test]
+  fn test_get_cache_dir() {
+    let cache_dir = GeoIPDownloader::get_cache_dir();
+    assert!(cache_dir.is_ok());
+
+    let path = cache_dir.unwrap();
+    assert!(path.to_string_lossy().contains("camoufox"));
+  }
+
+  #[test]
+  fn test_get_mmdb_file_path() {
+    let mmdb_path = GeoIPDownloader::get_mmdb_file_path();
+    assert!(mmdb_path.is_ok());
+
+    let path = mmdb_path.unwrap();
+    assert!(path.to_string_lossy().ends_with("GeoLite2-City.mmdb"));
+  }
+
+  #[test]
+  fn test_is_geoip_database_available() {
+    // This test will return false unless the database actually exists
+    // In a real environment, this would check the actual file system
+    let is_available = GeoIPDownloader::is_geoip_database_available();
+    // We can't assert a specific value since it depends on the system state
+    // But we can verify the function doesn't panic
+    println!("GeoIP database available: {is_available}");
+  }
+}

src-tauri/src/profile/manager.rs

@@ -335,20 +335,30 @@ impl ProfileManager {
     Ok(())
   }
 
-  pub fn update_camoufox_config(
+  pub async fn update_camoufox_config(
     &self,
+    app_handle: tauri::AppHandle,
     profile_name: &str,
     config: CamoufoxConfig,
-  ) -> Result<(), Box<dyn std::error::Error>> {
+  ) -> Result<(), Box<dyn std::error::Error + Send + Sync>> {
     // Find the profile by name
-    let profiles = self.list_profiles()?;
+    let profiles =
+      self
+        .list_profiles()
+        .map_err(|e| -> Box<dyn std::error::Error + Send + Sync> {
+          format!("Failed to list profiles: {e}").into()
+        })?;
     let mut profile = profiles
       .into_iter()
       .find(|p| p.name == profile_name)
-      .ok_or_else(|| format!("Profile {profile_name} not found"))?;
+      .ok_or_else(|| -> Box<dyn std::error::Error + Send + Sync> {
+        format!("Profile {profile_name} not found").into()
+      })?;
 
-    // Check if the browser is currently running
-    if profile.process_id.is_some() {
+    // Check if the browser is currently running using the comprehensive status check
+    let is_running = self.check_browser_status(app_handle, &profile).await?;
+
+    if is_running {
       return Err(
         "Cannot update Camoufox configuration while browser is running. Please stop the browser first.".into(),
       );
@@ -358,7 +368,11 @@ impl ProfileManager {
     profile.camoufox_config = Some(config);
 
     // Save the updated profile
-    self.save_profile(&profile)?;
+    self
+      .save_profile(&profile)
+      .map_err(|e| -> Box<dyn std::error::Error + Send + Sync> {
+        format!("Failed to save profile: {e}").into()
+      })?;
 
     println!("Camoufox configuration updated for profile '{profile_name}'.");
 
@@ -433,9 +447,6 @@ impl ProfileManager {
                   .map_err(|e| format!("Failed to update profile proxy: {e}"))?;
 
                 println!("Successfully started proxy for profile: {}", profile.name);
-
-                // Give the proxy a moment to fully start up
-                tokio::time::sleep(tokio::time::Duration::from_millis(500)).await;
               }
               Err(e) => {
                 eprintln!("Failed to start proxy: {e}");
@@ -498,10 +509,14 @@ impl ProfileManager {
     app_handle: tauri::AppHandle,
     profile: &BrowserProfile,
   ) -> Result<bool, Box<dyn std::error::Error + Send + Sync>> {
-    // Handle camoufox profiles using the same fast approach as other browsers
-    // No special handling needed - camoufox uses the same process checking logic
+    // Handle Camoufox profiles using nodecar-based status checking
+    if profile.browser == "camoufox" {
+      return self
+        .check_camoufox_status_via_nodecar(&app_handle, profile)
+        .await;
+    }
 
-    // For non-camoufox browsers, use the existing logic
+    // For non-camoufox browsers, use the existing PID-based logic
     let mut inner_profile = profile.clone();
     let system = System::new_all();
     let mut is_running = false;
@@ -517,12 +532,8 @@ impl ProfileManager {
         let profile_data_path_str = profile_data_path.to_string_lossy();
         let profile_path_match = cmd.iter().any(|s| {
           let arg = s.to_str().unwrap_or("");
-          // For Firefox-based browsers (including camoufox), check for exact profile path match
-          if profile.browser == "camoufox" {
-            // Camoufox uses user_data_dir like Chromium browsers
-            arg.contains(&format!("--user-data-dir={profile_data_path_str}"))
-              || arg == profile_data_path_str
-          } else if profile.browser == "tor-browser"
+          // For Firefox-based browsers, check for exact profile path match
+          if profile.browser == "tor-browser"
             || profile.browser == "firefox"
             || profile.browser == "firefox-developer"
             || profile.browser == "mullvad-browser"
@@ -577,13 +588,7 @@ impl ProfileManager {
             "zen" => exe_name.contains("zen"),
             "chromium" => exe_name.contains("chromium"),
             "brave" => exe_name.contains("brave"),
-            "camoufox" => {
-              exe_name.contains("camoufox")
-                || (exe_name.contains("firefox")
-                  && cmd
-                    .iter()
-                    .any(|arg| arg.to_str().unwrap_or("").contains("camoufox")))
-            }
+            // Camoufox is handled via nodecar, not PID-based checking
             _ => false,
           };
 
@@ -660,6 +665,77 @@ impl ProfileManager {
     Ok(is_running)
   }
 
+  // Check Camoufox status using nodecar-based approach
+  async fn check_camoufox_status_via_nodecar(
+    &self,
+    app_handle: &tauri::AppHandle,
+    profile: &BrowserProfile,
+  ) -> Result<bool, Box<dyn std::error::Error + Send + Sync>> {
+    use crate::camoufox::CamoufoxNodecarLauncher;
+
+    let launcher = CamoufoxNodecarLauncher::new(app_handle.clone());
+    let profiles_dir = self.get_profiles_dir();
+    let profile_data_path = profile.get_profile_data_path(&profiles_dir);
+    let profile_path_str = profile_data_path.to_string_lossy();
+
+    // Check if there's a running Camoufox instance for this profile
+    match launcher.find_camoufox_by_profile(&profile_path_str).await {
+      Ok(Some(camoufox_process)) => {
+        // Found a running instance, update profile with process info
+        let mut updated_profile = profile.clone();
+        updated_profile.process_id = camoufox_process.port;
+        if let Err(e) = self.save_profile(&updated_profile) {
+          println!("Warning: Failed to update Camoufox profile with process info: {e}");
+        }
+
+        // Emit profile update event to frontend
+        if let Err(e) = app_handle.emit("profile-updated", &updated_profile) {
+          println!("Warning: Failed to emit profile update event: {e}");
+        }
+
+        println!(
+          "Camoufox profile '{}' is running with PID: {:?}",
+          profile.name, camoufox_process.port
+        );
+        Ok(true)
+      }
+      Ok(None) => {
+        // No running instance found, clear process ID if set
+        if profile.process_id.is_some() {
+          let mut updated_profile = profile.clone();
+          updated_profile.process_id = None;
+          if let Err(e) = self.save_profile(&updated_profile) {
+            println!("Warning: Failed to clear Camoufox profile process info: {e}");
+          }
+
+          // Emit profile update event to frontend
+          if let Err(e) = app_handle.emit("profile-updated", &updated_profile) {
+            println!("Warning: Failed to emit profile update event: {e}");
+          }
+        }
+        println!("Camoufox profile '{}' is not running", profile.name);
+        Ok(false)
+      }
+      Err(e) => {
+        // Error checking status, assume not running and clear process ID
+        println!("Warning: Failed to check Camoufox status via nodecar: {e}");
+        if profile.process_id.is_some() {
+          let mut updated_profile = profile.clone();
+          updated_profile.process_id = None;
+          if let Err(e) = self.save_profile(&updated_profile) {
+            println!("Warning: Failed to clear Camoufox profile process info after error: {e}");
+          }
+
+          // Emit profile update event to frontend
+          if let Err(e) = app_handle.emit("profile-updated", &updated_profile) {
+            println!("Warning: Failed to emit profile update event: {e}");
+          }
+        }
+        Ok(false)
+      }
+    }
+  }
+
   // Helper function to check if a process matches TOR/Mullvad browser
   fn is_tor_or_mullvad_browser(
     &self,