From a35395bd5132e086f0a165aad1d6904185f314ee Mon Sep 17 00:00:00 2001 From: veeso Date: Sat, 1 May 2021 21:59:52 +0200 Subject: [PATCH] Fixed upload transfer error not being logged --- CHANGELOG.md | 2 + README.md | 10 +- src/host/mod.rs | 2 +- .../filetransfer_activity/session.rs | 113 +++++++++++------- 4 files changed, 77 insertions(+), 50 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e4e7fce..9969a60 100644 --- a/CHANGELOG.md +++ b/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` diff --git a/README.md b/README.md index 8552176..edc031a 100644 --- a/README.md +++ b/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 , diff --git a/src/ui/activities/filetransfer_activity/session.rs b/src/ui/activities/filetransfer_activity/session.rs index a2f137f..42e8190 100644 --- a/src/ui/activities/filetransfer_activity/session.rs +++ b/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!( - "Could not write remote file: {}", - err + return Err(TransferErrorReason::RemoteIOError( + 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) => { - return Err(format!( - "Failed to upload file \"{}\": {}", - local.abs_path.display(), - err - )) - } + Err(err) => return Err(TransferErrorReason::FileTransferError(err)), }, - Err(err) => { - return Err(format!( - "Failed to open file \"{}\": {}", - local.abs_path.display(), - err - )) - } + Err(err) => return Err(TransferErrorReason::HostError(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!( - "Could not write local file: {}", - err + return Err(TransferErrorReason::LocalIOError( + 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) => { - return Err(format!( - "Failed to download file \"{}\": {}", - remote.abs_path.display(), - err - )) - } + Err(err) => return Err(TransferErrorReason::FileTransferError(err)), } } - Err(err) => { - return Err(format!( - "Failed to open local file for write \"{}\": {}", - local.display(), - err - )) - } + Err(err) => return Err(TransferErrorReason::HostError(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 => {