style: return Err instead of panic on storer invalid paths (#987)

* style: return Err instead of panic on storer invalid paths

This is a quick fix. The errors in the `fs` crate should probably be improved when we have more bandwidth.

* fix(test): clone impl

Author: DavSanchez

agent-control/src/opamp/instance_id/on_host/storer.rs

@@ -4,6 +4,7 @@ use crate::opamp::instance_id::getter::DataStored;
 use crate::opamp::instance_id::storer::InstanceIDStorer;
 use fs::directory_manager::{DirectoryManagementError, DirectoryManager, DirectoryManagerFs};
 use fs::file_reader::{FileReader, FileReaderError};
+use fs::utils::FsError;
 use fs::writer_file::{FileWriter, WriteError};
 use fs::LocalFile;
 use std::fs::Permissions;
@@ -88,7 +89,10 @@ where
         // Get a ref to the target file's parent directory
         let dest_dir = dest_file
             .parent()
-            .expect("parent directory dest_file should be valid (not empty nor root dir)");
+            .ok_or(WriteError::from(FsError::InvalidPath(format!(
+                "no parent directory found for {} (empty or root dir)",
+                dest_file.display()
+            ))))?;
 
         self.dir_manager
             .create(dest_dir, Permissions::from_mode(DIRECTORY_PERMISSIONS))?;

agent-control/src/sub_agent/persister/config_persister.rs

@@ -59,9 +59,9 @@ pub mod tests {
                         ErrorDeletingDirectory(delete_error.to_string()),
                     ),
                     InvalidDirectory(invalid_dir_error) => match invalid_dir_error {
-                        FsError::InvalidPath() => {
-                            PersistError::DirectoryError(InvalidDirectory(FsError::InvalidPath()))
-                        }
+                        FsError::InvalidPath(s) => PersistError::DirectoryError(InvalidDirectory(
+                            FsError::InvalidPath(s.to_string()),
+                        )),
                         FsError::DotsDisallowed(msg) => PersistError::DirectoryError(
                             InvalidDirectory(FsError::DotsDisallowed(msg.to_string())),
                         ),
@@ -76,9 +76,9 @@ pub mod tests {
                             )))
                         }
                         InvalidPath(fs_error) => match fs_error {
-                            FsError::InvalidPath() => {
-                                PersistError::FileError(InvalidPath(FsError::InvalidPath()))
-                            }
+                            FsError::InvalidPath(s) => PersistError::FileError(InvalidPath(
+                                FsError::InvalidPath(s.to_string()),
+                            )),
                             FsError::DotsDisallowed(path) => PersistError::FileError(InvalidPath(
                                 FsError::DotsDisallowed(path.to_string()),
                             )),

fs/src/utils.rs

@@ -4,8 +4,8 @@ use thiserror::Error;
 
 #[derive(Error, Debug, Clone)]
 pub enum FsError {
-    #[error("invalid path")]
-    InvalidPath(),
+    #[error("invalid path: `{0}`")]
+    InvalidPath(String),
 
     #[error("dots disallowed in path `{0}`")]
     DotsDisallowed(String),
@@ -14,7 +14,10 @@ pub enum FsError {
 #[cfg(target_family = "unix")]
 pub fn validate_path(path: &Path) -> Result<(), FsError> {
     match path.to_str() {
-        None => Err(FsError::InvalidPath()),
+        None => Err(FsError::InvalidPath(format!(
+            "{} is not valid unicode",
+            path.to_string_lossy()
+        ))),
         Some(valid_path) => {
             // disallow dots
             let dots_regex = Regex::new(r"\.\.").unwrap();