Fixed upload transfer error not being logged

CHANGELOG.md

@@ -25,6 +25,8 @@ Released on FIXME: ??
   - Added the possibility to set different formatters for local and remote hosts
 - Bugfix:
   - Fixed wrong text wrap in log box
+  - Fixed error message not being shown after an upload failure
+  - [Issue 23](https://github.com/veeso/termscp/issues/23): Remove created file if transfer failed or was abrupted
 - Dependencies:
   - Added `tui-realm 0.1.0`
   - Removed `tui`

README.md

@@ -82,7 +82,7 @@ If you want to contribute to this project, don't forget to check out our contrib
 ### Cargo 🦀
 
 ```sh
-# Install termscp through cargo
+# Install termscp via cargo
 cargo install termscp
 ```
 
@@ -98,7 +98,7 @@ Requirements:
 Get `deb` package from [HERE](https://github.com/veeso/termscp/releases/latest/download/termscp_0.5.0_amd64.deb)
 or run `wget https://github.com/veeso/termscp/releases/latest/download/termscp_0.5.0_amd64.deb`
 
-then install through dpkg:
+then install via dpkg:
 
 ```sh
 dpkg -i termscp_*.deb
@@ -111,7 +111,7 @@ gdebi termscp_*.deb
 Get `rpm` package from [HERE](https://github.com/veeso/termscp/releases/latest/download/termscp-0.5.0-1.x86_64.rpm)
 or run `wget https://github.com/veeso/termscp/releases/latest/download/termscp-0.5.0-1.x86_64.rpm`
 
-then install through rpm:
+then install via rpm:
 
 ```sh
 rpm -U termscp_*.rpm
@@ -184,12 +184,10 @@ The developer documentation can be found on Rust Docs at <https://docs.rs/termsc
 
 - **Themes provider 🎨**: I'm still thinking about how I will implement this, but basically the idea is to have a configuration file where it will be possible
     to define the color schema for the entire application. I haven't planned this release yet
-- **Local and remote file explorer format 🃏**: From 0.5.0 you will be able to customize the file format for both local and remote hosts.
 - **Synchronized browsing of local and remote directories ⌚**: See [Issue 8](https://github.com/veeso/termscp/issues/8)
 - **Group file select 🤩**: Possibility to select a group of files in explorers to operate on
 
-No other new feature is planned at the moment. I actually think that termscp is getting mature and now I should focus upcoming updates more on bug fixing and
+No other new feature is planned at the moment. I actually think that termscp is getting mature and now I should focus upcoming updates more on bug fixing and code/performance improvements than on new features.
-code/performance improvements than on new features.
 Anyway there are some ideas which I'd like to implement. If you want to start working on them, feel free to open a PR:
 
 - Amazon S3 support

src/host/mod.rs

@@ -68,7 +68,7 @@ pub enum HostErrorType {
 /// ### HostError
 ///
 /// HostError is a wrapper for the error type and the exact io error
-
+#[derive(Debug)]
 pub struct HostError {
     pub error: HostErrorType,
     ioerr: Option<std::io::Error>,

src/ui/activities/filetransfer_activity/session.rs

@@ -33,7 +33,9 @@ extern crate tempfile;
 
 // Locals
 use super::{FileTransferActivity, LogLevel};
+use crate::filetransfer::FileTransferError;
 use crate::fs::{FsEntry, FsFile};
+use crate::host::HostError;
 use crate::utils::fmt::fmt_millis;
 
 // Ext
@@ -43,6 +45,26 @@ use std::fs::OpenOptions;
 use std::io::{Read, Seek, Write};
 use std::path::{Path, PathBuf};
 use std::time::{Instant, SystemTime};
+use thiserror::Error;
+
+/// ## TransferErrorReason
+///
+/// Describes the reason that caused an error during a file transfer
+#[derive(Error, Debug)]
+enum TransferErrorReason {
+    #[error("Abrupted")]
+    Abrupted,
+    #[error("Failed to seek file: {0}")]
+    CouldNotRewind(std::io::Error),
+    #[error("I/O error on localhost: {0}")]
+    LocalIOError(std::io::Error),
+    #[error("Host error: {0}")]
+    HostError(HostError),
+    #[error("I/O error on remote: {0}")]
+    RemoteIOError(std::io::Error),
+    #[error("File transfer error: {0}")]
+    FileTransferError(FileTransferError),
+}
 
 impl FileTransferActivity {
     /// ### connect
@@ -149,7 +171,22 @@ impl FileTransferActivity {
         // Match entry
         match entry {
             FsEntry::File(file) => {
-                let _ = self.filetransfer_send_file(file, remote_path.as_path(), file_name);
+                if let Err(err) =
+                    self.filetransfer_send_file(file, remote_path.as_path(), file_name)
+                {
+                    // Log error
+                    self.log_and_alert(
+                        LogLevel::Error,
+                        format!("Failed to upload file {}: {}", file.name, err),
+                    );
+                    // If transfer was abrupted or there was an IO error on remote, remove file
+                    if matches!(
+                        err,
+                        TransferErrorReason::Abrupted | TransferErrorReason::RemoteIOError(_)
+                    ) {
+                        // TODO: make dummy fs entry
+                    }
+                }
             }
             FsEntry::Directory(dir) => {
                 // Create directory on remote
@@ -252,7 +289,11 @@ impl FileTransferActivity {
                 if let Err(err) =
                     self.filetransfer_recv_file(local_file_path.as_path(), file, file_name)
                 {
-                    self.log_and_alert(LogLevel::Error, err);
+                    self.log_and_alert(
+                        LogLevel::Error,
+                        format!("Could not download file {}: {}", file.name, err),
+                    );
+                    // TODO: delete file
                 }
             }
             FsEntry::Directory(dir) => {
@@ -365,7 +406,7 @@ impl FileTransferActivity {
         local: &FsFile,
         remote: &Path,
         file_name: String,
-    ) -> Result<(), String> {
+    ) -> Result<(), TransferErrorReason> {
         // Upload file
         // Try to open local file
         match self
@@ -382,7 +423,7 @@ impl FileTransferActivity {
                         fhnd.seek(std::io::SeekFrom::End(0)).unwrap_or(0) as usize;
                     // rewind
                     if let Err(err) = fhnd.seek(std::io::SeekFrom::Start(0)) {
-                        return Err(format!("Could not rewind local file: {}", err));
+                        return Err(TransferErrorReason::CouldNotRewind(err));
                     }
                     // Write remote file
                     let mut total_bytes_written: usize = 0;
@@ -419,9 +460,8 @@ impl FileTransferActivity {
                                             }
                                             Err(err) => {
                                                 self.umount_progress_bar();
-                                                return Err(format!(
+                                                return Err(TransferErrorReason::RemoteIOError(
-                                                    "Could not write remote file: {}",
+                                                    err,
-                                                    err
                                                 ));
                                             }
                                         }
@@ -430,7 +470,7 @@ impl FileTransferActivity {
                             }
                             Err(err) => {
                                 self.umount_progress_bar();
-                                return Err(format!("Could not read local file: {}", err));
+                                return Err(TransferErrorReason::LocalIOError(err));
                             }
                         }
                         // Increase progress
@@ -452,6 +492,10 @@ impl FileTransferActivity {
                             format!("Could not finalize remote stream: \"{}\"", err).as_str(),
                         );
                     }
+                    // if upload was abrupted, return error
+                    if self.transfer.aborted {
+                        return Err(TransferErrorReason::Abrupted);
+                    }
                     self.log(
                         LogLevel::Info,
                         format!(
@@ -464,21 +508,9 @@ impl FileTransferActivity {
                         .as_ref(),
                     );
                 }
-                Err(err) => {
+                Err(err) => return Err(TransferErrorReason::FileTransferError(err)),
-                    return Err(format!(
-                        "Failed to upload file \"{}\": {}",
-                        local.abs_path.display(),
-                        err
-                    ))
-                }
             },
-            Err(err) => {
+            Err(err) => return Err(TransferErrorReason::HostError(err)),
-                return Err(format!(
-                    "Failed to open file \"{}\": {}",
-                    local.abs_path.display(),
-                    err
-                ))
-            }
         }
         Ok(())
     }
@@ -491,7 +523,7 @@ impl FileTransferActivity {
         local: &Path,
         remote: &FsFile,
         file_name: String,
-    ) -> Result<(), String> {
+    ) -> Result<(), TransferErrorReason> {
         // Try to open local file
         match self.context.as_ref().unwrap().local.open_file_write(local) {
             Ok(mut local_file) => {
@@ -531,9 +563,8 @@ impl FileTransferActivity {
                                                 Ok(bytes) => buf_start += bytes,
                                                 Err(err) => {
                                                     self.umount_progress_bar();
-                                                    return Err(format!(
+                                                    return Err(TransferErrorReason::LocalIOError(
-                                                        "Could not write local file: {}",
+                                                        err,
-                                                        err
                                                     ));
                                                 }
                                             }
@@ -542,7 +573,7 @@ impl FileTransferActivity {
                                 }
                                 Err(err) => {
                                     self.umount_progress_bar();
-                                    return Err(format!("Could not read remote file: {}", err));
+                                    return Err(TransferErrorReason::RemoteIOError(err));
                                 }
                             }
                             // Set progress
@@ -564,6 +595,10 @@ impl FileTransferActivity {
                                 format!("Could not finalize remote stream: \"{}\"", err).as_str(),
                             );
                         }
+                        // If download was abrupted, return Error
+                        if self.transfer.aborted {
+                            return Err(TransferErrorReason::Abrupted);
+                        }
                         // Apply file mode to file
                         #[cfg(any(target_os = "unix", target_os = "macos", target_os = "linux"))]
                         if let Some(pex) = remote.unix_pex {
@@ -594,22 +629,10 @@ impl FileTransferActivity {
                             .as_ref(),
                         );
                     }
-                    Err(err) => {
+                    Err(err) => return Err(TransferErrorReason::FileTransferError(err)),
-                        return Err(format!(
-                            "Failed to download file \"{}\": {}",
-                            remote.abs_path.display(),
-                            err
-                        ))
-                    }
                 }
             }
-            Err(err) => {
+            Err(err) => return Err(TransferErrorReason::HostError(err)),
-                return Err(format!(
-                    "Failed to open local file for write \"{}\": {}",
-                    local.display(),
-                    err
-                ))
-            }
         }
         Ok(())
     }
@@ -777,7 +800,7 @@ impl FileTransferActivity {
         };
         // Download file
         if let Err(err) = self.filetransfer_recv_file(tmpfile.path(), file, file.name.clone()) {
-            return Err(err);
+            return Err(format!("Could not open file {}: {}", file.name, err));
         }
         // Get current file modification time
         let prev_mtime: SystemTime = match self.context.as_ref().unwrap().local.stat(tmpfile.path())
@@ -841,7 +864,11 @@ impl FileTransferActivity {
                     file.abs_path.as_path(),
                     file.name.clone(),
                 ) {
-                    return Err(err);
+                    return Err(format!(
+                        "Could not write file {}: {}",
+                        file.abs_path.display(),
+                        err
+                    ));
                 }
             }
             false => {