feat(errors): integrate miette and generally improve error reporting

.github/workflows/ci.yml

@@ -28,7 +28,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        rust: [1.59.0, stable]
+        rust: [1.67.0, stable]
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:
@@ -52,7 +52,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        rust: [1.59.0, stable]
+        rust: [1.67.0, stable]
         os: [ubuntu-latest, macOS-latest, windows-latest]
 
     steps:

Cargo.toml

@@ -18,6 +18,7 @@ either = "1.6.1"
 futures = "0.3.17"
 hex = "0.4.3"
 memmap2 = "0.5.8"
+miette = "5.5.0"
 serde = "1.0.130"
 serde_derive = "1.0.130"
 serde_json = "1.0.68"

README.md

@@ -53,6 +53,7 @@ Minimum supported Rust version is `1.43.0`.
 - Pretty darn fast
 - Arbitrary metadata storage
 - Cross-platform: Windows and case-(in)sensitive filesystem support
+- [`miette`](https://crates.io/crates/miette) integration for detailed, helpful error reporting.
 - Punches nazis
 
 `async-std` is the default async runtime. To use `tokio` instead, turn off default features and enable the `tokio-runtime` feature, like this:

src/async_lib.rs

@@ -104,22 +104,25 @@ pub fn unwrap_joinhandle_value<T>(value: Result<T, tokio::task::JoinError>) -> T
     value.unwrap()
 }
 
-use crate::errors::{Internal, InternalResult};
 use tempfile::NamedTempFile;
 
+use crate::errors::IoErrorExt;
+
 #[cfg(feature = "async-std")]
 #[inline]
-pub async fn create_named_tempfile(tmp_path: std::path::PathBuf) -> InternalResult<NamedTempFile> {
+pub async fn create_named_tempfile(tmp_path: std::path::PathBuf) -> crate::Result<NamedTempFile> {
+    let cloned = tmp_path.clone();
     spawn_blocking(|| NamedTempFile::new_in(tmp_path))
         .await
-        .to_internal()
+        .with_context(|| format!("Failed to create a temp file at {}", cloned.display()))
 }
 
 #[cfg(feature = "tokio")]
 #[inline]
-pub async fn create_named_tempfile(tmp_path: std::path::PathBuf) -> InternalResult<NamedTempFile> {
-    let tmpfile = spawn_blocking(|| NamedTempFile::new_in(tmp_path))
+pub async fn create_named_tempfile(tmp_path: std::path::PathBuf) -> crate::Result<NamedTempFile> {
+    let cloned = tmp_path.clone();
+    Ok(spawn_blocking(|| NamedTempFile::new_in(tmp_path))
         .await
-        .to_internal()?;
-    tmpfile.to_internal()
+        .unwrap()
+        .with_context(|| format!("Failed to create a temp file at {}", cloned.display()))?)
 }

src/content/read.rs

@@ -1,13 +1,19 @@
 use std::fs::{self, File};
+use std::io::Read;
 use std::path::Path;
 use std::pin::Pin;
 use std::task::{Context, Poll};
 
+#[cfg(feature = "async-std")]
+use futures::io::AsyncReadExt;
+#[cfg(feature = "tokio")]
+use tokio::io::AsyncReadExt;
+
 use ssri::{Algorithm, Integrity, IntegrityChecker};
 
 use crate::async_lib::AsyncRead;
 use crate::content::path;
-use crate::errors::{Internal, Result};
+use crate::errors::{IoErrorExt, Result};
 
 pub struct Reader {
     fd: File,
@@ -71,46 +77,98 @@ impl AsyncReader {
 pub fn open(cache: &Path, sri: Integrity) -> Result<Reader> {
     let cpath = path::content_path(cache, &sri);
     Ok(Reader {
-        fd: File::open(cpath).to_internal()?,
+        fd: File::open(cpath).with_context(|| {
+            format!(
+                "Failed to open reader to {}",
+                path::content_path(cache, &sri).display()
+            )
+        })?,
         checker: IntegrityChecker::new(sri),
     })
 }
 
 pub async fn open_async(cache: &Path, sri: Integrity) -> Result<AsyncReader> {
     let cpath = path::content_path(cache, &sri);
     Ok(AsyncReader {
-        fd: crate::async_lib::File::open(cpath).await.to_internal()?,
+        fd: crate::async_lib::File::open(cpath).await.with_context(|| {
+            format!(
+                "Failed to open reader to {}",
+                path::content_path(cache, &sri).display()
+            )
+        })?,
         checker: IntegrityChecker::new(sri),
     })
 }
 
 pub fn read(cache: &Path, sri: &Integrity) -> Result<Vec<u8>> {
     let cpath = path::content_path(cache, sri);
-    let ret = fs::read(cpath).to_internal()?;
+    let ret = fs::read(cpath).with_context(|| {
+        format!(
+            "Failed to read contents for file at {}",
+            path::content_path(cache, sri).display()
+        )
+    })?;
     sri.check(&ret)?;
     Ok(ret)
 }
 
 pub async fn read_async<'a>(cache: &'a Path, sri: &'a Integrity) -> Result<Vec<u8>> {
     let cpath = path::content_path(cache, sri);
-    let ret = crate::async_lib::read(&cpath).await.to_internal()?;
+    let ret = crate::async_lib::read(&cpath).await.with_context(|| {
+        format!(
+            "Failed to read contents for file at {}",
+            path::content_path(cache, sri).display()
+        )
+    })?;
     sri.check(&ret)?;
     Ok(ret)
 }
 
 pub fn copy(cache: &Path, sri: &Integrity, to: &Path) -> Result<u64> {
     let cpath = path::content_path(cache, sri);
-    let ret = fs::copy(&cpath, to).to_internal()?;
-    let data = fs::read(cpath).to_internal()?;
-    sri.check(data)?;
+    let ret = fs::copy(&cpath, to).with_context(|| {
+        format!(
+            "Failed to copy cache contents from {} to {}",
+            path::content_path(cache, sri).display(),
+            to.display()
+        )
+    })?;
+    let mut reader = open(cache, sri.clone())?;
+    let mut buf: [u8; 1024] = [0; 1024];
+    while reader.read(&mut buf).with_context(|| {
+        format!(
+            "Failed to read cache contents while verifying integrity for {}",
+            path::content_path(cache, sri).display()
+        )
+    })? > 0
+    {}
+    reader.check()?;
+
     Ok(ret)
 }
 
 pub async fn copy_async<'a>(cache: &'a Path, sri: &'a Integrity, to: &'a Path) -> Result<u64> {
     let cpath = path::content_path(cache, sri);
-    let ret = crate::async_lib::copy(&cpath, to).await.to_internal()?;
-    let data = crate::async_lib::read(cpath).await.to_internal()?;
-    sri.check(data)?;
+    let ret = crate::async_lib::copy(&cpath, to).await.with_context(|| {
+        format!(
+            "Failed to copy cache contents from {} to {}",
+            path::content_path(cache, sri).display(),
+            to.display()
+        )
+    })?;
+    let mut reader = open_async(cache, sri.clone()).await?;
+    let mut buf: [u8; 1024] = [0; 1024];
+    while AsyncReadExt::read(&mut reader, &mut buf)
+        .await
+        .with_context(|| {
+            format!(
+                "Failed to read cache contents while verifying integrity for {}",
+                path::content_path(cache, sri).display()
+            )
+        })?
+        > 0
+    {}
+    reader.check()?;
     Ok(ret)
 }
 

src/content/rm.rs

@@ -4,16 +4,26 @@ use std::path::Path;
 use ssri::Integrity;
 
 use crate::content::path;
-use crate::errors::{Internal, Result};
+use crate::errors::{IoErrorExt, Result};
 
 pub fn rm(cache: &Path, sri: &Integrity) -> Result<()> {
-    fs::remove_file(path::content_path(cache, sri)).to_internal()?;
+    fs::remove_file(path::content_path(cache, sri)).with_context(|| {
+        format!(
+            "Failed to remove cache file {}",
+            path::content_path(cache, sri).display()
+        )
+    })?;
     Ok(())
 }
 
 pub async fn rm_async(cache: &Path, sri: &Integrity) -> Result<()> {
     crate::async_lib::remove_file(path::content_path(cache, sri))
         .await
-        .to_internal()?;
+        .with_context(|| {
+            format!(
+                "Failed to remove cache file {}",
+                path::content_path(cache, sri).display()
+            )
+        })?;
     Ok(())
 }

src/content/write.rs

@@ -12,7 +12,7 @@ use tempfile::NamedTempFile;
 
 use crate::async_lib::{AsyncWrite, JoinHandle};
 use crate::content::path;
-use crate::errors::{Internal, Result};
+use crate::errors::{IoErrorExt, Result};
 
 pub const MAX_MMAP_SIZE: usize = 1024 * 1024;
 
@@ -31,11 +31,30 @@ impl Writer {
         DirBuilder::new()
             .recursive(true)
             .create(&tmp_path)
-            .to_internal()?;
-        let mut tmpfile = NamedTempFile::new_in(tmp_path).to_internal()?;
+            .with_context(|| {
+                format!(
+                    "Failed to create cache directory for temporary files, at {}",
+                    tmp_path.display()
+                )
+            })?;
+        let tmp_path_clone = tmp_path.clone();
+        let mut tmpfile = NamedTempFile::new_in(tmp_path).with_context(|| {
+            format!(
+                "Failed to create temp file while initializing a writer, inside {}",
+                tmp_path_clone.display()
+            )
+        })?;
         let mmap = if let Some(size) = size {
             if size <= MAX_MMAP_SIZE {
-                tmpfile.as_file_mut().set_len(size as u64).to_internal()?;
+                tmpfile
+                    .as_file_mut()
+                    .set_len(size as u64)
+                    .with_context(|| {
+                        format!(
+                            "Failed to configure file length for temp file at {}",
+                            tmpfile.path().display()
+                        )
+                    })?;
                 unsafe { MmapMut::map_mut(tmpfile.as_file()).ok() }
             } else {
                 None
@@ -58,13 +77,31 @@ impl Writer {
             .recursive(true)
             // Safe unwrap. cpath always has multiple segments
             .create(cpath.parent().unwrap())
-            .to_internal()?;
-        let res = self.tmpfile.persist(&cpath).to_internal();
-        if res.is_err() {
-            // We might run into conflicts sometimes when persisting files.
-            // This is ok. We can deal. Let's just make sure the destination
-            // file actually exists, and we can move on.
-            std::fs::metadata(cpath).to_internal()?;
+            .with_context(|| {
+                format!(
+                    "Failed to create destination directory for cache contents, at {}",
+                    path::content_path(&self.cache, &sri)
+                        .parent()
+                        .unwrap()
+                        .display()
+                )
+            })?;
+        let res = self.tmpfile.persist(&cpath);
+        match res {
+            Ok(_) => {}
+            Err(e) => {
+                // We might run into conflicts sometimes when persisting files.
+                // This is ok. We can deal. Let's just make sure the destination
+                // file actually exists, and we can move on.
+                if !cpath.exists() {
+                    return Err(e.error).with_context(|| {
+                        format!(
+                            "Failed to persist cache contents while closing writer, at {}",
+                            path::content_path(&self.cache, &sri).display()
+                        )
+                    })?;
+                }
+            }
         }
         Ok(sri)
     }
@@ -118,11 +155,24 @@ impl AsyncWriter {
             .recursive(true)
             .create(&tmp_path)
             .await
-            .to_internal()?;
+            .with_context(|| {
+                format!(
+                    "Failed to create cache directory for temporary files, at {}",
+                    tmp_path.display()
+                )
+            })?;
         let mut tmpfile = crate::async_lib::create_named_tempfile(tmp_path).await?;
         let mmap = if let Some(size) = size {
             if size <= MAX_MMAP_SIZE {
-                tmpfile.as_file_mut().set_len(size as u64).to_internal()?;
+                tmpfile
+                    .as_file_mut()
+                    .set_len(size as u64)
+                    .with_context(|| {
+                        format!(
+                            "Failed to configure file length for temp file at {}",
+                            tmpfile.path().display()
+                        )
+                    })?;
                 unsafe { MmapMut::map_mut(tmpfile.as_file()).ok() }
             } else {
                 None
@@ -144,7 +194,7 @@ impl AsyncWriter {
         // NOTE: How do I even get access to `inner` safely???
         // let inner = ???;
         // Blocking, but should be a very fast op.
-        Ok(futures::future::poll_fn(|cx| {
+        futures::future::poll_fn(|cx| {
             let state = &mut *self.0.lock().unwrap();
 
             loop {
@@ -172,9 +222,12 @@ impl AsyncWriter {
                                 if res.is_err() {
                                     let _ = s.send(res.map(|_| sri));
                                 } else {
-                                    let res = tmpfile.persist(&cpath).with_context(|| {
-                                        String::from("persisting tempfile failed")
-                                    });
+                                    let res = tmpfile
+                                        .persist(&cpath)
+                                        .map_err(|e| e.error)
+                                        .with_context(|| {
+                                            format!("persisting file {} failed", cpath.display())
+                                        });
                                     if res.is_err() {
                                         // We might run into conflicts
                                         // sometimes when persisting files.
@@ -208,11 +261,12 @@ impl AsyncWriter {
                 }
             }
         })
-        .map(|opt| opt.ok_or_else(|| io_error("file closed")))
+        .map(|opt| opt.ok_or_else(|| crate::errors::io_error("file closed")))
         .await
-        .to_internal()?
+        .with_context(|| "Error while closing cache contents".to_string())?
         .await
-        .to_internal()??)
+        .map_err(|_| crate::errors::io_error("Operation cancelled"))
+        .with_context(|| "Error while closing cache contents".to_string())?
     }
 }
 
@@ -229,7 +283,9 @@ impl AsyncWrite for AsyncWriter {
                 State::Idle(opt) => {
                     // Grab a reference to the inner representation of the file or return an error
                     // if the file is closed.
-                    let inner = opt.as_mut().ok_or_else(|| io_error("file closed"))?;
+                    let inner = opt
+                        .as_mut()
+                        .ok_or_else(|| crate::errors::io_error("file closed"))?;
 
                     // Check if the operation has completed.
                     if let Some(Operation::Write(res)) = inner.last_op.take() {
@@ -372,10 +428,6 @@ impl AsyncWriter {
     }
 }
 
-fn io_error(err: impl Into<Box<dyn std::error::Error + Send + Sync>>) -> std::io::Error {
-    std::io::Error::new(std::io::ErrorKind::Other, err)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;