Add From trait impl for LayerError (#175)

Add From trait impl for LayerError.

CHANGELOG.md

@@ -13,16 +13,14 @@ It is now possible to use the remote's `addrinfo` by setting the `MIRRORD_REMOTE
 `true`, or using the `-d` option in mirrord-cli.
 - New feature, [Ephemeral Containers](https://github.com/metalbear-co/mirrord/issues/172).
 Use Kubernetes beta feature `Ephemeral Containers` to mirror traffic with the `--ephemeral-container` flag.
-- E2E tests for Golang using the Gin framework.
+- E2E tests on macos for Golang using the Gin framework.
 
 ### Changed
 - Refactored `mirrord-layer/socket` into a module structure similar to `mirrord-layer/file`.
-<<<<<<< HEAD
 - Refactored the error part of the many `Result<Response, ResponseError>`.
-=======
 - Refactored `file` related functions, created `FileHandler` and improved structure.
+- Refactored error handling in mirrord-layer.
 - E2E: Collect minikube logs and fix collecting container logs
->>>>>>> 0d7f1b10697426981e0908a915667944314c0dbe
 - E2E: macOS use colima instead of minikube.
 
 ### Fixed

mirrord-layer/src/error.rs

@@ -1,9 +1,12 @@
-use std::{env::VarError, os::unix::io::RawFd, str::ParseBoolError};
+use std::{env::VarError, os::unix::io::RawFd, path::PathBuf, ptr, str::ParseBoolError};
 
+use errno::set_errno;
 use kube::config::InferConfigError;
+use libc::FILE;
 use mirrord_protocol::{tcp::LayerTcp, ResponseError};
 use thiserror::Error;
 use tokio::sync::{mpsc::error::SendError, oneshot::error::RecvError};
+use tracing::error;
 
 use super::HookMessage;
 
@@ -40,7 +43,7 @@ pub enum LayerError {
     TryFromInt(#[from] std::num::TryFromIntError),
 
     #[error("mirrord-layer: Failed to find local fd `{0}`!")]
-    LocalFDNotFound(RawFd),
+    LocalFDNotFound(RawFd, PathBuf),
 
     #[error("mirrord-layer: HOOK_SENDER is `None`!")]
     EmptyHookSender,
@@ -86,6 +89,9 @@ pub enum LayerError {
 
     #[error("mirrord-layer: DNS does not resolve!")]
     DNSNoName,
+
+    #[error("mirrord-layer: Failed converting `to_str` with `{0}`!")]
+    Utf8(#[from] std::str::Utf8Error),
 }
 
 // Cannot have a generic From<T> implementation for this error, so explicitly implemented here.
@@ -94,3 +100,79 @@ impl<'a, T> From<std::sync::PoisonError<std::sync::MutexGuard<'a, T>>> for Layer
         LayerError::LockError
     }
 }
+
+// mapping based on - https://man7.org/linux/man-pages/man3/errno.3.html
+
+impl From<LayerError> for i64 {
+    fn from(fail: LayerError) -> Self {
+        error!("Error occured in Layer >> {:?}", fail);
+
+        let libc_error = match fail {
+            LayerError::VarError(_) => libc::EINVAL,
+            LayerError::ParseBoolError(_) => libc::EINVAL,
+            LayerError::SendErrorHookMessage(_) => libc::EBADMSG,
+            LayerError::SendErrorConnection(_) => libc::EBADMSG,
+            LayerError::SendErrorLayerTcp(_) => libc::EBADMSG,
+            LayerError::RecvError(_) => libc::EBADMSG,
+            LayerError::Null(_) => libc::EINVAL,
+            LayerError::TryFromInt(_) => libc::EINVAL,
+            LayerError::LocalFDNotFound(..) => libc::EBADF,
+            LayerError::EmptyHookSender => libc::EINVAL,
+            LayerError::NoConnectionId(_) => libc::ECONNREFUSED,
+            LayerError::IO(io_fail) => io_fail.raw_os_error().unwrap_or(libc::EIO),
+            LayerError::PortNotFound(_) => libc::EADDRNOTAVAIL,
+            LayerError::ConnectionIdNotFound(_) => libc::EADDRNOTAVAIL,
+            LayerError::ListenAlreadyExists => libc::EEXIST,
+            LayerError::SendErrorFileResponse => libc::EINVAL,
+            LayerError::SendErrorGetAddrInfoResponse => libc::EINVAL,
+            LayerError::LockError => libc::EINVAL,
+            LayerError::ResponseError(response_fail) => match response_fail {
+                ResponseError::AllocationFailure(_) => libc::ENOMEM,
+                ResponseError::NotFound(_) => libc::ENOENT,
+                ResponseError::NotDirectory(_) => libc::ENOTDIR,
+                ResponseError::NotFile(_) => libc::EISDIR,
+                ResponseError::RemoteIO(io_fail) => io_fail.raw_os_error.unwrap_or(libc::EIO),
+            },
+            LayerError::UnmatchedPong => libc::ETIMEDOUT,
+            LayerError::KubeConfigError(_) => libc::EINVAL,
+            LayerError::PodSpecNotFound(_) => libc::EINVAL,
+            LayerError::KubeError(_) => libc::EINVAL,
+            LayerError::JSONConvertError(_) => libc::EINVAL,
+            LayerError::TimeOutError => libc::ETIMEDOUT,
+            LayerError::DNSNoName => libc::EFAULT,
+            LayerError::Utf8(_) => libc::EINVAL,
+        };
+
+        set_errno(errno::Errno(libc_error));
+
+        -1
+    }
+}
+
+impl From<LayerError> for isize {
+    fn from(fail: LayerError) -> Self {
+        i64::from(fail) as _
+    }
+}
+
+impl From<LayerError> for usize {
+    fn from(fail: LayerError) -> Self {
+        let _ = i64::from(fail);
+
+        0
+    }
+}
+
+impl From<LayerError> for i32 {
+    fn from(fail: LayerError) -> Self {
+        i64::from(fail) as _
+    }
+}
+
+impl From<LayerError> for *mut FILE {
+    fn from(fail: LayerError) -> Self {
+        let _ = i64::from(fail);
+
+        ptr::null_mut()
+    }
+}

mirrord-layer/src/file/hooks.rs

@@ -10,6 +10,7 @@ use super::{
     OpenOptionsInternalExt, IGNORE_FILES, OPEN_FILES,
 };
 use crate::{
+    error::LayerError,
     file::ops::{lseek, open, read, write},
     macros::hook,
 };
@@ -18,15 +19,13 @@ use crate::{
 ///
 /// **Bypassed** by `raw_path`s that match `IGNORE_FILES` regex.
 pub(super) unsafe extern "C" fn open_detour(raw_path: *const c_char, open_flags: c_int) -> RawFd {
-    let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| {
-        error!(
-            "Failed converting raw_path {:#?} from `c_char` with {:#?}",
-            raw_path, fail
-        );
-        -1
-    }) {
-        Ok(path_str) => path_str.into(),
-        Err(fail) => return fail,
+    let path = match CStr::from_ptr(raw_path)
+        .to_str()
+        .map_err(LayerError::from)
+        .map(PathBuf::from)
+    {
+        Ok(path) => path,
+        Err(fail) => return fail.into(),
     };
 
     // Calls with non absolute paths are sent to libc::open.
@@ -36,12 +35,8 @@ pub(super) unsafe extern "C" fn open_detour(raw_path: *const c_char, open_flags:
         let open_options = OpenOptionsInternalExt::from_flags(open_flags);
         let open_result = open(path, open_options);
 
-        open_result
-            .map_err(|fail| {
-                error!("Failed opening file with {:#?}", fail);
-                -1
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = open_result.map_err(From::from);
+        result
     }
 }
 
@@ -52,26 +47,22 @@ pub(super) unsafe extern "C" fn fopen_detour(
     raw_path: *const c_char,
     raw_mode: *const c_char,
 ) -> *mut FILE {
-    let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| {
-        error!(
-            "Failed converting raw_path {:#?} from `c_char` with {:#?}",
-            raw_path, fail
-        );
-        std::ptr::null_mut()
-    }) {
-        Ok(path_str) => path_str.into(),
-        Err(fail) => return fail,
+    let path = match CStr::from_ptr(raw_path)
+        .to_str()
+        .map_err(LayerError::from)
+        .map(PathBuf::from)
+    {
+        Ok(path) => path,
+        Err(fail) => return fail.into(),
     };
 
-    let mode: String = match CStr::from_ptr(raw_mode).to_str().map_err(|fail| {
-        error!(
-            "Failed converting raw_mode {:#?} from `c_char` with {:#?}",
-            raw_mode, fail
-        );
-        std::ptr::null_mut()
-    }) {
-        Ok(mode_str) => mode_str.into(),
-        Err(fail) => return fail,
+    let mode = match CStr::from_ptr(raw_mode)
+        .to_str()
+        .map(String::from)
+        .map_err(LayerError::from)
+    {
+        Ok(mode) => mode,
+        Err(fail) => return fail.into(),
     };
 
     if IGNORE_FILES.is_match(path.to_str().unwrap()) || !path.is_absolute() {
@@ -80,12 +71,8 @@ pub(super) unsafe extern "C" fn fopen_detour(
         let open_options = OpenOptionsInternalExt::from_mode(mode);
         let fopen_result = fopen(path, open_options);
 
-        fopen_result
-            .map_err(|fail| {
-                error!("Failed opening file with {fail:#?}");
-                ptr::null_mut()
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = fopen_result.map_err(From::from);
+        result
     }
 }
 
@@ -94,15 +81,13 @@ pub(super) unsafe extern "C" fn fopen_detour(
 /// Converts a `RawFd` into `*mut FILE` only for files that are already being managed by
 /// mirrord-layer.
 pub(super) unsafe extern "C" fn fdopen_detour(fd: RawFd, raw_mode: *const c_char) -> *mut FILE {
-    let mode: String = match CStr::from_ptr(raw_mode).to_str().map_err(|fail| {
-        error!(
-            "Failed converting raw_mode {:#?} from `c_char` with {:#?}",
-            raw_mode, fail
-        );
-        std::ptr::null_mut()
-    }) {
-        Ok(mode_str) => mode_str.into(),
-        Err(fail) => return fail,
+    let mode = match CStr::from_ptr(raw_mode)
+        .to_str()
+        .map(String::from)
+        .map_err(LayerError::from)
+    {
+        Ok(mode) => mode,
+        Err(fail) => return fail.into(),
     };
 
     let open_files = OPEN_FILES.lock().unwrap();
@@ -112,12 +97,8 @@ pub(super) unsafe extern "C" fn fdopen_detour(fd: RawFd, raw_mode: *const c_char
         let open_options = OpenOptionsInternalExt::from_mode(mode);
         let fdopen_result = fdopen(local_fd, *remote_fd, open_options);
 
-        fdopen_result
-            .map_err(|fail| {
-                error!("Failed fdopen file with {fail:#?}");
-                ptr::null_mut()
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = fdopen_result.map_err(From::from);
+        result
     } else {
         libc::fdopen(fd, raw_mode)
     }
@@ -133,15 +114,13 @@ pub(super) unsafe extern "C" fn openat_detour(
     raw_path: *const c_char,
     open_flags: c_int,
 ) -> RawFd {
-    let path: PathBuf = match CStr::from_ptr(raw_path).to_str().map_err(|fail| {
-        error!(
-            "Failed converting raw_path {:#?} from `c_char` with {:#?}",
-            raw_path, fail
-        );
-        -1
-    }) {
-        Ok(path_str) => path_str.into(),
-        Err(fail) => return fail,
+    let path = match CStr::from_ptr(raw_path)
+        .to_str()
+        .map_err(LayerError::from)
+        .map(PathBuf::from)
+    {
+        Ok(path) => path,
+        Err(fail) => return fail.into(),
     };
 
     // `openat` behaves the same as `open` when the path is absolute.
@@ -158,12 +137,8 @@ pub(super) unsafe extern "C" fn openat_detour(
         if let Some(remote_fd) = remote_fd {
             let openat_result = openat(path, open_flags, remote_fd);
 
-            openat_result
-                .map_err(|fail| {
-                    error!("Failed opening file with {fail:#?}");
-                    -1
-                })
-                .unwrap_or_else(|fail| fail)
+            let (Ok(result) | Err(result)) = openat_result.map_err(From::from);
+            result
         } else {
             // Nope, it's relative outside of our hands.
 
@@ -200,12 +175,8 @@ pub(crate) unsafe extern "C" fn read_detour(
             read_amount.try_into().unwrap()
         });
 
-        read_result
-            .map_err(|fail| {
-                error!("Failed reading file with {fail:#?}");
-                -1
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = read_result.map_err(From::from);
+        result
     } else {
         libc::read(fd, out_buffer, count)
     }
@@ -243,12 +214,8 @@ pub(crate) unsafe extern "C" fn fread_detour(
             read_amount
         });
 
-        read_result
-            .map_err(|fail| {
-                error!("Failed reading file with {fail:#?}");
-                0
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = read_result.map_err(From::from);
+        result
     } else {
         libc::fread(out_buffer, element_size, number_of_elements, file_stream)
     }
@@ -289,12 +256,8 @@ pub(crate) unsafe extern "C" fn lseek_detour(fd: RawFd, offset: off_t, whence: c
 
         let lseek_result = lseek(remote_fd, seek_from).map(|offset| offset.try_into().unwrap());
 
-        lseek_result
-            .map_err(|fail| {
-                error!("Failed lseek operation with {fail:#?}");
-                -1
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = lseek_result.map_err(From::from);
+        result
     } else {
         libc::lseek(fd, offset, whence)
     }
@@ -322,12 +285,8 @@ pub(crate) unsafe extern "C" fn write_detour(
 
         let write_result = write(remote_fd, write_bytes);
 
-        write_result
-            .map_err(|fail| {
-                error!("Failed writing file with {fail:#?}");
-                -1
-            })
-            .unwrap_or_else(|fail| fail)
+        let (Ok(result) | Err(result)) = write_result.map_err(From::from);
+        result
     } else {
         libc::write(fd, buffer, count)
     }

mirrord-layer/src/file/ops.rs

@@ -157,12 +157,12 @@ pub(crate) fn fopen(
         path, open_options
     );
 
-    let local_file_fd = open(path, open_options)?;
+    let local_file_fd = open(path.clone(), open_options)?;
     let open_files = OPEN_FILES.lock().unwrap();
 
     open_files
         .get_key_value(&local_file_fd)
-        .ok_or(LayerError::LocalFDNotFound(local_file_fd))
+        .ok_or(LayerError::LocalFDNotFound(local_file_fd, path))
         // Convert the fd into a `*FILE`, this is be ok as long as `OPEN_FILES` holds the fd.
         .map(|(local_fd, _)| local_fd as *const _ as *mut _)
 }