types/File: return Results instead of panicking
Tests / Test on ${{ matrix.build }} (linux-amd64, ubuntu-latest, stable, x86_64-unknown-linux-gnu) (pull_request) Successful in 12m54s Details
Tests / Test on ${{ matrix.build }} (linux-amd64, ubuntu-latest, stable, x86_64-unknown-linux-gnu) (push) Successful in 23m47s Details

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
pull/316/head
Manos Pitsidianakis 2023-11-26 19:28:11 +02:00
parent 470cae6b88
commit a1cbb1988b
Signed by: Manos Pitsidianakis
GPG Key ID: 7729C7707F7E09D0
6 changed files with 235 additions and 139 deletions

View File

@ -1207,15 +1207,15 @@ impl Account {
if let Some(mailbox_hash) = saved_at {
Ok(mailbox_hash)
} else {
let file = crate::types::create_temp_file(bytes, None, None, Some("eml"), false);
log::trace!("message saved in {}", file.path.display());
let file = crate::types::File::create_temp_file(bytes, None, None, Some("eml"), false)?;
log::trace!("message saved in {}", file.path().display());
log::info!(
"Message was stored in {} so that you can restore it manually.",
file.path.display()
file.path().display()
);
Err(Error::new(format!(
"Message was stored in {} so that you can restore it manually.",
file.path.display()
file.path().display()
))
.set_summary("Could not save in any mailbox"))
}
@ -1958,22 +1958,33 @@ impl Account {
.job_executor
.set_job_success(job_id, false);
log::error!("Could not save message: {err}");
let file =
crate::types::create_temp_file(bytes, None, None, Some("eml"), false);
log::debug!("message saved in {}", file.path.display());
log::info!(
"Message was stored in {} so that you can restore it manually.",
file.path.display()
);
self.main_loop_handler
.send(ThreadEvent::UIEvent(UIEvent::Notification(
Some(format!("{}: could not save message", &self.name)),
format!(
match crate::types::File::create_temp_file(
bytes,
None,
None,
Some("eml"),
false,
) {
Ok(file) => {
log::debug!("message saved in {}", file.path().display());
log::info!(
"Message was stored in {} so that you can restore it manually.",
file.path.display()
),
Some(crate::types::NotificationType::Info),
)));
file.path().display()
);
self.main_loop_handler.send(ThreadEvent::UIEvent(
UIEvent::Notification(
Some(format!("{}: could not save message", &self.name)),
format!(
"Message was stored in {} so that you can restore it \
manually.",
file.path().display()
),
Some(crate::types::NotificationType::Info),
),
));
}
Err(err) => log::error!("Could not save message: {err}"),
}
}
}
JobRequest::SendMessage => {}

View File

@ -841,8 +841,12 @@ To: {}
}
fn update_from_file(&mut self, file: File, context: &mut Context) -> bool {
let result = file.read_to_string();
match self.draft.update(result.as_str()) {
match file.read_to_string().and_then(|res| {
self.draft.update(res.as_str()).map_err(|err| {
self.draft.set_body(res);
err
})
}) {
Ok(has_changes) => {
self.has_changes = has_changes;
true
@ -850,13 +854,9 @@ To: {}
Err(err) => {
context.replies.push_back(UIEvent::Notification(
Some("Could not parse draft headers correctly.".to_string()),
format!(
"{}\nThe invalid text has been set as the body of your draft",
&err
),
format!("{err}\nThe invalid text has been set as the body of your draft",),
Some(NotificationType::Error(melib::error::ErrorKind::None)),
));
self.draft.set_body(result);
self.has_changes = true;
false
}
@ -1898,13 +1898,24 @@ impl Component for Composer {
.clone(),
);
let f = create_temp_file(
let f = match File::create_temp_file(
self.draft.to_edit_string().as_bytes(),
None,
None,
Some("eml"),
true,
);
) {
Ok(f) => f,
Err(err) => {
context.replies.push_back(UIEvent::Notification(
None,
err.to_string(),
Some(NotificationType::Error(err.kind)),
));
self.set_dirty(true);
return true;
}
};
if *account_settings!(context[self.account_hash].composing.embed) {
match crate::terminal::embed::create_pty(
@ -1966,8 +1977,12 @@ impl Component for Composer {
}
}
context.replies.push_back(UIEvent::Fork(ForkType::Finished));
let result = f.read_to_string();
match self.draft.update(result.as_str()) {
match f.read_to_string().and_then(|res| {
self.draft.update(res.as_str()).map_err(|err| {
self.draft.set_body(res);
err
})
}) {
Ok(has_changes) => {
self.has_changes = has_changes;
}
@ -1975,12 +1990,10 @@ impl Component for Composer {
context.replies.push_back(UIEvent::Notification(
Some("Could not parse draft headers correctly.".to_string()),
format!(
"{}\nThe invalid text has been set as the body of your draft",
&err
"{err}\nThe invalid text has been set as the body of your draft",
),
Some(NotificationType::Error(melib::error::ErrorKind::None)),
));
self.draft.set_body(result);
self.has_changes = true;
}
}
@ -1998,15 +2011,21 @@ impl Component for Composer {
));
return false;
}
let f = create_temp_file(&[], None, None, None, true);
match Command::new("sh")
.args(["-c", command])
.stdin(Stdio::null())
.stdout(Stdio::from(f.file()))
.spawn()
.and_then(|child| Ok(child.wait_with_output()?.stderr))
match File::create_temp_file(&[], None, None, None, true)
.and_then(|f| {
let std_file = f.as_std_file()?;
Ok((
f,
Command::new("sh")
.args(["-c", command])
.stdin(Stdio::null())
.stdout(Stdio::from(std_file))
.spawn()?,
))
})
.and_then(|(f, child)| Ok((f, child.wait_with_output()?.stderr)))
{
Ok(stderr) => {
Ok((f, stderr)) => {
if !stderr.is_empty() {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(format!(
@ -2016,7 +2035,7 @@ impl Component for Composer {
));
}
let attachment =
match melib::email::compose::attachment_from_file(f.path()) {
match melib::email::compose::attachment_from_file(&f.path()) {
Ok(a) => a,
Err(err) => {
context.replies.push_back(UIEvent::Notification(
@ -2548,7 +2567,7 @@ pub fn send_draft_async(
))))
.unwrap();
} else if !store_sent_mail && is_ok {
let f = create_temp_file(message.as_bytes(), None, None, Some("eml"), false);
let f = File::create_temp_file(message.as_bytes(), None, None, Some("eml"), false)?;
log::info!(
"store_sent_mail is false; stored sent mail to {}",
f.path().display()

View File

@ -1532,33 +1532,36 @@ impl Component for EnvelopeView {
let attachment_type = attachment.mime_type();
let filename = attachment.filename();
if let Ok(command) = query_default_app(&attachment_type) {
let p = create_temp_file(
match File::create_temp_file(
&attachment.decode(Default::default()),
filename.as_deref(),
None,
None,
true,
);
let exec_cmd = desktop_exec_to_command(
&command,
p.path.display().to_string(),
false,
);
match Command::new("sh")
.args(["-c", &exec_cmd])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
{
Ok(child) => {
)
.and_then(|p| {
let exec_cmd = desktop_exec_to_command(
&command,
p.path().display().to_string(),
false,
);
Ok((
p,
Command::new("sh")
.args(["-c", &exec_cmd])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()?,
))
}) {
Ok((p, child)) => {
context.temp_files.push(p);
context.children.push(child);
}
Err(err) => {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(format!(
"Failed to start `{}`: {}",
&exec_cmd, err
"Failed to execute command: {err}"
)),
));
}

View File

@ -173,31 +173,36 @@ impl Component for HtmlView {
command
};
if let Some(command) = command {
let p = create_temp_file(&self.bytes, None, None, Some("html"), true);
context
.replies
.push_back(UIEvent::StatusEvent(StatusEvent::UpdateSubStatus(
command.to_string(),
)));
let exec_cmd =
super::desktop_exec_to_command(&command, p.path.display().to_string(), false);
match File::create_temp_file(&self.bytes, None, None, Some("html"), true).and_then(
|p| {
let exec_cmd = super::desktop_exec_to_command(
&command,
p.path().display().to_string(),
false,
);
match Command::new("sh")
.args(["-c", &exec_cmd])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()
{
Ok(child) => {
Ok((
p,
Command::new("sh")
.args(["-c", &exec_cmd])
.stdin(Stdio::piped())
.stdout(Stdio::piped())
.spawn()?,
))
},
) {
Ok((p, child)) => {
context.temp_files.push(p);
context.children.push(child);
}
Err(err) => {
context.replies.push_back(UIEvent::StatusEvent(
StatusEvent::DisplayMessage(format!(
"Failed to start `{}`: {}",
&exec_cmd, err
)),
StatusEvent::DisplayMessage(format!("Failed to start {err}",)),
));
}
}

View File

@ -34,7 +34,7 @@ use melib::{email::Attachment, log, text_processing::GlobMatch, Error, Result};
use crate::{
state::Context,
types::{create_temp_file, ForkType, UIEvent},
types::{File, ForkType, UIEvent},
};
pub struct MailcapEntry {
@ -165,31 +165,34 @@ impl MailcapEntry {
.map(|arg| match *arg {
"%s" => {
needs_stdin = false;
let _f = create_temp_file(
let _f = File::create_temp_file(
&a.decode(Default::default()),
None,
None,
None,
true,
);
)?;
let p = _f.path().display().to_string();
f = Some(_f);
p
Ok(p)
}
"%t" => a.content_type().to_string(),
"%t" => Ok(a.content_type().to_string()),
param if param.starts_with("%{") && param.ends_with('}') => {
let param = &param["%{".len()..param.len() - 1];
if let Some(v) = params.iter().find(|(k, _)| *k == param.as_bytes()) {
String::from_utf8_lossy(v.1).into()
} else if param == "charset" {
String::from("utf-8")
} else {
String::new()
}
Ok(
if let Some(v) = params.iter().find(|(k, _)| *k == param.as_bytes())
{
String::from_utf8_lossy(v.1).into()
} else if param == "charset" {
String::from("utf-8")
} else {
String::new()
},
)
}
a => a.to_string(),
a => Ok(a.to_string()),
})
.collect::<Vec<String>>();
.collect::<Result<Vec<String>>>()?;
let cmd_string = format!("{} {}", cmd, args.join(" "));
log::trace!("Executing: sh -c \"{}\"", cmd_string.replace('"', "\\\""));
if copiousoutput {

View File

@ -24,14 +24,17 @@ use std::{
fs::OpenOptions,
io::{Read, Write},
os::unix::fs::PermissionsExt,
path::PathBuf,
path::{Path, PathBuf},
};
use melib::uuid::Uuid;
use melib::{error::*, uuid::Uuid};
/// Temporary file that can optionally cleaned up when it is dropped.
#[derive(Debug)]
pub struct File {
pub path: PathBuf,
/// File's path.
path: PathBuf,
/// Delete file when it is dropped.
delete_on_drop: bool,
}
@ -44,69 +47,121 @@ impl Drop for File {
}
impl File {
pub fn file(&self) -> std::fs::File {
/// Open as a standard library file type.
pub fn as_std_file(&self) -> Result<std::fs::File> {
OpenOptions::new()
.read(true)
.write(true)
.create(true)
.open(&self.path)
.unwrap()
.chain_err_summary(|| format!("Could not create/open path {}", self.path.display()))
}
pub fn path(&self) -> &PathBuf {
/// The file's path.
pub fn path(&self) -> &Path {
&self.path
}
pub fn read_to_string(&self) -> String {
let mut buf = Vec::new();
let mut f = fs::File::open(&self.path)
.unwrap_or_else(|_| panic!("Can't open {}", &self.path.display()));
f.read_to_end(&mut buf)
.unwrap_or_else(|_| panic!("Can't read {}", &self.path.display()));
String::from_utf8(buf).unwrap()
}
}
/// Returned `File` will be deleted when dropped if delete_on_drop is set, so
/// make sure to add it on `context.temp_files` to reap it later.
pub fn create_temp_file(
bytes: &[u8],
filename: Option<&str>,
path: Option<&mut PathBuf>,
extension: Option<&str>,
delete_on_drop: bool,
) -> File {
let mut dir = std::env::temp_dir();
let path = path.unwrap_or_else(|| {
dir.push("meli");
std::fs::DirBuilder::new()
.recursive(true)
.create(&dir)
.unwrap();
if let Some(filename) = filename {
dir.push(filename)
} else {
let u = Uuid::new_v4();
dir.push(u.as_simple().to_string());
/// Convenience method to read `File` to `String`.
pub fn read_to_string(&self) -> Result<String> {
fn inner(path: &Path) -> Result<String> {
let mut buf = Vec::new();
let mut f = fs::File::open(path)?;
f.read_to_end(&mut buf)?;
Ok(String::from_utf8(buf)?)
}
&mut dir
});
if let Some(ext) = extension {
path.set_extension(ext);
inner(&self.path).chain_err_summary(|| format!("Can't read {}", self.path.display()))
}
let mut f = std::fs::File::create(&path).unwrap();
let metadata = f.metadata().unwrap();
let mut permissions = metadata.permissions();
/// Returned `File` will be deleted when dropped if delete_on_drop is set,
/// so make sure to add it on `context.temp_files` to reap it later.
pub fn create_temp_file(
bytes: &[u8],
filename: Option<&str>,
path: Option<&mut PathBuf>,
extension: Option<&str>,
delete_on_drop: bool,
) -> Result<Self> {
let mut dir = std::env::temp_dir();
permissions.set_mode(0o600); // Read/write for owner only.
f.set_permissions(permissions).unwrap();
let path = if let Some(p) = path {
p
} else {
dir.push("meli");
std::fs::DirBuilder::new().recursive(true).create(&dir)?;
if let Some(filename) = filename {
dir.push(filename)
} else {
let u = Uuid::new_v4();
dir.push(u.as_simple().to_string());
}
&mut dir
};
if let Some(ext) = extension {
path.set_extension(ext);
}
fn inner(path: &Path, bytes: &[u8], delete_on_drop: bool) -> Result<File> {
let mut f = std::fs::File::create(path)?;
let metadata = f.metadata()?;
let mut permissions = metadata.permissions();
f.write_all(bytes).unwrap();
f.flush().unwrap();
File {
path: path.clone(),
delete_on_drop,
permissions.set_mode(0o600); // Read/write for owner only.
f.set_permissions(permissions)?;
f.write_all(bytes)?;
f.flush()?;
Ok(File {
path: path.to_path_buf(),
delete_on_drop,
})
}
inner(path, bytes, delete_on_drop)
.chain_err_summary(|| format!("Could not create file at path {}", path.display()))
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_file_invalid_path() {
let f = File {
path: PathBuf::from("//////"),
delete_on_drop: true,
};
f.as_std_file().unwrap_err();
}
#[test]
fn test_file_delete_on_drop() {
const S: &str = "hello world";
let tempdir = tempfile::tempdir().unwrap();
let delete_on_drop = File::create_temp_file(
S.as_bytes(),
None,
Some(&mut tempdir.path().join("test")),
None,
true,
)
.unwrap();
assert_eq!(&delete_on_drop.read_to_string().unwrap(), S);
drop(delete_on_drop);
assert!(!tempdir.path().join("test").try_exists().unwrap());
let persist = File::create_temp_file(
S.as_bytes(),
None,
Some(&mut tempdir.path().join("test")),
None,
false,
)
.unwrap();
assert_eq!(&persist.read_to_string().unwrap(), S);
drop(persist);
assert!(tempdir.path().join("test").try_exists().unwrap());
_ = tempdir.close();
}
}