Refactor error handling in repository module to include structured error variants with detailed context.

- Replaced string-based `FileReadError`, `FileWriteError`, and `DirectoryCreateError` with structured error types using `PathBuf` and `source`.
- Updated all affected methods in `rest_backend`, `file_backend`, and tests to use the new structured error variants.
- Improved error messages for clarity and debugging by embedding file paths and sources directly into error data.
This commit is contained in:
Till Wegmueller 2025-12-08 23:13:27 +01:00
parent ee02bf3cf0
commit c4910bb434
No known key found for this signature in database
4 changed files with 46 additions and 35 deletions

View file

@ -342,20 +342,19 @@ impl Transaction {
if temp_file_path.exists() { if temp_file_path.exists() {
// If it exists, remove it to avoid any issues with existing content // If it exists, remove it to avoid any issues with existing content
fs::remove_file(&temp_file_path).map_err(|e| { fs::remove_file(&temp_file_path).map_err(|e| {
RepositoryError::FileWriteError(format!( RepositoryError::FileWriteError {
"Failed to remove existing temp file: {}", path: temp_file_path.clone(),
e source: e,
)) }
})?; })?;
} }
// Read the file content // Read the file content
let file_content = fs::read(file_path).map_err(|e| { let file_content = fs::read(file_path).map_err(|e| {
RepositoryError::FileReadError(format!( RepositoryError::FileReadError {
"Failed to read file {}: {}", path: file_path.to_path_buf(),
file_path.display(), source: e,
e }
))
})?; })?;
// Create a payload with the hash information if it doesn't exist // Create a payload with the hash information if it doesn't exist
@ -383,10 +382,10 @@ impl Transaction {
// Write the compressed data to the temp file // Write the compressed data to the temp file
fs::write(&temp_file_path, &compressed_data).map_err(|e| { fs::write(&temp_file_path, &compressed_data).map_err(|e| {
RepositoryError::FileWriteError(format!( RepositoryError::FileWriteError {
"Failed to write compressed data to temp file: {}", path: temp_file_path.clone(),
e source: e,
)) }
})?; })?;
// Calculate hash of the compressed data // Calculate hash of the compressed data
@ -410,10 +409,10 @@ impl Transaction {
// Write the compressed data to the temp file // Write the compressed data to the temp file
fs::write(&temp_file_path, &compressed_data).map_err(|e| { fs::write(&temp_file_path, &compressed_data).map_err(|e| {
RepositoryError::FileWriteError(format!( RepositoryError::FileWriteError {
"Failed to write LZ4 compressed data to temp file: {}", path: temp_file_path.clone(),
e source: e,
)) }
})?; })?;
// Calculate hash of the compressed data // Calculate hash of the compressed data
@ -663,7 +662,7 @@ impl ReadableRepository for FileBackend {
// Load the repository configuration // Load the repository configuration
let config_path = path.join(REPOSITORY_CONFIG_FILENAME); let config_path = path.join(REPOSITORY_CONFIG_FILENAME);
let config_data = fs::read_to_string(config_path)?; let config_data = fs::read_to_string(&config_path).map_err(|e| RepositoryError::ConfigReadError(format!("{}: {}", config_path.display(), e)))?;
let config: RepositoryConfig = serde_json::from_str(&config_data)?; let config: RepositoryConfig = serde_json::from_str(&config_data)?;
Ok(FileBackend { Ok(FileBackend {
@ -1280,7 +1279,7 @@ impl ReadableRepository for FileBackend {
// If destination already exists and matches digest, do nothing // If destination already exists and matches digest, do nothing
if dest.exists() { if dest.exists() {
let bytes = fs::read(dest)?; let bytes = fs::read(dest).map_err(|e| RepositoryError::FileReadError { path: dest.to_path_buf(), source: e })?;
match crate::digest::Digest::from_bytes(&bytes, algo.clone(), crate::digest::DigestSource::PrimaryPayloadHash) { match crate::digest::Digest::from_bytes(&bytes, algo.clone(), crate::digest::DigestSource::PrimaryPayloadHash) {
Ok(comp) if comp.hash == hash => return Ok(()), Ok(comp) if comp.hash == hash => return Ok(()),
_ => { /* fall through to overwrite */ } _ => { /* fall through to overwrite */ }
@ -1288,7 +1287,7 @@ impl ReadableRepository for FileBackend {
} }
// Read source content and verify digest // Read source content and verify digest
let bytes = fs::read(&source_path)?; let bytes = fs::read(&source_path).map_err(|e| RepositoryError::FileReadError { path: source_path.clone(), source: e })?;
match crate::digest::Digest::from_bytes(&bytes, algo, crate::digest::DigestSource::PrimaryPayloadHash) { match crate::digest::Digest::from_bytes(&bytes, algo, crate::digest::DigestSource::PrimaryPayloadHash) {
Ok(comp) => { Ok(comp) => {
if comp.hash != hash { if comp.hash != hash {
@ -1674,14 +1673,14 @@ impl FileBackend {
// Preferred path: publisher-scoped manifest path // Preferred path: publisher-scoped manifest path
let path = Self::construct_manifest_path(&self.path, publisher, fmri.stem(), &version); let path = Self::construct_manifest_path(&self.path, publisher, fmri.stem(), &version);
if path.exists() { if path.exists() {
return std::fs::read_to_string(&path).map_err(|e| RepositoryError::FileReadError(format!("{}", e))); return std::fs::read_to_string(&path).map_err(|e| RepositoryError::FileReadError { path, source: e });
} }
// Fallbacks: global pkg layout without publisher // Fallbacks: global pkg layout without publisher
let encoded_stem = Self::url_encode(fmri.stem()); let encoded_stem = Self::url_encode(fmri.stem());
let encoded_version = Self::url_encode(&version); let encoded_version = Self::url_encode(&version);
let alt1 = self.path.join("pkg").join(&encoded_stem).join(&encoded_version); let alt1 = self.path.join("pkg").join(&encoded_stem).join(&encoded_version);
if alt1.exists() { if alt1.exists() {
return std::fs::read_to_string(&alt1).map_err(|e| RepositoryError::FileReadError(format!("{}", e))); return std::fs::read_to_string(&alt1).map_err(|e| RepositoryError::FileReadError { path: alt1, source: e });
} }
let alt2 = self let alt2 = self
.path .path
@ -1691,7 +1690,7 @@ impl FileBackend {
.join(&encoded_stem) .join(&encoded_stem)
.join(&encoded_version); .join(&encoded_version);
if alt2.exists() { if alt2.exists() {
return std::fs::read_to_string(&alt2).map_err(|e| RepositoryError::FileReadError(format!("{}", e))); return std::fs::read_to_string(&alt2).map_err(|e| RepositoryError::FileReadError { path: alt2, source: e });
} }
Err(RepositoryError::NotFound(format!("manifest for {} not found", fmri))) Err(RepositoryError::NotFound(format!("manifest for {} not found", fmri)))
} }
@ -2079,7 +2078,7 @@ impl FileBackend {
} }
// Read the manifest content for hash calculation // Read the manifest content for hash calculation
let manifest_content = fs::read_to_string(&manifest_path)?; let manifest_content = fs::read_to_string(&manifest_path).map_err(|e| RepositoryError::FileReadError { path: manifest_path.clone(), source: e })?;
// Parse the manifest using parse_file which handles JSON correctly // Parse the manifest using parse_file which handles JSON correctly
let manifest = Manifest::parse_file(&manifest_path)?; let manifest = Manifest::parse_file(&manifest_path)?;

View file

@ -6,7 +6,7 @@
use miette::Diagnostic; use miette::Diagnostic;
use std::collections::HashMap; use std::collections::HashMap;
use std::io; use std::io;
use std::path::{Path, StripPrefixError}; use std::path::{Path, PathBuf, StripPrefixError};
use thiserror::Error; use thiserror::Error;
/// Result type for repository operations /// Result type for repository operations
@ -57,26 +57,38 @@ pub enum RepositoryError {
)] )]
ConfigWriteError(String), ConfigWriteError(String),
#[error("failed to create directory: {0}")] #[error("failed to create directory {path}: {source}")]
#[diagnostic( #[diagnostic(
code(ips::repository_error::directory_create), code(ips::repository_error::directory_create),
help("Check that the parent directory exists and is writable") help("Check that the parent directory exists and is writable")
)] )]
DirectoryCreateError(String), DirectoryCreateError {
path: PathBuf,
#[source]
source: io::Error,
},
#[error("failed to read file: {0}")] #[error("failed to read file {path}: {source}")]
#[diagnostic( #[diagnostic(
code(ips::repository_error::file_read), code(ips::repository_error::file_read),
help("Check that the file exists and is readable") help("Check that the file exists and is readable")
)] )]
FileReadError(String), FileReadError {
path: PathBuf,
#[source]
source: io::Error,
},
#[error("failed to write file: {0}")] #[error("failed to write file {path}: {source}")]
#[diagnostic( #[diagnostic(
code(ips::repository_error::file_write), code(ips::repository_error::file_write),
help("Check that the directory is writable") help("Check that the directory is writable")
)] )]
FileWriteError(String), FileWriteError {
path: PathBuf,
#[source]
source: io::Error,
},
#[error("failed to parse JSON: {0}")] #[error("failed to parse JSON: {0}")]
#[diagnostic( #[diagnostic(

View file

@ -917,14 +917,14 @@ impl RestBackend {
.map_err(|e| { .map_err(|e| {
// Report failure // Report failure
progress.finish(&progress_info); progress.finish(&progress_info);
RepositoryError::FileWriteError(format!("Failed to create file: {}", e)) RepositoryError::FileWriteError { path: file_path.clone(), source: e }
})?; })?;
file.write_all(&content) file.write_all(&content)
.map_err(|e| { .map_err(|e| {
// Report failure // Report failure
progress.finish(&progress_info); progress.finish(&progress_info);
RepositoryError::FileWriteError(format!("Failed to write file: {}", e)) RepositoryError::FileWriteError { path: file_path.clone(), source: e }
})?; })?;
debug!("Stored catalog file: {}", file_path.display()); debug!("Stored catalog file: {}", file_path.display());
@ -981,7 +981,7 @@ impl RestBackend {
let attrs_content = fs::read_to_string(&attrs_path) let attrs_content = fs::read_to_string(&attrs_path)
.map_err(|e| { .map_err(|e| {
progress_reporter.finish(&overall_progress); progress_reporter.finish(&overall_progress);
RepositoryError::FileReadError(format!("Failed to read catalog.attrs: {}", e)) RepositoryError::FileReadError { path: attrs_path.clone(), source: e }
})?; })?;
let attrs: Value = serde_json::from_str(&attrs_content) let attrs: Value = serde_json::from_str(&attrs_content)

View file

@ -81,7 +81,7 @@ mod tests {
// Check if the manifest file exists // Check if the manifest file exists
if !manifest_path.exists() { if !manifest_path.exists() {
println!("Error: Manifest file does not exist"); println!("Error: Manifest file does not exist");
return Err(RepositoryError::FileReadError(format!( return Err(RepositoryError::NotFound(format!(
"Manifest file does not exist: {}", "Manifest file does not exist: {}",
manifest_path.display() manifest_path.display()
))); )));