From cb01cbefc498b60a398e79ba0a7f687f30a2e383 Mon Sep 17 00:00:00 2001
From: Stu Hood <stuhood@gmail.com>
Date: Thu, 6 Apr 2023 15:52:50 -0700
Subject: [PATCH] Remove implicit conversion from `std::io::Error` to
 `StoreError` (#18696)

Remove implicit conversion from `std::io::Error` to `StoreError`, since
(as described on #18587) the former generally does not contain enough
information to debug an issue.

Fixes #18587.
---
 src/rust/engine/fs/store/src/lib.rs | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/src/rust/engine/fs/store/src/lib.rs b/src/rust/engine/fs/store/src/lib.rs
index 8873add892c..637049a7a63 100644
--- a/src/rust/engine/fs/store/src/lib.rs
+++ b/src/rust/engine/fs/store/src/lib.rs
@@ -42,7 +42,7 @@ use std::fmt::{self, Debug, Display};
 use std::fs::OpenOptions;
 use std::fs::Permissions as FSPermissions;
 use std::future::Future;
-use std::io::{self, Write};
+use std::io::Write;
 use std::os::unix::fs::{MetadataExt, OpenOptionsExt, PermissionsExt};
 use std::path::{Path, PathBuf};
 use std::sync::{Arc, Weak};
@@ -143,12 +143,6 @@ impl From<String> for StoreError {
   }
 }
 
-impl From<io::Error> for StoreError {
-  fn from(err: io::Error) -> Self {
-    Self::Unclassified(err.to_string())
-  }
-}
-
 // Summary of the files and directories uploaded with an operation
 // ingested_file_{count, bytes}: Number and combined size of processed files
 // uploaded_file_{count, bytes}: Number and combined size of files uploaded to the remote
@@ -1448,7 +1442,12 @@ impl Store {
     destination: PathBuf,
     target: String,
   ) -> Result<(), StoreError> {
-    symlink(target, destination).await?;
+    symlink(&target, &destination).await.map_err(|e| {
+      format!(
+        "Failed to create symlink to {target} at {}: {e}",
+        destination.display()
+      )
+    })?;
     Ok(())
   }
 
@@ -1464,9 +1463,19 @@ impl Store {
     // It also has the benefit of playing nicely with Docker for macOS file virtualization: see
     // #18162.
     #[cfg(target_os = "macos")]
-    copy(target, destination).await?;
+    copy(&target, &destination).await.map_err(|e| {
+      format!(
+        "Failed to copy from {target} to {}: {e}",
+        destination.display()
+      )
+    })?;
     #[cfg(not(target_os = "macos"))]
-    hard_link(target, destination).await?;
+    hard_link(&target, &destination).await.map_err(|e| {
+      format!(
+        "Failed to create hardlink to {target} at {}: {e}",
+        destination.display()
+      )
+    })?;
     Ok(())
   }