Merge pull request #425 from amazonlinux/block-party-errors

block-party: Use a full snafu error rather than an io::Error shim

workspaces/growpart/src/diskpart/error.rs

@@ -12,13 +12,13 @@ pub enum Error {
     #[snafu(display("Failed to find block device for '{}': {}", path.display(), source))]
     FindBlockDevice {
         path: std::path::PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Failed to find disk for '{}': {}", path.display(), source))]
     FindDisk {
         path: std::path::PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Expected partition for '{}'", path.display()))]

workspaces/updater/block-party/Cargo.toml

@@ -4,3 +4,6 @@ version = "0.1.0"
 authors = ["iliana destroyer of worlds <[email protected]>"]
 edition = "2018"
 publish = false
+
+[dependencies]
+snafu = "0.5.0"

workspaces/updater/block-party/src/lib.rs

@@ -9,14 +9,101 @@
 #![deny(missing_docs, rust_2018_idioms)]
 #![warn(clippy::pedantic)]
 
+use snafu::{ensure, OptionExt, ResultExt};
 use std::ffi::OsString;
 use std::fmt;
 use std::fs;
-use std::io::{Error, ErrorKind, Result};
+use std::io;
 use std::os::linux::fs::MetadataExt;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 
+mod error {
+    use snafu::Snafu;
+    use std::io;
+    use std::path::PathBuf;
+
+    #[derive(Debug, Snafu)]
+    #[snafu(visibility = "pub(super)")]
+    /// The error type for this library.
+    pub enum Error {
+        #[snafu(display("Target of {} ends in `..`", path.display()))]
+        /// The target of a link ends in `..`
+        LinkTargetFileName {
+            /// Contains the invalid link path.
+            path: PathBuf
+        },
+
+        #[snafu(display("Cannot parse {} as major/minor numbers: {}", path.display(), source))]
+        /// Can't parse the given path as major/minor numbers.
+        MajorMinorParseInt {
+            /// Contains the path we failed to parse as major/minor.
+            path: PathBuf,
+            /// The source error describing the parse failure.
+            source: std::num::ParseIntError,
+        },
+
+        #[snafu(display(
+                "Cannot parse {} as major/minor numbers: invalid number of colons",
+                path.display())
+            )]
+        /// Can't parse the given string as major/minor numbers because it has an invalid number
+        /// of colons.
+        MajorMinorLen {
+            /// Contains the path which in turn contains an invalid major/minor string.
+            path: PathBuf
+        },
+
+        #[snafu(display("Unable to read device name through link at {}: {} ", path.display(), source))]
+        /// Unable to read device name through the given link.
+        SysPathLinkRead {
+            /// Contains the path we failed to read.
+            path: PathBuf,
+            /// The source error describing the read failure.
+            source: io::Error
+        },
+
+        #[snafu(display("Unable to read filesystem metadata of {}: {} ", path.display(), source))]
+        /// Unable to read filesystem metadata of a given path.
+        PathMetadata {
+            /// Contains the path for which we failed to read metadata.
+            path: PathBuf,
+            /// The source error describing the read failure.
+            source: io::Error
+        },
+
+        #[snafu(display("Unable to read file {}: {} ", path.display(), source))]
+        /// Unable to read a given file.
+        FileRead {
+            /// Contains the path we failed to read.
+            path: PathBuf,
+            /// The source error describing the read failure.
+            source: io::Error
+        },
+
+        #[snafu(display("Unable to list directory {}: {} ", path.display(), source))]
+        /// Unable to list a given directory.
+        ListDirectory {
+            /// Contains the directory we failed to list.
+            path: PathBuf,
+            /// The source error describing the list failure.
+            source: io::Error
+        },
+
+        #[snafu(display("Unable to read directory entry in {}: {} ", path.display(), source))]
+        /// Unable to read a listed directory entry.
+        ReadDirectoryEntry {
+            /// Contains the directory with an entry we failed to read.
+            path: PathBuf,
+            /// The source error describing the read failure.
+            source: io::Error
+        },
+    }
+}
+pub use error::Error;
+/// Convenience alias pointing to our Error type.
+pub type Result<T> = std::result::Result<T, error::Error>;
+
 /// Get the path in `/sys/dev/block` for a major/minor number.
 fn sys_path(major: u64, minor: u64) -> PathBuf {
     PathBuf::from("/sys/dev/block").join(format!("{}:{}", major, minor))
@@ -34,10 +121,10 @@ impl BlockDevice {
     /// Creates a `BlockDevice` for a major/minor number.
     pub fn from_major_minor(major: u64, minor: u64) -> Result<Self> {
         let path = sys_path(major, minor);
-        let link_target = fs::read_link(&path)?;
+        let link_target = fs::read_link(&path).context(error::SysPathLinkRead { path })?;
         let device_name = link_target
             .file_name()
-            .ok_or_else(|| ErrorShim::LinkTargetFileName(&link_target))?
+            .context(error::LinkTargetFileName { path: &link_target })?
             .to_owned();
 
         Ok(Self {
@@ -49,32 +136,35 @@ impl BlockDevice {
 
     /// Creates a `BlockDevice` from a path residing on a block device.
     pub fn from_device_path<P: AsRef<Path>>(path: P) -> Result<Self> {
-        let metadata = fs::metadata(&path)?;
+        let path = path.as_ref();
+        let metadata = fs::metadata(path).context(error::PathMetadata { path })?;
         let major = metadata.st_dev() >> 8;
         let minor = metadata.st_dev() & 0xff;
         Ok(Self::from_major_minor(major, minor)?)
     }
 
     /// Creates a `BlockDevice` from a special block device node.
     pub fn from_device_node<P: AsRef<Path>>(path: P) -> Result<Self> {
-        let metadata = fs::metadata(&path)?;
+        let path = path.as_ref();
+        let metadata = fs::metadata(&path).context(error::PathMetadata { path })?;
         let major = metadata.st_rdev() >> 8;
         let minor = metadata.st_rdev() & 0xff;
         Ok(Self::from_major_minor(major, minor)?)
     }
 
     /// Creates a `BlockDevice` from the major:minor string from the file at `path`.
     fn from_major_minor_in_file<P: AsRef<Path>>(path: P) -> Result<Self> {
-        let s = fs::read_to_string(path.as_ref())?;
+        let path = path.as_ref();
+        let s = fs::read_to_string(path).context(error::FileRead { path })?;
         let parts = s
             .trim()
             .splitn(2, ':')
             .map(u64::from_str)
             .collect::<std::result::Result<Vec<_>, _>>()
-            .map_err(|err| ErrorShim::MajorMinorParseInt(path.as_ref(), err))?;
-        if parts.len() != 2 {
-            Err(ErrorShim::MajorMinorLen(path.as_ref()))?;
-        }
+            .context(error::MajorMinorParseInt { path })?;
+
+        ensure!(parts.len() == 2, error::MajorMinorLen { path });
+
         Self::from_major_minor(parts[0], parts[1])
     }
 
@@ -93,8 +183,10 @@ impl BlockDevice {
     //#[allow(clippy::identity_conversion)] // https://github.com/rust-lang/rust-clippy/issues/4133
     pub fn disk(&self) -> Result<Option<Self>> {
         // Globbing for /sys/block/*/{self.device_name}/dev
-        for entry in fs::read_dir("/sys/block")? {
-            let entry = entry?;
+        for entry in
+            fs::read_dir("/sys/block").context(error::ListDirectory { path: "/sys/block" })?
+        {
+            let entry = entry.context(error::ReadDirectoryEntry { path: "/sys/block" })?;
             if entry.path().join(&self.device_name).exists() {
                 return Self::from_major_minor_in_file(entry.path().join("dev")).map(Some);
             }
@@ -111,15 +203,19 @@ impl BlockDevice {
     pub fn partition(&self, part_num: u32) -> Result<Option<Self>> {
         let sys_path = self.sys_path();
         // Globbing for /sys/dev/block/{major}:{minor}/*/partition
-        for entry in fs::read_dir(&sys_path)? {
-            let entry = entry?;
+        for entry in fs::read_dir(&sys_path).context(error::ListDirectory { path: &sys_path })? {
+            let entry = entry.context(error::ReadDirectoryEntry { path: &sys_path })?;
             if entry.path().is_dir() {
                 let partition_path = entry.path().join("partition");
                 let partition_str = match fs::read_to_string(&partition_path) {
                     Ok(s) => s,
                     Err(err) => match err.kind() {
-                        ErrorKind::NotFound => continue,
-                        _ => return Err(err),
+                        io::ErrorKind::NotFound => continue,
+                        _ => {
+                            return Err(err).context(error::FileRead {
+                                path: partition_path,
+                            })
+                        }
                     },
                 };
                 if partition_str.trim() == part_num.to_string() {
@@ -135,7 +231,10 @@ impl BlockDevice {
     /// For example, given a dm-verity device, this iterator would return the data device and the
     /// hash device.
     pub fn lower_devices(&self) -> Result<LowerIter> {
-        fs::read_dir(&self.sys_path().join("slaves")).map(|iter| LowerIter { iter })
+        let path = self.sys_path().join("slaves");
+        fs::read_dir(&path)
+            .context(error::ListDirectory { path: &path })
+            .map(move |iter| LowerIter { path, iter })
     }
 }
 
@@ -155,6 +254,7 @@ impl PartialEq for BlockDevice {
 ///
 /// This struct is created by [`BlockDevice::lower_devices`].
 pub struct LowerIter {
+    path: PathBuf,
     iter: fs::ReadDir,
 }
 
@@ -164,38 +264,9 @@ impl Iterator for LowerIter {
     fn next(&mut self) -> Option<Result<BlockDevice>> {
         self.iter
             .next()
-            .map(|entry| BlockDevice::from_major_minor_in_file(entry?.path().join("dev")))
-    }
-}
-
-enum ErrorShim<'a> {
-    LinkTargetFileName(&'a Path),
-    MajorMinorParseInt(&'a Path, std::num::ParseIntError),
-    MajorMinorLen(&'a Path),
-}
-
-impl<'a> From<ErrorShim<'a>> for Error {
-    fn from(err: ErrorShim<'_>) -> Self {
-        match err {
-            ErrorShim::LinkTargetFileName(path) => Self::new(
-                ErrorKind::InvalidData,
-                format!("target of {} ends in `..`", path.display()),
-            ),
-            ErrorShim::MajorMinorParseInt(path, err) => Self::new(
-                ErrorKind::InvalidData,
-                format!(
-                    "cannot parse {} as major/minor numbers: {}",
-                    path.display(),
-                    err
-                ),
-            ),
-            ErrorShim::MajorMinorLen(path) => Self::new(
-                ErrorKind::InvalidData,
-                format!(
-                    "cannot parse {} as major/minor numbers: invalid number of colons",
-                    path.display()
-                ),
-            ),
-        }
+            .map(|entry| {
+                let entry = entry.context(error::ReadDirectoryEntry { path: &self.path })?;
+                BlockDevice::from_major_minor_in_file(entry.path().join("dev"))
+            })
     }
 }

workspaces/updater/signpost/src/error.rs

@@ -19,19 +19,19 @@ pub enum Error {
     #[snafu(display("Failed to get block device from path {}: {}", device.display(), source))]
     BlockDeviceFromPath {
         device: PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Failed to get disk from partition {}: {}", device.display(), source))]
     DiskFromPartition {
         device: PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Failed to get partition on disk {}: {}", device.display(), source))]
     PartitionFromDisk {
         device: PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Failed to find GPT on device {}: {}", device.display(), source))]
@@ -74,7 +74,7 @@ pub enum Error {
     #[snafu(display("Failed to get lower devices for {}: {}", root.display(), source))]
     RootLowerDevices {
         root: PathBuf,
-        source: std::io::Error,
+        source: block_party::Error,
     },
 
     #[snafu(display("Block device {} is not a partition", device.display()))]