feat(java): Enhance Java detection and error handling

- Added support for detecting Java installations from SDKMAN! in `find_sdkman_java`.
- Improved `run_which_command_with_timeout` to handle command timeouts gracefully.
- Introduced a unified `JavaError` enum for consistent error handling across Java operations.
- Updated functions to return `Result` types instead of `Option` for better error reporting.
- Enhanced `load_cached_catalog` and `save_catalog_cache` to use `JavaError`.
- Refactored `fetch_java_catalog`, `fetch_java_release`, and `fetch_available_versions` to return `JavaError`.
- Improved validation functions to return detailed errors when checking Java installations.
- Added tests for version parsing and compatibility checks.
- Updated `resolve_java_for_launch` to handle instance-specific and global Java paths.

4 files changed, 121 insertions, 33 deletions

diff --git a/src-tauri/src/core/java/detection.rs b/src-tauri/src/core/java/detection.rs
index 512769b..95e7803 100644
--- a/src-tauri/src/core/java/detection.rs
+++ b/src-tauri/src/core/java/detection.rs
@@ -10,6 +10,13 @@ use super::strip_unc_prefix;
 
 const WHICH_TIMEOUT: Duration = Duration::from_secs(2);
 
+/// Finds Java installation from SDKMAN! if available
+///
+/// Checks the standard SDKMAN! installation path:
+/// `~/.sdkman/candidates/java/current/bin/java`
+///
+/// # Returns
+/// `Some(PathBuf)` if SDKMAN! Java is found and exists, `None` otherwise
 pub fn find_sdkman_java() -> Option<PathBuf> {
     let home = std::env::var("HOME").ok()?;
     let sdkman_path = PathBuf::from(&home).join(".sdkman/candidates/java/current/bin/java");
@@ -20,17 +27,42 @@ pub fn find_sdkman_java() -> Option<PathBuf> {
     }
 }
 
+/// Runs `which` (Unix) or `where` (Windows) command to find Java in PATH with timeout
+///
+/// This function spawns a subprocess to locate the `java` executable in the system PATH.
+/// It enforces a 2-second timeout to prevent hanging if the command takes too long.
+///
+/// # Returns
+/// `Some(String)` containing the output (paths separated by newlines) if successful,
+/// `None` if the command fails, times out, or returns non-zero exit code
+///
+/// # Platform-specific behavior
+/// - Unix/Linux/macOS: Uses `which java`
+/// - Windows: Uses `where java` and hides the console window
+///
+/// # Timeout Behavior
+/// If the command does not complete within 2 seconds, the process is killed
+/// and `None` is returned. This prevents the launcher from hanging on systems
+/// where `which`/`where` may be slow or unresponsive.
 fn run_which_command_with_timeout() -> Option<String> {
     let mut cmd = Command::new(if cfg!(windows) { "where" } else { "which" });
     cmd.arg("java");
-    // Hide console window
+    // Hide console window on Windows
     #[cfg(target_os = "windows")]
     cmd.creation_flags(0x08000000);
     cmd.stdout(Stdio::piped());
 
     let mut child = cmd.spawn().ok()?;
+    let start = std::time::Instant::now();
 
     loop {
+        // Check if timeout has been exceeded
+        if start.elapsed() > WHICH_TIMEOUT {
+            let _ = child.kill();
+            let _ = child.wait();
+            return None;
+        }
+
         match child.try_wait() {
             Ok(Some(status)) => {
                 if status.success() {
@@ -45,6 +77,7 @@ fn run_which_command_with_timeout() -> Option<String> {
                 }
             }
             Ok(None) => {
+                // Command still running, sleep briefly before checking again
                 std::thread::sleep(Duration::from_millis(50));
             }
             Err(_) => {
@@ -56,10 +89,30 @@ fn run_which_command_with_timeout() -> Option<String> {
     }
 }
 
+/// Detects all available Java installations on the system
+///
+/// This function searches for Java installations in multiple locations:
+/// - **All platforms**: `JAVA_HOME` environment variable, `java` in PATH
+/// - **Linux**: `/usr/lib/jvm`, `/usr/java`, `/opt/java`, `/opt/jdk`, `/opt/openjdk`, SDKMAN!
+/// - **macOS**: `/Library/Java/JavaVirtualMachines`, `/System/Library/Java/JavaVirtualMachines`,
+///   Homebrew paths (`/usr/local/opt/openjdk`, `/opt/homebrew/opt/openjdk`), SDKMAN!
+/// - **Windows**: `Program Files`, `Program Files (x86)`, `LOCALAPPDATA` for various JDK distributions
+///
+/// # Returns
+/// A vector of `PathBuf` pointing to Java executables found on the system.
+/// Note: Paths may include symlinks and duplicates; callers should canonicalize and deduplicate as needed.
+///
+/// # Examples
+/// ```ignore
+/// let candidates = get_java_candidates();
+/// for java_path in candidates {
+///     println!("Found Java at: {}", java_path.display());
+/// }
+/// ```
 pub fn get_java_candidates() -> Vec<PathBuf> {
     let mut candidates = Vec::new();
 
-    // Only attempt 'which' or 'where' if is not Windows
+    // Try to find Java in PATH using 'which' or 'where' command with timeout
     // CAUTION: linux 'which' may return symlinks, so we need to canonicalize later
     if let Some(paths_str) = run_which_command_with_timeout() {
         for line in paths_str.lines() {
@@ -93,7 +146,6 @@ pub fn get_java_candidates() -> Vec<PathBuf> {
             }
         }
 
-        let home = std::env::var("HOME").unwrap_or_default();
         // Check common SDKMAN! java candidates
         if let Some(sdkman_java) = find_sdkman_java() {
             candidates.push(sdkman_java);
@@ -182,7 +234,7 @@ pub fn get_java_candidates() -> Vec<PathBuf> {
         }
     }
 
-    // Check JAVA_HOME java candidate
+    // Check JAVA_HOME environment variable
     if let Ok(java_home) = std::env::var("JAVA_HOME") {
         let bin_name = if cfg!(windows) { "java.exe" } else { "java" };
         let java_path = PathBuf::from(&java_home).join("bin").join(bin_name);
diff --git a/src-tauri/src/core/java/provider.rs b/src-tauri/src/core/java/provider.rs
index 1b79681..8aa0a0d 100644
--- a/src-tauri/src/core/java/provider.rs
+++ b/src-tauri/src/core/java/provider.rs
@@ -1,23 +1,47 @@
-use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo};
+use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaError};
 use tauri::AppHandle;
 
+/// Trait for Java distribution providers (e.g., Adoptium, Corretto)
+///
+/// Implementations handle fetching Java catalogs and release information
+/// from different distribution providers.
 pub trait JavaProvider: Send + Sync {
     /// Fetch the Java catalog (all available versions for this provider)
+    ///
+    /// # Arguments
+    /// * `app_handle` - The Tauri app handle for cache access
+    /// * `force_refresh` - If true, bypass cache and fetch fresh data
+    ///
+    /// # Returns
+    /// * `Ok(JavaCatalog)` with available versions
+    /// * `Err(JavaError)` if fetch or parsing fails
     async fn fetch_catalog(
         &self,
         app_handle: &AppHandle,
         force_refresh: bool,
-    ) -> Result<JavaCatalog, String>;
+    ) -> Result<JavaCatalog, JavaError>;
 
     /// Fetch a specific Java release
+    ///
+    /// # Arguments
+    /// * `major_version` - The major version number (e.g., 17, 21)
+    /// * `image_type` - Whether to fetch JRE or JDK
+    ///
+    /// # Returns
+    /// * `Ok(JavaDownloadInfo)` with download details
+    /// * `Err(JavaError)` if fetch or parsing fails
     async fn fetch_release(
         &self,
         major_version: u32,
         image_type: ImageType,
-    ) -> Result<JavaDownloadInfo, String>;
+    ) -> Result<JavaDownloadInfo, JavaError>;
 
     /// Get list of available major versions
-    async fn available_versions(&self) -> Result<Vec<u32>, String>;
+    ///
+    /// # Returns
+    /// * `Ok(Vec<u32>)` with available major versions
+    /// * `Err(JavaError)` if fetch fails
+    async fn available_versions(&self) -> Result<Vec<u32>, JavaError>;
 
     /// Get provider name (e.g., "adoptium", "corretto")
     #[allow(dead_code)]
diff --git a/src-tauri/src/core/java/providers/adoptium.rs b/src-tauri/src/core/java/providers/adoptium.rs
index aac2bf2..13ef2a5 100644
--- a/src-tauri/src/core/java/providers/adoptium.rs
+++ b/src-tauri/src/core/java/providers/adoptium.rs
@@ -1,5 +1,5 @@
 use crate::core::java::provider::JavaProvider;
-use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaReleaseInfo};
+use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaError, JavaReleaseInfo};
 use serde::Deserialize;
 use tauri::AppHandle;
 
@@ -69,9 +69,9 @@ impl JavaProvider for AdoptiumProvider {
         &self,
         app_handle: &AppHandle,
         force_refresh: bool,
-    ) -> Result<JavaCatalog, String> {
+    ) -> Result<JavaCatalog, JavaError> {
         if !force_refresh {
-            if let Some(cached) = super::super::load_cached_catalog(app_handle) {
+            if let Ok(Some(cached)) = crate::core::java::load_cached_catalog(app_handle) {
                 return Ok(cached);
             }
         }
@@ -86,10 +86,14 @@ impl JavaProvider for AdoptiumProvider {
             .header("Accept", "application/json")
             .send()
             .await
-            .map_err(|e| format!("Failed to fetch available releases: {}", e))?
+            .map_err(|e| {
+                JavaError::NetworkError(format!("Failed to fetch available releases: {}", e))
+            })?
             .json()
             .await
-            .map_err(|e| format!("Failed to parse available releases: {}", e))?;
+            .map_err(|e| {
+                JavaError::SerializationError(format!("Failed to parse available releases: {}", e))
+            })?;
 
         // Parallelize HTTP requests for better performance
         let mut fetch_tasks = Vec::new();
@@ -205,7 +209,7 @@ impl JavaProvider for AdoptiumProvider {
         &self,
         major_version: u32,
         image_type: ImageType,
-    ) -> Result<JavaDownloadInfo, String> {
+    ) -> Result<JavaDownloadInfo, JavaError> {
         let os = self.os_name();
         let arch = self.arch_name();
 
@@ -220,24 +224,23 @@ impl JavaProvider for AdoptiumProvider {
             .header("Accept", "application/json")
             .send()
             .await
-            .map_err(|e| format!("Network request failed: {}", e))?;
+            .map_err(|e| JavaError::NetworkError(format!("Network request failed: {}", e)))?;
 
         if !response.status().is_success() {
-            return Err(format!(
+            return Err(JavaError::NetworkError(format!(
                 "Adoptium API returned error: {} - The version/platform might be unavailable",
                 response.status()
-            ));
+            )));
         }
 
-        let assets: Vec<AdoptiumAsset> = response
-            .json()
-            .await
-            .map_err(|e| format!("Failed to parse API response: {}", e))?;
+        let assets: Vec<AdoptiumAsset> = response.json().await.map_err(|e| {
+            JavaError::SerializationError(format!("Failed to parse API response: {}", e))
+        })?;
 
         let asset = assets
             .into_iter()
             .next()
-            .ok_or_else(|| format!("Java {} {} download not found", major_version, image_type))?;
+            .ok_or_else(|| JavaError::NotFound)?;
 
         Ok(JavaDownloadInfo {
             version: asset.version.semver.clone(),
@@ -250,17 +253,16 @@ impl JavaProvider for AdoptiumProvider {
         })
     }
 
-    async fn available_versions(&self) -> Result<Vec<u32>, String> {
+    async fn available_versions(&self) -> Result<Vec<u32>, JavaError> {
         let url = format!("{}/info/available_releases", ADOPTIUM_API_BASE);
 
         let response = reqwest::get(url)
             .await
-            .map_err(|e| format!("Network request failed: {}", e))?;
+            .map_err(|e| JavaError::NetworkError(format!("Network request failed: {}", e)))?;
 
-        let releases: AvailableReleases = response
-            .json()
-            .await
-            .map_err(|e| format!("Failed to parse response: {}", e))?;
+        let releases: AvailableReleases = response.json().await.map_err(|e| {
+            JavaError::SerializationError(format!("Failed to parse response: {}", e))
+        })?;
 
         Ok(releases.available_releases)
     }
diff --git a/src-tauri/src/main.rs b/src-tauri/src/main.rs
index e0a71b5..b74c746 100644
--- a/src-tauri/src/main.rs
+++ b/src-tauri/src/main.rs
@@ -1549,7 +1549,9 @@ async fn fetch_adoptium_java(
         "jdk" => core::java::ImageType::Jdk,
         _ => core::java::ImageType::Jre,
     };
-    core::java::fetch_java_release(major_version, img_type).await
+    core::java::fetch_java_release(major_version, img_type)
+        .await
+        .map_err(|e| e.to_string())
 }
 
 /// Download and install Adoptium Java
@@ -1565,13 +1567,17 @@ async fn download_adoptium_java(
         _ => core::java::ImageType::Jre,
     };
     let path = custom_path.map(std::path::PathBuf::from);
-    core::java::download_and_install_java(&app_handle, major_version, img_type, path).await
+    core::java::download_and_install_java(&app_handle, major_version, img_type, path)
+        .await
+        .map_err(|e| e.to_string())
 }
 
 /// Get available Adoptium Java versions
 #[tauri::command]
 async fn fetch_available_java_versions() -> Result<Vec<u32>, String> {
-    core::java::fetch_available_versions().await
+    core::java::fetch_available_versions()
+        .await
+        .map_err(|e| e.to_string())
 }
 
 /// Fetch Java catalog with platform availability (uses cache)
@@ -1579,7 +1585,9 @@ async fn fetch_available_java_versions() -> Result<Vec<u32>, String> {
 async fn fetch_java_catalog(
     app_handle: tauri::AppHandle,
 ) -> Result<core::java::JavaCatalog, String> {
-    core::java::fetch_java_catalog(&app_handle, false).await
+    core::java::fetch_java_catalog(&app_handle, false)
+        .await
+        .map_err(|e| e.to_string())
 }
 
 /// Refresh Java catalog (bypass cache)
@@ -1587,7 +1595,9 @@ async fn fetch_java_catalog(
 async fn refresh_java_catalog(
     app_handle: tauri::AppHandle,
 ) -> Result<core::java::JavaCatalog, String> {
-    core::java::fetch_java_catalog(&app_handle, true).await
+    core::java::fetch_java_catalog(&app_handle, true)
+        .await
+        .map_err(|e| e.to_string())
 }
 
 /// Cancel current Java download