From d8acbd2ce0f09ed14bce383e59bbbcb40a01904e Mon Sep 17 00:00:00 2001 From: Josh Megnauth Date: Mon, 1 Sep 2025 01:28:26 -0400 Subject: [PATCH] Fix extracting password protected archives Closes: #1157 The fix splits the "canceled" and "failed" states for OperationError. It also preserves that state because some functions overwrote the state by rewrapping the error. --- i18n/en/cosmic_files.ftl | 1 + src/archive.rs | 75 ++++---- src/operation/controller.rs | 20 +- src/operation/mod.rs | 360 +++++++++++++++++++++++------------- src/operation/reader.rs | 4 +- src/operation/recursive.rs | 59 ++++-- src/tab.rs | 11 +- 7 files changed, 341 insertions(+), 189 deletions(-) diff --git a/i18n/en/cosmic_files.ftl b/i18n/en/cosmic_files.ftl index c1941b8..55558c5 100644 --- a/i18n/en/cosmic_files.ftl +++ b/i18n/en/cosmic_files.ftl @@ -185,6 +185,7 @@ no-history = No items in history. pending = Pending progress = {$percent}% progress-cancelled = {$percent}%, cancelled +progress-failed = {$percent}%, failed progress-paused = {$percent}%, paused failed = Failed complete = Complete diff --git a/src/archive.rs b/src/archive.rs index 6d55fea..e35ee46 100644 --- a/src/archive.rs +++ b/src/archive.rs @@ -47,53 +47,58 @@ pub fn extract( controller: &Controller, ) -> Result<(), OperationError> { let mime = mime_for_path(path, None, false); - let controller = controller.clone(); let password = password.clone(); match mime.essence_str() { - "application/gzip" | "application/x-compressed-tar" => OpReader::new(path, controller) - .map(io::BufReader::new) - .map(flate2::read::GzDecoder::new) - .map(tar::Archive::new) - .and_then(|mut archive| archive.unpack(&new_dir)) - .map_err(OperationError::from_str)?, - "application/x-tar" => OpReader::new(path, controller) + "application/gzip" | "application/x-compressed-tar" => { + OpReader::new(path, controller.clone()) + .map(io::BufReader::new) + .map(flate2::read::GzDecoder::new) + .map(tar::Archive::new) + .and_then(|mut archive| archive.unpack(new_dir)) + .map_err(|e| OperationError::from_err(e, controller))? + } + "application/x-tar" => OpReader::new(path, controller.clone()) .map(io::BufReader::new) .map(tar::Archive::new) - .and_then(|mut archive| archive.unpack(&new_dir)) - .map_err(OperationError::from_str)?, + .and_then(|mut archive| archive.unpack(new_dir)) + .map_err(|e| OperationError::from_err(e, controller))?, "application/zip" => fs::File::open(path) .map(io::BufReader::new) .map(zip::ZipArchive::new) - .map_err(OperationError::from_str)? - .and_then(move |mut archive| zip_extract(&mut archive, &new_dir, password, controller)) + .map_err(|e| OperationError::from_err(e, controller))? + .and_then(move |mut archive| { + zip_extract(&mut archive, new_dir, password, controller.clone()) + }) .map_err(|e| match e { ZipError::UnsupportedArchive(ZipError::PASSWORD_REQUIRED) - | ZipError::InvalidPassword => OperationError { - kind: OperationErrorType::PasswordRequired, - }, - _ => OperationError::from_str(e), + | ZipError::InvalidPassword => { + OperationError::from_kind(OperationErrorType::PasswordRequired, controller) + } + _ => OperationError::from_err(e, controller), })?, #[cfg(feature = "bzip2")] "application/x-bzip" | "application/x-bzip-compressed-tar" | "application/x-bzip2" - | "application/x-bzip2-compressed-tar" => OpReader::new(path, controller) + | "application/x-bzip2-compressed-tar" => OpReader::new(path, controller.clone()) .map(io::BufReader::new) .map(bzip2::read::BzDecoder::new) .map(tar::Archive::new) - .and_then(|mut archive| archive.unpack(&new_dir)) - .map_err(OperationError::from_str)?, + .and_then(|mut archive| archive.unpack(new_dir)) + .map_err(|e| OperationError::from_err(e, controller))?, #[cfg(feature = "xz2")] - "application/x-xz" | "application/x-xz-compressed-tar" => OpReader::new(path, controller) - .map(io::BufReader::new) - .map(xz2::read::XzDecoder::new) - .map(tar::Archive::new) - .and_then(|mut archive| archive.unpack(&new_dir)) - .map_err(OperationError::from_str)?, - _ => Err(OperationError::from_str(format!( - "unsupported mime type {:?}", - mime - )))?, + "application/x-xz" | "application/x-xz-compressed-tar" => { + OpReader::new(path, controller.clone()) + .map(io::BufReader::new) + .map(xz2::read::XzDecoder::new) + .map(tar::Archive::new) + .and_then(|mut archive| archive.unpack(new_dir)) + .map_err(|e| OperationError::from_err(e, controller))? + } + _ => Err(OperationError::from_err( + format!("unsupported mime type {:?}", mime), + controller, + ))?, } Ok(()) } @@ -135,7 +140,7 @@ fn zip_extract>( controller .check() .await - .map_err(|err| io::Error::new(io::ErrorKind::Other, err)) + .map_err(|s| io::Error::other(OperationError::from_state(s, &controller))) })?; controller.set_progress((i as f32) / total_files as f32); @@ -143,11 +148,10 @@ fn zip_extract>( let mut file = match &password { None => archive.by_index(i), Some(pwd) => archive.by_index_decrypt(i, pwd.as_bytes()), - } - .map_err(|e| e)?; + }?; let filepath = file .enclosed_name() - .ok_or(ZipError::InvalidArchive("Invalid file path".into()))?; + .ok_or(ZipError::InvalidArchive("Invalid file path"))?; let outpath = directory.as_ref().join(filepath); @@ -206,8 +210,7 @@ fn zip_extract>( let mut file = match &password { None => archive.by_index(i), Some(pwd) => archive.by_index_decrypt(i, pwd.as_bytes()), - } - .map_err(|e| e)?; + }?; // create all pending dirs while let Some(pending_dir) = pending_directory_creates.pop_front() { @@ -226,7 +229,7 @@ fn zip_extract>( controller .check() .await - .map_err(|err| io::Error::new(io::ErrorKind::Other, err)) + .map_err(|s| io::Error::other(OperationError::from_state(s, &controller))) })?; let count = file.read(&mut buffer)?; diff --git a/src/operation/controller.rs b/src/operation/controller.rs index e02524e..01eb9f3 100644 --- a/src/operation/controller.rs +++ b/src/operation/controller.rs @@ -1,11 +1,10 @@ -use crate::fl; - use std::sync::{Arc, Mutex}; use tokio::sync::Notify; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum ControllerState { Cancelled, + Failed, Paused, Running, } @@ -37,10 +36,11 @@ impl Default for Controller { } impl Controller { - pub async fn check(&self) -> Result<(), String> { + pub async fn check(&self) -> Result<(), ControllerState> { loop { match self.state() { - ControllerState::Cancelled => return Err(fl!("cancelled")), + ControllerState::Cancelled => return Err(ControllerState::Cancelled), + ControllerState::Failed => return Err(ControllerState::Failed), ControllerState::Paused => (), ControllerState::Running => return Ok(()), } @@ -74,6 +74,10 @@ impl Controller { self.set_state(ControllerState::Cancelled); } + pub fn is_failed(&self) -> bool { + matches!(self.state(), ControllerState::Failed) + } + pub fn is_paused(&self) -> bool { matches!(self.state(), ControllerState::Paused) } @@ -83,7 +87,7 @@ impl Controller { } pub fn unpause(&self) { - if !self.is_cancelled() { + if !self.is_cancelled() | !self.is_failed() { self.set_state(ControllerState::Running); } } @@ -100,8 +104,8 @@ impl Clone for Controller { impl Drop for Controller { fn drop(&mut self) { - // Cancel operations if primary controller is dropped - if self.primary { + // Cancel operations if primary controller is dropped and controller is still running + if self.primary && self.state() != ControllerState::Failed { self.cancel(); } } diff --git a/src/operation/mod.rs b/src/operation/mod.rs index e9247a8..2dd5343 100644 --- a/src/operation/mod.rs +++ b/src/operation/mod.rs @@ -6,9 +6,9 @@ use crate::{ tab, }; use cosmic::iced::futures::{channel::mpsc::Sender, SinkExt}; -use std::fmt::Formatter; use std::{ borrow::Cow, + fmt::Formatter, fs, io::{self, Read, Write}, path::{Path, PathBuf}, @@ -90,8 +90,10 @@ async fn copy_or_move( controller: Controller, ) -> Result { let msg_tx = msg_tx.clone(); + let controller_c = controller.clone(); compio::runtime::spawn(async move { + let controller = controller_c; log::info!( "{} {:?} to {:?}", match method { @@ -150,6 +152,7 @@ async fn copy_or_move( let mut context = Context::new(controller.clone()); { + let controller = controller.clone(); context = context.on_progress(move |_op, progress| { let item_progress = match progress.total_bytes { Some(total_bytes) => { @@ -177,14 +180,12 @@ async fn copy_or_move( context .recursive_copy_or_move(from_to_pairs, method) - .await - .map_err(OperationError::from_str)?; + .await?; Result::::Ok(context.op_sel) }) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str) } fn copy_unique_path(from: &Path, to: &Path) -> PathBuf { @@ -374,13 +375,42 @@ pub struct OperationError { } impl OperationError { - pub fn from_str(err: T) -> Self { + pub fn from_state(state: ControllerState, controller: &Controller) -> Self { + let message = if state == ControllerState::Failed { + controller.set_state(ControllerState::Failed); + fl!("failed") + } else { + controller.cancel(); + fl!("cancelled") + }; + + Self { + kind: OperationErrorType::Generic(message), + } + } + + pub fn from_err(err: T, controller: &Controller) -> Self { + controller.set_state(ControllerState::Failed); + OperationError { kind: OperationErrorType::Generic(err.to_string()), } } + + pub fn from_kind(kind: OperationErrorType, controller: &Controller) -> Self { + controller.set_state(ControllerState::Failed); + OperationError { kind } + } + + pub fn from_msg(m: impl Into) -> Self { + OperationError { + kind: OperationErrorType::Generic(m.into()), + } + } } +impl std::error::Error for OperationError {} + impl std::fmt::Display for OperationError { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { match &self.kind { @@ -397,6 +427,7 @@ impl Operation { ControllerState::Running => fl!("progress", percent = percent), ControllerState::Paused => fl!("progress-paused", percent = percent), ControllerState::Cancelled => fl!("progress-cancelled", percent = percent), + ControllerState::Failed => fl!("progress-failed", percent = percent), }; match self { Self::Compress { paths, to, .. } => fl!( @@ -583,13 +614,15 @@ impl Operation { archive_type, password, } => { + let controller_c = controller.clone(); compio::runtime::spawn_blocking( move || -> Result { + let controller = controller_c; let Some(relative_root) = to.parent() else { - return Err(OperationError::from_str(format!( - "path {:?} has no parent directory", - to - ))); + return Err(OperationError::from_err( + format!("path {:?} has no parent directory", to), + &controller, + )); }; let op_sel = OperationSelection { @@ -602,7 +635,8 @@ impl Operation { if path.is_dir() { let new_paths_it = WalkDir::new(path).into_iter(); for entry in new_paths_it.skip(1) { - let entry = entry.map_err(OperationError::from_str)?; + let entry = entry + .map_err(|e| OperationError::from_err(e, &controller))?; paths.push(entry.into_path()); } } @@ -619,40 +653,50 @@ impl Operation { ) }) .map(tar::Builder::new) - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; let total_paths = paths.len(); for (i, path) in paths.iter().enumerate() { futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) + controller + .check() + .await + .map_err(|e| OperationError::from_state(e, &controller)) })?; controller.set_progress((i as f32) / total_paths as f32); if let Some(relative_path) = path .strip_prefix(relative_root) - .map_err(OperationError::from_str)? + .map_err(|e| OperationError::from_err(e, &controller))? .to_str() { archive .append_path_with_name(path, relative_path) - .map_err(OperationError::from_str)?; + .map_err(|e| { + OperationError::from_err(e, &controller) + })?; } } - archive.finish().map_err(OperationError::from_str)?; + archive + .finish() + .map_err(|e| OperationError::from_err(e, &controller))?; } ArchiveType::Zip => { let mut archive = fs::File::create(&to) .map(io::BufWriter::new) .map(zip::ZipWriter::new) - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; let total_paths = paths.len(); let mut buffer = vec![0; 4 * 1024 * 1024]; for (i, path) in paths.iter().enumerate() { futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller)) })?; controller.set_progress((i as f32) / total_paths as f32); @@ -666,15 +710,16 @@ impl Operation { } if let Some(relative_path) = path .strip_prefix(relative_root) - .map_err(OperationError::from_str)? + .map_err(|e| OperationError::from_err(e, &controller))? .to_str() { if path.is_file() { - let mut file = fs::File::open(path) - .map_err(OperationError::from_str)?; - let metadata = file - .metadata() - .map_err(OperationError::from_str)?; + let mut file = fs::File::open(path).map_err(|e| { + OperationError::from_err(e, &controller) + })?; + let metadata = file.metadata().map_err(|e| { + OperationError::from_err(e, &controller) + })?; let total = metadata.len(); if total >= 4 * 1024 * 1024 * 1024 { // The large file option must be enabled for files above 4 GiB @@ -688,25 +733,27 @@ impl Operation { } archive .start_file(relative_path, zip_options) - .map_err(OperationError::from_str)?; + .map_err(|e| { + OperationError::from_err(e, &controller) + })?; let mut current = 0; loop { futures::executor::block_on(async { - controller - .check() - .await - .map_err(OperationError::from_str) + controller.check().await.map_err(|s| { + OperationError::from_state(s, &controller) + }) })?; - let count = file - .read(&mut buffer) - .map_err(OperationError::from_str)?; + let count = + file.read(&mut buffer).map_err(|e| { + OperationError::from_err(e, &controller) + })?; if count == 0 { break; } - archive - .write_all(&buffer[..count]) - .map_err(OperationError::from_str)?; + archive.write_all(&buffer[..count]).map_err( + |e| OperationError::from_err(e, &controller), + )?; current += count; let file_progress = current as f32 / total as f32; @@ -717,12 +764,16 @@ impl Operation { } else { archive .add_directory(relative_path, zip_options) - .map_err(OperationError::from_str)?; + .map_err(|e| { + OperationError::from_err(e, &controller) + })?; } } } - archive.finish().map_err(OperationError::from_str)?; + archive + .finish() + .map_err(|e| OperationError::from_err(e, &controller))?; } } @@ -731,7 +782,6 @@ impl Operation { ) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str) } Self::Copy { paths, to } => { copy_or_move(paths, to, Method::Copy, msg_tx, controller).await @@ -740,15 +790,17 @@ impl Operation { let total = paths.len(); for (i, path) in paths.into_iter().enumerate() { futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller)) })?; controller.set_progress((i as f32) / (total as f32)); let _items_opt = compio::runtime::spawn_blocking(|| trash::delete(path)) .await - .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(wrap_compio_spawn_error)?; //TODO: items_opt allows for easy restore } Ok(OperationSelection::default()) @@ -764,23 +816,28 @@ impl Operation { ) ))] { + let controller_clone = controller.clone(); compio::runtime::spawn_blocking(move || -> Result<(), OperationError> { + let controller = controller_clone; let count = items.len(); for (i, item) in items.into_iter().enumerate() { futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller)) })?; controller.set_progress(i as f32 / count as f32); trash::os_limited::purge_all([item]) - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(()) }) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(OperationSelection::default()) } @@ -795,24 +852,30 @@ impl Operation { ) ))] { + let controller_clone = controller.clone(); compio::runtime::spawn_blocking(move || -> Result<(), OperationError> { - let items = trash::os_limited::list().map_err(OperationError::from_str)?; + let controller = controller_clone; + let items = trash::os_limited::list() + .map_err(|e| OperationError::from_err(e, &controller))?; let count = items.len(); for (i, item) in items.into_iter().enumerate() { futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller)) })?; controller.set_progress(i as f32 / count as f32); trash::os_limited::purge_all([item]) - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(()) }) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(OperationSelection::default()) } @@ -820,40 +883,46 @@ impl Operation { paths, to, password, - } => compio::runtime::spawn_blocking( - move || -> Result { - let total_paths = paths.len(); - let mut op_sel = OperationSelection::default(); - for (i, path) in paths.iter().enumerate() { - futures::executor::block_on(async { - controller.check().await.map_err(OperationError::from_str) - })?; + } => { + let controller_clone = controller.clone(); + compio::runtime::spawn_blocking( + move || -> Result { + let controller = controller_clone; + let total_paths = paths.len(); + let mut op_sel = OperationSelection::default(); + for (i, path) in paths.iter().enumerate() { + futures::executor::block_on(async { + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller)) + })?; - controller.set_progress((i as f32) / total_paths as f32); + controller.set_progress((i as f32) / total_paths as f32); - if let Some(file_name) = path.file_name().and_then(|f| f.to_str()) { - let dir_name = get_directory_name(file_name); - let mut new_dir = to.join(dir_name); + if let Some(file_name) = path.file_name().and_then(|f| f.to_str()) { + let dir_name = get_directory_name(file_name); + let mut new_dir = to.join(dir_name); - if new_dir.exists() { - if let Some(new_dir_parent) = new_dir.parent() { - new_dir = copy_unique_path(&new_dir, new_dir_parent); + if new_dir.exists() { + if let Some(new_dir_parent) = new_dir.parent() { + new_dir = copy_unique_path(&new_dir, new_dir_parent); + } } + + op_sel.ignored.push(path.clone()); + op_sel.selected.push(new_dir.clone()); + + crate::archive::extract(path, &new_dir, &password, &controller)?; } - - op_sel.ignored.push(path.clone()); - op_sel.selected.push(new_dir.clone()); - - crate::archive::extract(path, &new_dir, &password, &controller)?; } - } - Ok(op_sel) - }, - ) + Ok(op_sel) + }, + ) + } .await - .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str), + .map_err(wrap_compio_spawn_error)?, Self::Move { paths, to, @@ -868,36 +937,51 @@ impl Operation { ) .await } - Self::NewFolder { path } => compio::runtime::spawn(async move { - controller.check().await.map_err(OperationError::from_str)?; - compio::fs::create_dir(&path) - .await - .map_err(OperationError::from_str)?; - Result::<_, OperationError>::Ok(OperationSelection { - ignored: Vec::new(), - selected: vec![path], + Self::NewFolder { path } => { + let controller_clone = controller.clone(); + compio::runtime::spawn(async move { + let controller = controller_clone; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; + compio::fs::create_dir(&path) + .await + .map_err(|e| OperationError::from_err(e, &controller))?; + Result::<_, OperationError>::Ok(OperationSelection { + ignored: Vec::new(), + selected: vec![path], + }) }) - }) + } .await - .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str), - Self::NewFile { path } => compio::runtime::spawn(async move { - controller.check().await.map_err(OperationError::from_str)?; - compio::fs::File::create(&path) - .await - .map_err(OperationError::from_str)?; - Result::<_, OperationError>::Ok(OperationSelection { - ignored: Vec::new(), - selected: vec![path], + .map_err(wrap_compio_spawn_error)?, + Self::NewFile { path } => { + let controller_clone = controller.clone(); + compio::runtime::spawn(async move { + let controller = controller_clone; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; + compio::fs::File::create(&path) + .await + .map_err(|e| OperationError::from_err(e, &controller))?; + Result::<_, OperationError>::Ok(OperationSelection { + ignored: Vec::new(), + selected: vec![path], + }) }) - }) + } .await - .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str), + .map_err(wrap_compio_spawn_error)?, Self::PermanentlyDelete { paths } => { let total = paths.len(); for (idx, path) in paths.into_iter().enumerate() { - controller.check().await.map_err(OperationError::from_str)?; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; controller.set_progress((idx as f32) / (total as f32)); @@ -914,8 +998,8 @@ impl Operation { } }) .await - .map_err(OperationError::from_str)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))? + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(OperationSelection::default()) @@ -926,35 +1010,45 @@ impl Operation { recently_used_xbel::remove_recently_used(&path_refs) }) .await - .map_err(OperationError::from_str)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))? + .map_err(|e| OperationError::from_err(e, &controller))?; Ok(OperationSelection::default()) } - Self::Rename { from, to } => compio::runtime::spawn(async move { - controller.check().await.map_err(OperationError::from_str)?; - compio::fs::rename(&from, &to) - .await - .map_err(OperationError::from_str)?; - Result::<_, OperationError>::Ok(OperationSelection { - ignored: vec![from], - selected: vec![to], + Self::Rename { from, to } => { + let controller_clone = controller.clone(); + + compio::runtime::spawn(async move { + let controller = controller_clone; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; + compio::fs::rename(&from, &to) + .await + .map_err(|e| OperationError::from_err(e, &controller))?; + Result::<_, OperationError>::Ok(OperationSelection { + ignored: vec![from], + selected: vec![to], + }) }) - }) + } .await - .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str), + .map_err(wrap_compio_spawn_error)?, #[cfg(target_os = "macos")] Self::Restore { .. } => { // TODO: add support for macos - return OperationError::from_str("Restoring from trash is not supported on macos"); + return OperationError::from_msg("Restoring from trash is not supported on macos"); } #[cfg(not(target_os = "macos"))] Self::Restore { items } => { let total = items.len(); let mut paths = Vec::with_capacity(total); for (i, item) in items.into_iter().enumerate() { - controller.check().await.map_err(OperationError::from_str)?; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; controller.set_progress((i as f32) / (total as f32)); @@ -963,7 +1057,7 @@ impl Operation { compio::runtime::spawn_blocking(|| trash::os_limited::restore_all([item])) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(OperationSelection { ignored: Vec::new(), @@ -971,50 +1065,63 @@ impl Operation { }) } Self::SetExecutableAndLaunch { path } => { - controller.check().await.map_err(OperationError::from_str)?; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; + let controller_clone = controller.clone(); compio::runtime::spawn_blocking(move || -> Result<(), OperationError> { + let controller = controller_clone; //TODO: what to do on non-Unix systems? #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; let mut perms = fs::metadata(&path) - .map_err(OperationError::from_str)? + .map_err(|e| OperationError::from_err(e, &controller))? .permissions(); let current_mode = perms.mode(); let new_mode = current_mode | 0o111; perms.set_mode(new_mode); - fs::set_permissions(&path, perms).map_err(OperationError::from_str)?; + fs::set_permissions(&path, perms) + .map_err(|e| OperationError::from_err(e, &controller))?; } let mut command = std::process::Command::new(path); - spawn_detached(&mut command).map_err(OperationError::from_str)?; + spawn_detached(&mut command) + .map_err(|e| OperationError::from_err(e, &controller))?; Ok(()) }) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; Ok(OperationSelection::default()) } Self::SetPermissions { path, mode } => { - controller.check().await.map_err(OperationError::from_str)?; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; + let controller_clone = controller.clone(); compio::runtime::spawn_blocking(move || -> Result<(), OperationError> { + let controller = controller_clone; //TODO: what to do on non-Unix systems? #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; let perms = fs::Permissions::from_mode(mode); - fs::set_permissions(&path, perms).map_err(OperationError::from_str)?; + fs::set_permissions(&path, perms) + .map_err(|e| OperationError::from_err(e, &controller))?; } Ok(()) }) .await .map_err(wrap_compio_spawn_error)? - .map_err(OperationError::from_str)?; + .map_err(|e| OperationError::from_err(e, &controller))?; Ok(OperationSelection::default()) } }; @@ -1026,13 +1133,18 @@ impl Operation { } #[track_caller] -fn wrap_compio_spawn_error(_unwind: Box) -> OperationError { +fn wrap_compio_spawn_error(err: Box) -> OperationError { log::error!( "compio runtime spawn failed: {}", std::backtrace::Backtrace::capture() ); - OperationError::from_str("compio runtime spawn failed") + // Preserve error if it's already an OperationError + if let Ok(err) = err.downcast() { + *err + } else { + OperationError::from_msg("compio runtime spawn failed") + } } #[cfg(test)] diff --git a/src/operation/reader.rs b/src/operation/reader.rs index a037eba..6447a1e 100644 --- a/src/operation/reader.rs +++ b/src/operation/reader.rs @@ -1,5 +1,7 @@ use std::{fs, io, path::Path}; +use crate::operation::OperationError; + use super::Controller; // Special reader just for operations, handling cancel and progress @@ -29,7 +31,7 @@ impl io::Read for OpReader { self.controller .check() .await - .map_err(|err| io::Error::new(io::ErrorKind::Other, err)) + .map_err(|s| io::Error::other(OperationError::from_state(s, &self.controller))) })?; let count = self.file.read(buf)?; diff --git a/src/operation/recursive.rs b/src/operation/recursive.rs index bc2fdc3..23f7e53 100644 --- a/src/operation/recursive.rs +++ b/src/operation/recursive.rs @@ -7,6 +7,8 @@ use std::time::Instant; use std::{cell::Cell, error::Error, fs, ops::ControlFlow, path::PathBuf, rc::Rc}; use walkdir::WalkDir; +use crate::operation::OperationError; + use super::{copy_unique_path, Controller, OperationSelection, ReplaceResult}; pub enum Method { @@ -52,11 +54,14 @@ impl Context { &mut self, from_to_pairs: Vec<(PathBuf, PathBuf)>, method: Method, - ) -> Result { + ) -> Result { let mut ops = Vec::new(); let mut cleanup_ops = Vec::new(); for (from_parent, to_parent) in from_to_pairs { - self.controller.check().await?; + self.controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &self.controller))?; if from_parent == to_parent { // Skip matching source and destination @@ -64,10 +69,16 @@ impl Context { } for entry in WalkDir::new(&from_parent).into_iter() { - self.controller.check().await?; + self.controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &self.controller))?; let entry = entry.map_err(|err| { - format!("failed to walk directory {:?}: {}", from_parent, err) + OperationError::from_err( + format!("failed to walk directory {:?}: {}", from_parent, err), + &self.controller, + ) })?; let file_type = entry.file_type(); let from = entry.into_path(); @@ -79,21 +90,31 @@ impl Context { Method::Move { cross_device_copy } => OpKind::Move { cross_device_copy }, } } else if file_type.is_symlink() { - let target = fs::read_link(&from) - .map_err(|err| format!("failed to read link {:?}: {}", from, err))?; + let target = fs::read_link(&from).map_err(|err| { + OperationError::from_err( + format!("failed to read link {:?}: {}", from, err), + &self.controller, + ) + })?; OpKind::Symlink { target } } else { //TODO: present dialog and allow continue - return Err(format!("{} is not a known file type", from.display())); + return Err(OperationError::from_err( + format!("{} is not a known file type", from.display()), + &self.controller, + )); }; let to = if from == from_parent { // When copying a file, from matches from_parent, and to_parent must be used to_parent.clone() } else { let relative = from.strip_prefix(&from_parent).map_err(|err| { - format!( - "failed to remove prefix {:?} from {:?}: {}", - from_parent, from, err + OperationError::from_err( + format!( + "failed to remove prefix {:?} from {:?}: {}", + from_parent, from, err + ), + &self.controller, ) })?; //TODO: ensure to is inside of to_parent? @@ -127,7 +148,10 @@ impl Context { let total_ops = ops.len(); for (current_ops, mut op) in ops.into_iter().enumerate() { - self.controller.check().await?; + self.controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &self.controller))?; let progress = Progress { current_ops, @@ -137,9 +161,12 @@ impl Context { }; (self.on_progress)(&op, &progress); if op.run(self, progress).await.map_err(|err| { - format!( - "failed to {:?} {:?} to {:?}: {}", - op.kind, op.from, op.to, err + OperationError::from_err( + format!( + "failed to {:?} {:?} to {:?}: {}", + op.kind, op.from, op.to, err + ), + &self.controller, ) })? { // The from path is ignored in the operation selection if it is a top level item @@ -336,9 +363,9 @@ impl Op { (ctx.on_progress)(self, &progress); // Also check if the progress was cancelled. - if let Err(why) = ctx.controller.check().await { + if let Err(state) = ctx.controller.check().await { ctx.buf = buf_out; - return Err(why.into()); + return Err(OperationError::from_state(state, &ctx.controller).into()); } } diff --git a/src/tab.rs b/src/tab.rs index 710ccb6..8e2a118 100644 --- a/src/tab.rs +++ b/src/tab.rs @@ -81,7 +81,7 @@ use crate::{ mime_icon::{mime_for_path, mime_icon}, mounter::MOUNTERS, mouse_area, - operation::Controller, + operation::{Controller, OperationError}, thumbnail_cacher::{CachedThumbnail, ThumbnailCacher, ThumbnailSize}, thumbnailer::thumbnailer, }; @@ -2469,10 +2469,13 @@ pub struct Tab { window_id: Option, } -async fn calculate_dir_size(path: &Path, controller: Controller) -> Result { +async fn calculate_dir_size(path: &Path, controller: Controller) -> Result { let mut total = 0; for entry_res in WalkDir::new(path) { - controller.check().await?; + controller + .check() + .await + .map_err(|s| OperationError::from_state(s, &controller))?; //TODO: report more errors? if let Ok(entry) = entry_res { @@ -5717,7 +5720,7 @@ impl Tab { ); Message::DirectorySize( path.clone(), - DirSize::Error(err), + DirSize::Error(err.to_string()), ) } }