From 3cac1eb488f4d634b307912c98d2e596d3098cb6 Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Mon, 10 Jun 2019 09:12:51 -0400
Subject: [PATCH] Start implementing errors for pipes
---
src/lib/shell/pipe_exec/fork.rs | 15 +++---
src/lib/shell/pipe_exec/mod.rs | 90 +++++++++++++++------------------
2 files changed, 49 insertions(+), 56 deletions(-)
diff --git a/src/lib/shell/pipe_exec/fork.rs b/src/lib/shell/pipe_exec/fork.rs
index 43cb23d9..d6f44d8d 100644
--- a/src/lib/shell/pipe_exec/fork.rs
+++ b/src/lib/shell/pipe_exec/fork.rs
@@ -5,7 +5,6 @@ use super::{
job_control::{BackgroundProcess, ProcessState},
};
use crate::parser::pipelines::Pipeline;
-use std::process::exit;
impl<'a> Shell<'a> {
/// Ensures that the forked child is given a unique process ID.
@@ -31,7 +30,14 @@ impl<'a> Shell<'a> {
Self::create_process_group(0);
// After execution of it's commands, exit with the last command's status.
- sys::fork_exit(self.pipe(pipeline).as_os_code());
+ sys::fork_exit(
+ self.pipe(pipeline)
+ .unwrap_or_else(|err| {
+ eprintln!("{}", err);
+ Status::COULD_NOT_EXEC
+ })
+ .as_os_code(),
+ );
}
Ok(pid) => {
if state != ProcessState::Empty {
@@ -40,10 +46,7 @@ impl<'a> Shell<'a> {
}
Status::SUCCESS
}
- Err(why) => {
- eprintln!("ion: background fork failed: {}", why);
- exit(1);
- }
+ Err(why) => Status::error(format!("ion: background fork failed: {}", why)),
}
}
}
diff --git a/src/lib/shell/pipe_exec/mod.rs b/src/lib/shell/pipe_exec/mod.rs
index fe9e31ee..3383f40d 100644
--- a/src/lib/shell/pipe_exec/mod.rs
+++ b/src/lib/shell/pipe_exec/mod.rs
@@ -41,9 +41,9 @@ use std::{
#[derive(Debug, Error)]
pub enum InputError {
- #[error(display = "ion: failed to redirect '{}' to stdin: {}", file, why)]
+ #[error(display = "failed to redirect '{}' to stdin: {}", file, why)]
File { file: String, why: io::Error },
- #[error(display = "ion: failed to redirect herestring '{}' to stdin: {}", string, why)]
+ #[error(display = "failed to redirect herestring '{}' to stdin: {}", string, why)]
HereString { string: String, why: io::Error },
}
@@ -54,17 +54,25 @@ pub struct OutputError {
why: io::Error,
}
-#[derive(Debug)]
+#[derive(Debug, Error)]
pub enum RedirectError {
- Input(InputError),
- Output(OutputError),
+ #[error(display = "{}", cause)]
+ Input { cause: InputError },
+ #[error(display = "{}", cause)]
+ Output { cause: OutputError },
+}
+
+#[derive(Debug, Error)]
+pub enum PipeError {
+ #[error(display = "ion: {}", cause)]
+ RedirectError { cause: RedirectError },
}
impl fmt::Display for OutputError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(
f,
- "ion: failed to redirect {} to file '{}': {}",
+ "failed to redirect {} to file '{}': {}",
match self.redirect {
RedirectFrom::Both => "both stdout and stderr",
RedirectFrom::Stdout => "stdout",
@@ -81,30 +89,16 @@ impl std::error::Error for OutputError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.why) }
}
-impl fmt::Display for RedirectError {
- fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
- match self {
- RedirectError::Output(why) => write!(f, "ion: failed to redirect {}", why),
- RedirectError::Input(why) => write!(f, "ion: failed to redirect {}", why),
- }
- }
-}
-
-impl std::error::Error for RedirectError {
- fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
- Some(match self {
- RedirectError::Output(why) => why,
- RedirectError::Input(why) => why,
- })
- }
-}
-
impl From<OutputError> for RedirectError {
- fn from(error: OutputError) -> Self { RedirectError::Output(error) }
+ fn from(cause: OutputError) -> Self { RedirectError::Output { cause } }
}
impl From<InputError> for RedirectError {
- fn from(error: InputError) -> Self { RedirectError::Input(error) }
+ fn from(cause: InputError) -> Self { RedirectError::Input { cause } }
+}
+
+impl From<RedirectError> for PipeError {
+ fn from(cause: RedirectError) -> Self { PipeError::RedirectError { cause } }
}
/// Create an OS pipe and write the contents of a byte slice to one end
@@ -442,7 +436,10 @@ impl<'b> Shell<'b> {
PipeType::Background => self.fork_pipe(pipeline, command, ProcessState::Running),
// Execute each command in the pipeline, giving each command the foreground.
PipeType::Normal => {
- let exit_status = self.pipe(pipeline);
+ let exit_status = self.pipe(pipeline).unwrap_or_else(|err| {
+ eprintln!("{}", err);
+ Status::COULD_NOT_EXEC
+ });
// Set the shell as the foreground process again to regain the TTY.
if !self.opts.is_background_shell {
let _ = sys::tcsetpgrp(0, process::id());
@@ -455,14 +452,8 @@ impl<'b> Shell<'b> {
/// Executes a piped job `job1 | job2 | job3`
///
/// This function will panic if called with an empty slice
- fn pipe(&mut self, pipeline: Pipeline<'b>) -> Status {
- let mut commands = match prepare(self, pipeline) {
- Ok(c) => c.into_iter().peekable(),
- Err(why) => {
- eprintln!("{}", why);
- return Status::COULD_NOT_EXEC;
- }
- };
+ fn pipe(&mut self, pipeline: Pipeline<'b>) -> Result<Status, PipeError> {
+ let mut commands = prepare(self, pipeline)?.into_iter().peekable();
if let Some((mut parent, mut kind)) = commands.next() {
if kind == RedirectFrom::None && !parent.needs_forking() {
@@ -471,7 +462,7 @@ impl<'b> Shell<'b> {
let _ = io::stdout().flush();
let _ = io::stderr().flush();
- status
+ Ok(status)
} else {
let (mut pgid, mut last_pid, mut current_pid) = (0, 0, 0);
@@ -510,18 +501,17 @@ impl<'b> Shell<'b> {
RedirectFrom::None => (),
RedirectFrom::Stderr => parent.stderr(writer),
RedirectFrom::Stdout => parent.stdout(writer),
- RedirectFrom::Both => match writer.try_clone() {
- Err(e) => {
- eprintln!(
- "ion: failed to redirect stdout and stderr: {}",
- e
- );
- }
- Ok(duped) => {
- parent.stderr(writer);
- parent.stdout(duped);
- }
- },
+ RedirectFrom::Both => {
+ let duped = writer.try_clone().map_err(|why| {
+ RedirectError::from(OutputError {
+ redirect: kind,
+ file: "pipe".to_string(),
+ why,
+ })
+ })?;
+ parent.stderr(writer);
+ parent.stdout(duped);
+ }
}
}
}
@@ -554,10 +544,10 @@ impl<'b> Shell<'b> {
let _ = io::stdout().flush();
let _ = io::stderr().flush();
}
- status
+ Ok(status)
}
} else {
- Status::SUCCESS
+ Ok(Status::SUCCESS)
}
}
}
--
GitLab