refactor(java): improve error handling and logging

- Extract JavaError to dedicated error.rs module
- Add serde defaults for JavaInstallation optional fields
- Replace unwrap() with proper error propagation
- Add detailed logging for Java resolution priority chain
- Improve error mapping in validation (NotFound vs VerificationFailed)

Reviewed-by: Claude Sonnet 4.5

4 files changed, 139 insertions, 20 deletions

diff --git a/src-tauri/src/core/java/error.rs b/src-tauri/src/core/java/error.rs
new file mode 100644
index 0000000..bf78d3b
--- /dev/null
+++ b/src-tauri/src/core/java/error.rs
@@ -0,0 +1,95 @@
+use std::fmt;
+
+/// Unified error type for Java component operations
+///
+/// This enum represents all possible errors that can occur in the Java component,
+/// providing a consistent error handling interface across all modules.
+#[derive(Debug, Clone)]
+pub enum JavaError {
+    // Java installation not found at the specified path
+    NotFound,
+    // Invalid Java version format or unable to parse version
+    InvalidVersion(String),
+    // Java installation verification failed (e.g., -version command failed)
+    VerificationFailed(String),
+    // Network error during API calls or downloads
+    NetworkError(String),
+    // File I/O error (reading, writing, or accessing files)
+    IoError(String),
+    // Timeout occurred during operation
+    Timeout(String),
+    // Serialization/deserialization error
+    SerializationError(String),
+    // Invalid configuration or parameters
+    InvalidConfig(String),
+    // Download or installation failed
+    DownloadFailed(String),
+    // Extraction or decompression failed
+    ExtractionFailed(String),
+    // Checksum verification failed
+    ChecksumMismatch(String),
+    // Other unspecified errors
+    Other(String),
+}
+
+impl fmt::Display for JavaError {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        match self {
+            JavaError::NotFound => write!(f, "Java installation not found"),
+            JavaError::InvalidVersion(msg) => write!(f, "Invalid Java version: {}", msg),
+            JavaError::VerificationFailed(msg) => write!(f, "Java verification failed: {}", msg),
+            JavaError::NetworkError(msg) => write!(f, "Network error: {}", msg),
+            JavaError::IoError(msg) => write!(f, "I/O error: {}", msg),
+            JavaError::Timeout(msg) => write!(f, "Operation timeout: {}", msg),
+            JavaError::SerializationError(msg) => write!(f, "Serialization error: {}", msg),
+            JavaError::InvalidConfig(msg) => write!(f, "Invalid configuration: {}", msg),
+            JavaError::DownloadFailed(msg) => write!(f, "Download failed: {}", msg),
+            JavaError::ExtractionFailed(msg) => write!(f, "Extraction failed: {}", msg),
+            JavaError::ChecksumMismatch(msg) => write!(f, "Checksum mismatch: {}", msg),
+            JavaError::Other(msg) => write!(f, "{}", msg),
+        }
+    }
+}
+
+impl std::error::Error for JavaError {}
+
+/// Convert JavaError to String for Tauri command results
+impl From<JavaError> for String {
+    fn from(err: JavaError) -> Self {
+        err.to_string()
+    }
+}
+
+/// Convert std::io::Error to JavaError
+impl From<std::io::Error> for JavaError {
+    fn from(err: std::io::Error) -> Self {
+        JavaError::IoError(err.to_string())
+    }
+}
+
+/// Convert serde_json::Error to JavaError
+impl From<serde_json::Error> for JavaError {
+    fn from(err: serde_json::Error) -> Self {
+        JavaError::SerializationError(err.to_string())
+    }
+}
+
+/// Convert reqwest::Error to JavaError
+impl From<reqwest::Error> for JavaError {
+    fn from(err: reqwest::Error) -> Self {
+        if err.is_timeout() {
+            JavaError::Timeout(err.to_string())
+        } else if err.is_connect() || err.is_request() {
+            JavaError::NetworkError(err.to_string())
+        } else {
+            JavaError::NetworkError(err.to_string())
+        }
+    }
+}
+
+/// Convert String to JavaError
+impl From<String> for JavaError {
+    fn from(err: String) -> Self {
+        JavaError::Other(err)
+    }
+}
diff --git a/src-tauri/src/core/java/persistence.rs b/src-tauri/src/core/java/persistence.rs
index d1e999e..fd81394 100644
--- a/src-tauri/src/core/java/persistence.rs
+++ b/src-tauri/src/core/java/persistence.rs
@@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize};
 use std::path::PathBuf;
 use tauri::{AppHandle, Manager};
 
+use super::error::JavaError;
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct JavaConfig {
     pub user_defined_paths: Vec<String>,
@@ -34,26 +36,43 @@ pub fn load_java_config(app_handle: &AppHandle) -> JavaConfig {
     }
 
     match std::fs::read_to_string(&config_path) {
-        Ok(content) => serde_json::from_str(&content).unwrap_or_default(),
-        Err(_) => JavaConfig::default(),
+        Ok(content) => match serde_json::from_str(&content) {
+            Ok(config) => config,
+            Err(err) => {
+                // Log the error but don't panic - return default config
+                log::warn!(
+                    "Failed to parse Java config at {}: {}. Using default configuration.",
+                    config_path.display(),
+                    err
+                );
+                JavaConfig::default()
+            }
+        },
+        Err(err) => {
+            log::warn!(
+                "Failed to read Java config at {}: {}. Using default configuration.",
+                config_path.display(),
+                err
+            );
+            JavaConfig::default()
+        }
     }
 }
 
-pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), String> {
+pub fn save_java_config(app_handle: &AppHandle, config: &JavaConfig) -> Result<(), JavaError> {
     let config_path = get_java_config_path(app_handle);
-    let content = serde_json::to_string_pretty(config).map_err(|e| e.to_string())?;
-    std::fs::create_dir_all(
-        config_path
-            .parent()
-            .expect("Java config path should have a parent directory"),
-    )
-    .map_err(|e| e.to_string())?;
-    std::fs::write(&config_path, content).map_err(|e| e.to_string())?;
+    let content = serde_json::to_string_pretty(config)?;
+
+    std::fs::create_dir_all(config_path.parent().ok_or_else(|| {
+        JavaError::InvalidConfig("Java config path has no parent directory".to_string())
+    })?)?;
+
+    std::fs::write(&config_path, content)?;
     Ok(())
 }
 
 #[allow(dead_code)]
-pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), String> {
+pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(), JavaError> {
     let mut config = load_java_config(app_handle);
     if !config.user_defined_paths.contains(&path) {
         config.user_defined_paths.push(path);
@@ -62,14 +81,17 @@ pub fn add_user_defined_path(app_handle: &AppHandle, path: String) -> Result<(),
 }
 
 #[allow(dead_code)]
-pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), String> {
+pub fn remove_user_defined_path(app_handle: &AppHandle, path: &str) -> Result<(), JavaError> {
     let mut config = load_java_config(app_handle);
     config.user_defined_paths.retain(|p| p != path);
     save_java_config(app_handle, &config)
 }
 
 #[allow(dead_code)]
-pub fn set_preferred_java_path(app_handle: &AppHandle, path: Option<String>) -> Result<(), String> {
+pub fn set_preferred_java_path(
+    app_handle: &AppHandle,
+    path: Option<String>,
+) -> Result<(), JavaError> {
     let mut config = load_java_config(app_handle);
     config.preferred_java_path = path;
     save_java_config(app_handle, &config)
@@ -82,11 +104,11 @@ pub fn get_preferred_java_path(app_handle: &AppHandle) -> Option<String> {
 }
 
 #[allow(dead_code)]
-pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), String> {
+pub fn update_last_detection_time(app_handle: &AppHandle) -> Result<(), JavaError> {
     let mut config = load_java_config(app_handle);
     config.last_detection_time = std::time::SystemTime::now()
         .duration_since(std::time::UNIX_EPOCH)
-        .unwrap()
+        .map_err(|e| JavaError::Other(format!("System time error: {}", e)))?
         .as_secs();
     save_java_config(app_handle, &config)
 }
diff --git a/src-tauri/src/core/java/providers/adoptium.rs b/src-tauri/src/core/java/providers/adoptium.rs
index 4b06721..40e1757 100644
--- a/src-tauri/src/core/java/providers/adoptium.rs
+++ b/src-tauri/src/core/java/providers/adoptium.rs
@@ -1,5 +1,6 @@
+use crate::core::java::error::JavaError;
 use crate::core::java::provider::JavaProvider;
-use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaError, JavaReleaseInfo};
+use crate::core::java::{ImageType, JavaCatalog, JavaDownloadInfo, JavaReleaseInfo};
 use serde::Deserialize;
 use tauri::AppHandle;
 
@@ -183,7 +184,10 @@ impl JavaProvider for AdoptiumProvider {
                     // Task completed but returned None, should not happen in current implementation
                 }
                 Err(e) => {
-                    eprintln!("AdoptiumProvider::fetch_catalog task join error: {:?}", e);
+                    return Err(JavaError::NetworkError(format!(
+                        "Failed to join Adoptium catalog fetch task: {}",
+                        e
+                    )));
                 }
             }
         }
diff --git a/src-tauri/src/core/mod.rs b/src-tauri/src/core/mod.rs
index 12dff7c..dcbd47a 100644
--- a/src-tauri/src/core/mod.rs
+++ b/src-tauri/src/core/mod.rs
@@ -12,5 +12,3 @@ pub mod manifest;
 pub mod maven;
 pub mod rules;
 pub mod version_merge;
-
-pub use java::JavaInstallation;