Make non-zero exit codes errors

crates/nu-cli/src/config_files.rs

@@ -214,7 +214,8 @@ pub fn eval_config_contents(
             let prev_file = engine_state.file.take();
             engine_state.file = Some(config_path.clone());
 
-            eval_source(
+            // TODO: ignore this error?
+            let _ = eval_source(
                 engine_state,
                 stack,
                 &contents,

crates/nu-cli/src/eval_cmds.rs

@@ -92,11 +92,7 @@ pub fn evaluate_commands(
             t_mode.coerce_str()?.parse().unwrap_or_default();
     }
 
-    if let Some(status) = pipeline.print(engine_state, stack, no_newline, false)? {
+    pipeline.print(engine_state, stack, no_newline, false)?;
-        if status.code() != 0 {
-            std::process::exit(status.code())
-        }
-    }
 
     info!("evaluate {}:{}:{}", file!(), line!(), column!());
 

crates/nu-cli/src/eval_file.rs

@@ -118,11 +118,7 @@ pub fn evaluate_file(
             };
 
         // Print the pipeline output of the last command of the file.
-        if let Some(status) = pipeline.print(engine_state, stack, true, false)? {
+        pipeline.print(engine_state, stack, true, false)?;
-            if status.code() != 0 {
-                std::process::exit(status.code())
-            }
-        }
 
         // Invoke the main command with arguments.
         // Arguments with whitespaces are quoted, thus can be safely concatenated by whitespace.
@@ -140,7 +136,7 @@ pub fn evaluate_file(
     };
 
     if exit_code != 0 {
-        std::process::exit(exit_code)
+        std::process::exit(exit_code);
     }
 
     info!("evaluate {}:{}:{}", file!(), line!(), column!());

crates/nu-cli/src/util.rs

@@ -210,18 +210,22 @@ pub fn eval_source(
     let start_time = std::time::Instant::now();
 
     let exit_code = match evaluate_source(engine_state, stack, source, fname, input, allow_return) {
-        Ok(code) => code.unwrap_or(0),
+        Ok(failed) => {
+            let code = i32::from(failed);
+            stack.add_env_var(
+                "LAST_EXIT_CODE".into(),
+                Value::int(code.into(), Span::unknown()),
+            );
+            code
+        }
         Err(err) => {
             report_error_new(engine_state, &err);
-            1
+            let code = err.exit_code();
+            stack.set_last_exit_code(&err);
+            code
         }
     };
 
-    stack.add_env_var(
-        "LAST_EXIT_CODE".to_string(),
-        Value::int(exit_code.into(), Span::unknown()),
-    );
-
     // reset vt processing, aka ansi because illbehaved externals can break it
     #[cfg(windows)]
     {
@@ -244,7 +248,7 @@ fn evaluate_source(
     fname: &str,
     input: PipelineData,
     allow_return: bool,
-) -> Result<Option<i32>, ShellError> {
+) -> Result<bool, ShellError> {
     let (block, delta) = {
         let mut working_set = StateWorkingSet::new(engine_state);
         let output = parse(
@@ -259,7 +263,12 @@ fn evaluate_source(
 
         if let Some(err) = working_set.parse_errors.first() {
             report_error(&working_set, err);
-            return Ok(Some(1));
+            return Ok(true);
+        }
+
+        if let Some(err) = working_set.compile_errors.first() {
+            report_error(&working_set, err);
+            // Not a fatal error, for now
         }
 
         if let Some(err) = working_set.compile_errors.first() {
@@ -278,25 +287,23 @@ fn evaluate_source(
         eval_block::<WithoutDebug>(engine_state, stack, &block, input)
     }?;
 
-    let status = if let PipelineData::ByteStream(..) = pipeline {
+    if let PipelineData::ByteStream(..) = pipeline {
-        pipeline.print(engine_state, stack, false, false)?
+        pipeline.print(engine_state, stack, false, false)
+    } else if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
+        let pipeline = eval_hook(
+            engine_state,
+            stack,
+            Some(pipeline),
+            vec![],
+            &hook,
+            "display_output",
+        )?;
+        pipeline.print(engine_state, stack, false, false)
     } else {
-        if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
+        pipeline.print(engine_state, stack, true, false)
-            let pipeline = eval_hook(
+    }?;
-                engine_state,
-                stack,
-                Some(pipeline),
-                vec![],
-                &hook,
-                "display_output",
-            )?;
-            pipeline.print(engine_state, stack, false, false)
-        } else {
-            pipeline.print(engine_state, stack, true, false)
-        }?
-    };
 
-    Ok(status.map(|status| status.code()))
+    Ok(false)
 }
 
 #[cfg(test)]

crates/nu-cmd-lang/src/core_commands/do_.rs

@@ -1,7 +1,7 @@
 use nu_engine::{command_prelude::*, get_eval_block_with_early_return, redirect_env};
 use nu_protocol::{
     engine::Closure,
-    process::{ChildPipe, ChildProcess, ExitStatus},
+    process::{ChildPipe, ChildProcess},
     ByteStream, ByteStreamSource, OutDest,
 };
 use std::{
@@ -147,13 +147,7 @@ impl Command for Do {
                             None
                         };
 
-                        if child.wait()? != ExitStatus::Exited(0) {
+                        child.wait()?.check_ok(span)?;
-                            return Err(ShellError::ExternalCommand {
-                                label: "External command failed".to_string(),
-                                help: stderr_msg,
-                                span,
-                            });
-                        }
 
                         let mut child = ChildProcess::from_raw(None, None, None, span);
                         if let Some(stdout) = stdout {

crates/nu-cmd-lang/src/core_commands/for_.rs

@@ -93,25 +93,10 @@ impl Command for For {
                     stack.add_var(var_id, x);
 
                     match eval_block(&engine_state, stack, block, PipelineData::empty()) {
-                        Err(ShellError::Break { .. }) => {
+                        Err(ShellError::Break { .. }) => break,
-                            break;
+                        Err(ShellError::Continue { .. }) => continue,
-                        }
+                        Err(err) => return Err(err),
-                        Err(ShellError::Continue { .. }) => {
+                        Ok(data) => data.drain()?,
-                            continue;
-                        }
-                        Err(err) => {
-                            return Err(err);
-                        }
-                        Ok(data) => {
-                            if let Some(status) = data.drain()? {
-                                let code = status.code();
-                                if code != 0 {
-                                    return Ok(
-                                        PipelineData::new_external_stream_with_only_exit_code(code),
-                                    );
-                                }
-                            }
-                        }
                     }
                 }
             }
@@ -121,25 +106,10 @@ impl Command for For {
                     stack.add_var(var_id, x);
 
                     match eval_block(&engine_state, stack, block, PipelineData::empty()) {
-                        Err(ShellError::Break { .. }) => {
+                        Err(ShellError::Break { .. }) => break,
-                            break;
+                        Err(ShellError::Continue { .. }) => continue,
-                        }
+                        Err(err) => return Err(err),
-                        Err(ShellError::Continue { .. }) => {
+                        Ok(data) => data.drain()?,
-                            continue;
-                        }
-                        Err(err) => {
-                            return Err(err);
-                        }
-                        Ok(data) => {
-                            if let Some(status) = data.drain()? {
-                                let code = status.code();
-                                if code != 0 {
-                                    return Ok(
-                                        PipelineData::new_external_stream_with_only_exit_code(code),
-                                    );
-                                }
-                            }
-                        }
                     }
                 }
             }

crates/nu-cmd-lang/src/core_commands/if_.rs

@@ -127,10 +127,9 @@ impl Command for If {
                     eval_block(engine_state, stack, block, input)
                 } else {
                     eval_expression_with_input(engine_state, stack, else_expr, input)
-                        .map(|res| res.0)
                 }
             } else {
-                eval_expression_with_input(engine_state, stack, else_case, input).map(|res| res.0)
+                eval_expression_with_input(engine_state, stack, else_case, input)
             }
         } else {
             Ok(PipelineData::empty())

crates/nu-cmd-lang/src/core_commands/loop_.rs

@@ -56,23 +56,10 @@ impl Command for Loop {
             engine_state.signals().check(head)?;
 
             match eval_block(engine_state, stack, block, PipelineData::empty()) {
-                Err(ShellError::Break { .. }) => {
+                Err(ShellError::Break { .. }) => break,
-                    break;
+                Err(ShellError::Continue { .. }) => continue,
-                }
+                Err(err) => return Err(err),
-                Err(ShellError::Continue { .. }) => {
+                Ok(data) => data.drain()?,
-                    continue;
-                }
-                Err(err) => {
-                    return Err(err);
-                }
-                Ok(data) => {
-                    if let Some(status) = data.drain()? {
-                        let code = status.code();
-                        if code != 0 {
-                            return Ok(PipelineData::new_external_stream_with_only_exit_code(code));
-                        }
-                    }
-                }
             }
         }
         Ok(PipelineData::empty())

crates/nu-cmd-lang/src/core_commands/match_.rs

@@ -81,7 +81,7 @@ impl Command for Match {
                         let block = engine_state.get_block(block_id);
                         eval_block(engine_state, stack, block, input)
                     } else {
-                        eval_expression_with_input(engine_state, stack, expr, input).map(|x| x.0)
+                        eval_expression_with_input(engine_state, stack, expr, input)
                     };
                 }
             } else {

crates/nu-cmd-lang/src/core_commands/try_.rs

@@ -62,29 +62,16 @@ impl Command for Try {
         let eval_block = get_eval_block(engine_state);
 
         match eval_block(engine_state, stack, try_block, input) {
-            Err(error) => {
+            Err(err) => run_catch(err, call.head, catch_block, engine_state, stack, eval_block),
-                let error = intercept_block_control(error)?;
+            Ok(PipelineData::Value(Value::Error { error, .. }, ..)) => run_catch(
-                let err_record = err_to_record(error, call.head);
+                *error,
-                handle_catch(err_record, catch_block, engine_state, stack, eval_block)
+                call.head,
-            }
+                catch_block,
-            Ok(PipelineData::Value(Value::Error { error, .. }, ..)) => {
+                engine_state,
-                let error = intercept_block_control(*error)?;
+                stack,
-                let err_record = err_to_record(error, call.head);
+                eval_block,
-                handle_catch(err_record, catch_block, engine_state, stack, eval_block)
+            ),
-            }
+            Ok(pipeline) => Ok(pipeline),
-            // external command may fail to run
-            Ok(pipeline) => {
-                let (pipeline, external_failed) = pipeline.check_external_failed()?;
-                if external_failed {
-                    let status = pipeline.drain()?;
-                    let code = status.map(|status| status.code()).unwrap_or(0);
-                    stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(code.into(), call.head));
-                    let err_value = Value::nothing(call.head);
-                    handle_catch(err_value, catch_block, engine_state, stack, eval_block)
-                } else {
-                    Ok(pipeline)
-                }
-            }
         }
     }
 
@@ -109,28 +96,32 @@ impl Command for Try {
     }
 }
 
-fn handle_catch(
+fn run_catch(
-    err_value: Value,
+    error: ShellError,
-    catch_block: Option<Closure>,
+    span: Span,
+    catch: Option<Closure>,
     engine_state: &EngineState,
     stack: &mut Stack,
-    eval_block_fn: EvalBlockFn,
+    eval_block: EvalBlockFn,
 ) -> Result<PipelineData, ShellError> {
-    if let Some(catch_block) = catch_block {
+    let error = intercept_block_control(error)?;
-        let catch_block = engine_state.get_block(catch_block.block_id);
+
+    if let Some(catch) = catch {
+        let error = err_to_record(error, span);
+        let block = engine_state.get_block(catch.block_id);
         // Put the error value in the positional closure var
-        if let Some(var) = catch_block.signature.get_positional(0) {
+        if let Some(var) = block.signature.get_positional(0) {
             if let Some(var_id) = &var.var_id {
-                stack.add_var(*var_id, err_value.clone());
+                stack.add_var(*var_id, error.clone());
             }
         }
 
-        eval_block_fn(
+        eval_block(
             engine_state,
             stack,
-            catch_block,
+            block,
             // Make the error accessible with $in, too
-            err_value.into_pipeline_data(),
+            error.into_pipeline_data(),
         )
     } else {
         Ok(PipelineData::empty())

crates/nu-cmd-lang/src/core_commands/while_.rs

@@ -73,27 +73,10 @@ impl Command for While {
                         let block = engine_state.get_block(block_id);
 
                         match eval_block(engine_state, stack, block, PipelineData::empty()) {
-                            Err(ShellError::Break { .. }) => {
+                            Err(ShellError::Break { .. }) => break,
-                                break;
+                            Err(ShellError::Continue { .. }) => continue,
-                            }
+                            Err(err) => return Err(err),
-                            Err(ShellError::Continue { .. }) => {
+                            Ok(data) => data.drain()?,
-                                continue;
-                            }
-                            Err(err) => {
-                                return Err(err);
-                            }
-                            Ok(data) => {
-                                if let Some(status) = data.drain()? {
-                                    let code = status.code();
-                                    if code != 0 {
-                                        return Ok(
-                                            PipelineData::new_external_stream_with_only_exit_code(
-                                                code,
-                                            ),
-                                        );
-                                    }
-                                }
-                            }
                         }
                     } else {
                         break;

crates/nu-command/src/debug/timeit.rs

@@ -59,7 +59,7 @@ impl Command for TimeIt {
             } else {
                 let eval_expression_with_input = get_eval_expression_with_input(engine_state);
                 let expression = &command_to_run.clone();
-                eval_expression_with_input(engine_state, stack, expression, input)?.0
+                eval_expression_with_input(engine_state, stack, expression, input)?
             }
         } else {
             PipelineData::empty()

crates/nu-engine/src/eval.rs

@@ -250,7 +250,7 @@ pub fn eval_expression_with_input<D: DebugContext>(
     stack: &mut Stack,
     expr: &Expression,
     mut input: PipelineData,
-) -> Result<(PipelineData, bool), ShellError> {
+) -> Result<PipelineData, ShellError> {
     match &expr.expr {
         Expr::Call(call) => {
             input = eval_call::<D>(engine_state, stack, call, input)?;
@@ -298,16 +298,7 @@ pub fn eval_expression_with_input<D: DebugContext>(
         }
     };
 
-    // If input an external command,
+    Ok(input)
-    // then `might_consume_external_result` will consume `stderr` if `stdout` is `None`.
-    // This should not happen if the user wants to capture stderr.
-    if !matches!(stack.stdout(), OutDest::Pipe | OutDest::Capture)
-        && matches!(stack.stderr(), OutDest::Capture)
-    {
-        Ok((input, false))
-    } else {
-        input.check_external_failed()
-    }
 }
 
 fn eval_redirection<D: DebugContext>(
@@ -401,9 +392,8 @@ fn eval_element_with_input_inner<D: DebugContext>(
     stack: &mut Stack,
     element: &PipelineElement,
     input: PipelineData,
-) -> Result<(PipelineData, bool), ShellError> {
+) -> Result<PipelineData, ShellError> {
-    let (data, failed) =
+    let data = eval_expression_with_input::<D>(engine_state, stack, &element.expr, input)?;
-        eval_expression_with_input::<D>(engine_state, stack, &element.expr, input)?;
 
     if let Some(redirection) = element.redirection.as_ref() {
         let is_external = if let PipelineData::ByteStream(stream, ..) = &data {
@@ -473,7 +463,7 @@ fn eval_element_with_input_inner<D: DebugContext>(
         PipelineData::Empty => PipelineData::Empty,
     };
 
-    Ok((data, failed))
+    Ok(data)
 }
 
 fn eval_element_with_input<D: DebugContext>(
@@ -481,20 +471,11 @@ fn eval_element_with_input<D: DebugContext>(
     stack: &mut Stack,
     element: &PipelineElement,
     input: PipelineData,
-) -> Result<(PipelineData, bool), ShellError> {
+) -> Result<PipelineData, ShellError> {
     D::enter_element(engine_state, element);
-    match eval_element_with_input_inner::<D>(engine_state, stack, element, input) {
+    let result = eval_element_with_input_inner::<D>(engine_state, stack, element, input);
-        Ok((data, failed)) => {
+    D::leave_element(engine_state, element, &result);
-            let res = Ok(data);
+    result
-            D::leave_element(engine_state, element, &res);
-            res.map(|data| (data, failed))
-        }
-        Err(err) => {
-            let res = Err(err);
-            D::leave_element(engine_state, element, &res);
-            res.map(|data| (data, false))
-        }
-    }
 }
 
 pub fn eval_block_with_early_return<D: DebugContext>(
@@ -542,14 +523,7 @@ pub fn eval_block<D: DebugContext>(
                 (next_out.or(Some(OutDest::Pipe)), next_err),
             )?;
             let stack = &mut stack.push_redirection(stdout, stderr);
-            let (output, failed) =
+            input = eval_element_with_input::<D>(engine_state, stack, element, input)?;
-                eval_element_with_input::<D>(engine_state, stack, element, input)?;
-            if failed {
-                // External command failed.
-                // Don't return `Err(ShellError)`, so nushell won't show an extra error message.
-                return Ok(output);
-            }
-            input = output;
         }
 
         if last_pipeline {
@@ -560,13 +534,7 @@ pub fn eval_block<D: DebugContext>(
                 (stack.pipe_stdout().cloned(), stack.pipe_stderr().cloned()),
             )?;
             let stack = &mut stack.push_redirection(stdout, stderr);
-            let (output, failed) = eval_element_with_input::<D>(engine_state, stack, last, input)?;
+            input = eval_element_with_input::<D>(engine_state, stack, last, input)?;
-            if failed {
-                // External command failed.
-                // Don't return `Err(ShellError)`, so nushell won't show an extra error message.
-                return Ok(output);
-            }
-            input = output;
         } else {
             let (stdout, stderr) = eval_element_redirection::<D>(
                 engine_state,
@@ -575,32 +543,20 @@ pub fn eval_block<D: DebugContext>(
                 (None, None),
             )?;
             let stack = &mut stack.push_redirection(stdout, stderr);
-            let (output, failed) = eval_element_with_input::<D>(engine_state, stack, last, input)?;
+            match eval_element_with_input::<D>(engine_state, stack, last, input)? {
-            if failed {
-                // External command failed.
-                // Don't return `Err(ShellError)`, so nushell won't show an extra error message.
-                return Ok(output);
-            }
-            input = PipelineData::Empty;
-            match output {
                 PipelineData::ByteStream(stream, ..) => {
                     let span = stream.span();
-                    let status = stream.drain()?;
+                    if let Err(err) = stream.drain() {
-                    if let Some(status) = status {
+                        stack.set_last_exit_code(&err);
-                        stack.add_env_var(
+                        return Err(err);
-                            "LAST_EXIT_CODE".into(),
+                    } else {
-                            Value::int(status.code().into(), span),
+                        stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, span));
-                        );
-                        if status.code() != 0 {
-                            break;
-                        }
                     }
                 }
-                PipelineData::ListStream(stream, ..) => {
+                PipelineData::ListStream(stream, ..) => stream.drain()?,
-                    stream.drain()?;
-                }
                 PipelineData::Value(..) | PipelineData::Empty => {}
             }
+            input = PipelineData::Empty;
         }
     }
 
@@ -639,8 +595,7 @@ pub fn eval_collect<D: DebugContext>(
         expr,
         // We still have to pass it as input
         input.into_pipeline_data_with_metadata(metadata),
-    )
+    );
-    .map(|(result, _failed)| result);
 
     stack.remove_var(var_id);
 

crates/nu-engine/src/eval_helpers.rs

@@ -25,12 +25,8 @@ pub type EvalBlockWithEarlyReturnFn =
 pub type EvalExpressionFn = fn(&EngineState, &mut Stack, &Expression) -> Result<Value, ShellError>;
 
 /// Type of eval_expression_with_input() function
-pub type EvalExpressionWithInputFn = fn(
+pub type EvalExpressionWithInputFn =
-    &EngineState,
+    fn(&EngineState, &mut Stack, &Expression, PipelineData) -> Result<PipelineData, ShellError>;
-    &mut Stack,
-    &Expression,
-    PipelineData,
-) -> Result<(PipelineData, bool), ShellError>;
 
 /// Type of eval_subexpression() function
 pub type EvalSubexpressionFn =

crates/nu-engine/src/eval_ir.rs

@@ -207,18 +207,6 @@ fn eval_ir_block_impl<D: DebugContext>(
             Ok(InstructionResult::Return(reg_id)) => {
                 return Ok(ctx.take_reg(reg_id));
             }
-            Ok(InstructionResult::ExitCode(exit_code)) => {
-                if let Some(error_handler) = ctx.stack.error_handlers.pop(ctx.error_handler_base) {
-                    // If an error handler is set, branch there
-                    prepare_error_handler(ctx, error_handler, None);
-                    pc = error_handler.handler_index;
-                } else {
-                    // If not, exit the block with the exit code
-                    return Ok(PipelineData::new_external_stream_with_only_exit_code(
-                        exit_code,
-                    ));
-                }
-            }
             Err(
                 err @ (ShellError::Return { .. }
                 | ShellError::Continue { .. }
@@ -281,7 +269,6 @@ enum InstructionResult {
     Continue,
     Branch(usize),
     Return(RegId),
-    ExitCode(i32),
 }
 
 /// Perform an instruction
@@ -790,7 +777,7 @@ fn eval_instruction<D: DebugContext>(
         }
         Instruction::CheckExternalFailed { dst, src } => {
             let data = ctx.take_reg(*src);
-            let (data, failed) = data.check_external_failed()?;
+            let (data, failed) = todo!(); // data.check_external_failed()?;
             ctx.put_reg(*src, data);
             ctx.put_reg(*dst, Value::bool(failed, *span).into_pipeline_data());
             Ok(Continue)
@@ -1365,20 +1352,21 @@ fn collect(data: PipelineData, fallback_span: Span) -> Result<PipelineData, Shel
 /// Helper for drain behavior. Returns `Ok(ExitCode)` on failed external.
 fn drain(ctx: &mut EvalContext<'_>, data: PipelineData) -> Result<InstructionResult, ShellError> {
     use self::InstructionResult::*;
-    let span = data.span().unwrap_or(Span::unknown());
+    match data {
-    if let Some(exit_status) = data.drain()? {
+        PipelineData::ByteStream(stream, ..) => {
-        ctx.stack.add_env_var(
+            let span = stream.span();
-            "LAST_EXIT_CODE".into(),
+            if let Err(err) = stream.drain() {
-            Value::int(exit_status.code() as i64, span),
+                ctx.stack.set_last_exit_code(&err);
-        );
+                return Err(err);
-        if exit_status.code() == 0 {
+            } else {
-            Ok(Continue)
+                ctx.stack
-        } else {
+                    .add_env_var("LAST_EXIT_CODE".into(), Value::int(0, span));
-            Ok(ExitCode(exit_status.code()))
+            }
         }
-    } else {
+        PipelineData::ListStream(stream, ..) => stream.drain()?,
-        Ok(Continue)
+        PipelineData::Value(..) | PipelineData::Empty => {}
     }
+    Ok(Continue)
 }
 
 enum RedirectionStream {

crates/nu-protocol/src/engine/stack.rs

@@ -279,6 +279,14 @@ impl Stack {
         }
     }
 
+    pub fn set_last_exit_code(&mut self, error: &ShellError) {
+        let code = error.exit_code_spanned();
+        self.add_env_var(
+            "LAST_EXIT_CODE".into(),
+            Value::int(code.item.into(), code.span),
+        );
+    }
+
     pub fn last_overlay_name(&self) -> Result<String, ShellError> {
         self.active_overlays
             .last()

crates/nu-protocol/src/errors/shell_error.rs

@@ -1437,14 +1437,18 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"#
 }
 
 impl ShellError {
+    pub fn exit_code_spanned(&self) -> Spanned<i32> {
+        let (item, span) = match *self {
+            Self::NonZeroExitCode { exit_code, span } => (exit_code, span),
+            Self::ProcessSignaled { signal, span, .. }
+            | Self::ProcessCoreDumped { signal, span, .. } => (-signal, span),
+            _ => (1, Span::unknown()), // TODO: better span here
+        };
+        Spanned { item, span }
+    }
+
     pub fn exit_code(&self) -> i32 {
-        match *self {
+        self.exit_code_spanned().item
-            Self::NonZeroExitCode { exit_code, .. } => exit_code,
-            Self::ProcessSignaled { signal, .. } | Self::ProcessCoreDumped { signal, .. } => {
-                -signal
-            }
-            _ => 1,
-        }
     }
 
     // TODO: Implement as From trait

crates/nu-protocol/src/pipeline/byte_stream.rs

@@ -1,6 +1,6 @@
 //! Module managing the streaming of raw bytes between pipeline elements
 use crate::{
-    process::{ChildPipe, ChildProcess, ExitStatus},
+    process::{ChildPipe, ChildProcess},
     ErrSpan, IntoSpanned, OutDest, PipelineData, ShellError, Signals, Span, Type, Value,
 };
 use serde::{Deserialize, Serialize};
@@ -551,14 +551,14 @@ impl ByteStream {
     ///
     /// If the source of the [`ByteStream`] is [`ByteStreamSource::Child`],
     /// then the [`ExitStatus`] of the [`ChildProcess`] is returned.
-    pub fn drain(self) -> Result<Option<ExitStatus>, ShellError> {
+    pub fn drain(self) -> Result<(), ShellError> {
         match self.stream {
             ByteStreamSource::Read(read) => {
                 copy_with_signals(read, io::sink(), self.span, &self.signals)?;
-                Ok(None)
+                Ok(())
             }
-            ByteStreamSource::File(_) => Ok(None),
+            ByteStreamSource::File(_) => Ok(()),
-            ByteStreamSource::Child(child) => Ok(Some(child.wait()?)),
+            ByteStreamSource::Child(child) => child.wait()?.check_ok(self.span),
         }
     }
 
@@ -566,7 +566,7 @@ impl ByteStream {
     ///
     /// If the source of the [`ByteStream`] is [`ByteStreamSource::Child`],
     /// then the [`ExitStatus`] of the [`ChildProcess`] is returned.
-    pub fn print(self, to_stderr: bool) -> Result<Option<ExitStatus>, ShellError> {
+    pub fn print(self, to_stderr: bool) -> Result<(), ShellError> {
         if to_stderr {
             self.write_to(&mut io::stderr())
         } else {
@@ -578,17 +578,15 @@ impl ByteStream {
     ///
     /// If the source of the [`ByteStream`] is [`ByteStreamSource::Child`],
     /// then the [`ExitStatus`] of the [`ChildProcess`] is returned.
-    pub fn write_to(self, dest: impl Write) -> Result<Option<ExitStatus>, ShellError> {
+    pub fn write_to(self, dest: impl Write) -> Result<(), ShellError> {
         let span = self.span;
         let signals = &self.signals;
         match self.stream {
             ByteStreamSource::Read(read) => {
                 copy_with_signals(read, dest, span, signals)?;
-                Ok(None)
             }
             ByteStreamSource::File(file) => {
                 copy_with_signals(file, dest, span, signals)?;
-                Ok(None)
             }
             ByteStreamSource::Child(mut child) => {
                 // All `OutDest`s except `OutDest::Capture` will cause `stderr` to be `None`.
@@ -606,36 +604,33 @@ impl ByteStream {
                         }
                     }
                 }
-                Ok(Some(child.wait()?))
+                child.wait()?.check_ok(span)?;
             }
         }
+        Ok(())
     }
 
     pub(crate) fn write_to_out_dests(
         self,
         stdout: &OutDest,
         stderr: &OutDest,
-    ) -> Result<Option<ExitStatus>, ShellError> {
+    ) -> Result<(), ShellError> {
         let span = self.span;
         let signals = &self.signals;
 
         match self.stream {
             ByteStreamSource::Read(read) => {
                 write_to_out_dest(read, stdout, true, span, signals)?;
-                Ok(None)
             }
-            ByteStreamSource::File(file) => {
+            ByteStreamSource::File(file) => match stdout {
-                match stdout {
+                OutDest::Pipe | OutDest::Capture | OutDest::Null => {}
-                    OutDest::Pipe | OutDest::Capture | OutDest::Null => {}
+                OutDest::Inherit => {
-                    OutDest::Inherit => {
+                    copy_with_signals(file, io::stdout(), span, signals)?;
-                        copy_with_signals(file, io::stdout(), span, signals)?;
-                    }
-                    OutDest::File(f) => {
-                        copy_with_signals(file, f.as_ref(), span, signals)?;
-                    }
                 }
-                Ok(None)
+                OutDest::File(f) => {
-            }
+                    copy_with_signals(file, f.as_ref(), span, signals)?;
+                }
+            },
             ByteStreamSource::Child(mut child) => {
                 match (child.stdout.take(), child.stderr.take()) {
                     (Some(out), Some(err)) => {
@@ -682,9 +677,11 @@ impl ByteStream {
                     }
                     (None, None) => {}
                 }
-                Ok(Some(child.wait()?))
+                child.wait()?.check_ok(span)?;
             }
         }
+
+        Ok(())
     }
 }
 

crates/nu-protocol/src/pipeline/pipeline_data.rs

@@ -1,12 +1,11 @@
 use crate::{
     ast::{Call, PathMember},
     engine::{EngineState, Stack},
-    process::{ChildPipe, ChildProcess, ExitStatus},
+    ByteStream, ByteStreamType, Config, ListStream, OutDest, PipelineMetadata, Range, ShellError,
-    ByteStream, ByteStreamType, Config, ErrSpan, ListStream, OutDest, PipelineMetadata, Range,
+    Signals, Span, Type, Value,
-    ShellError, Signals, Span, Type, Value,
 };
 use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush};
-use std::io::{Cursor, Read, Write};
+use std::io::Write;
 
 const LINE_ENDING_PATTERN: &[char] = &['\r', '\n'];
 
@@ -52,16 +51,6 @@ impl PipelineData {
         PipelineData::Empty
     }
 
-    /// create a `PipelineData::ByteStream` with proper exit_code
-    ///
-    /// It's useful to break running without raising error at user level.
-    pub fn new_external_stream_with_only_exit_code(exit_code: i32) -> PipelineData {
-        let span = Span::unknown();
-        let mut child = ChildProcess::from_raw(None, None, None, span);
-        child.set_exit_code(exit_code);
-        PipelineData::ByteStream(ByteStream::child(child, span), None)
-    }
-
     pub fn metadata(&self) -> Option<PipelineMetadata> {
         match self {
             PipelineData::Empty => None,
@@ -157,11 +146,7 @@ impl PipelineData {
     ) -> Result<PipelineData, ShellError> {
         match (self, stack.stdout()) {
             (PipelineData::ByteStream(stream, ..), stdout) => {
-                if let Some(exit_status) = stream.write_to_out_dests(stdout, stack.stderr())? {
+                stream.write_to_out_dests(stdout, stack.stderr())?;
-                    return Ok(PipelineData::new_external_stream_with_only_exit_code(
-                        exit_status.code(),
-                    ));
-                }
             }
             (data, OutDest::Pipe | OutDest::Capture) => return Ok(data),
             (PipelineData::Empty, ..) => {}
@@ -193,15 +178,12 @@ impl PipelineData {
         Ok(PipelineData::Empty)
     }
 
-    pub fn drain(self) -> Result<Option<ExitStatus>, ShellError> {
+    pub fn drain(self) -> Result<(), ShellError> {
         match self {
-            PipelineData::Empty => Ok(None),
+            PipelineData::Empty => Ok(()),
             PipelineData::Value(Value::Error { error, .. }, ..) => Err(*error),
-            PipelineData::Value(..) => Ok(None),
+            PipelineData::Value(..) => Ok(()),
-            PipelineData::ListStream(stream, ..) => {
+            PipelineData::ListStream(stream, ..) => stream.drain(),
-                stream.drain()?;
-                Ok(None)
-            }
             PipelineData::ByteStream(stream, ..) => stream.drain(),
         }
     }
@@ -462,68 +444,6 @@ impl PipelineData {
         }
     }
 
-    /// Try to catch the external command exit status and detect if it failed.
-    ///
-    /// This is useful for external commands with semicolon, we can detect errors early to avoid
-    /// commands after the semicolon running.
-    ///
-    /// Returns `self` and a flag that indicates if the external command run failed. If `self` is
-    /// not [`PipelineData::ByteStream`], the flag will be `false`.
-    ///
-    /// Currently this will consume an external command to completion.
-    pub fn check_external_failed(self) -> Result<(Self, bool), ShellError> {
-        if let PipelineData::ByteStream(stream, metadata) = self {
-            // Preserve stream attributes
-            let span = stream.span();
-            let type_ = stream.type_();
-            let known_size = stream.known_size();
-            match stream.into_child() {
-                Ok(mut child) => {
-                    // Only check children without stdout. This means that nothing
-                    // later in the pipeline can possibly consume output from this external command.
-                    if child.stdout.is_none() {
-                        // Note:
-                        // In run-external's implementation detail, the result sender thread
-                        // send out stderr message first, then stdout message, then exit_code.
-                        //
-                        // In this clause, we already make sure that `stdout` is None
-                        // But not the case of `stderr`, so if `stderr` is not None
-                        // We need to consume stderr message before reading external commands' exit code.
-                        //
-                        // Or we'll never have a chance to read exit_code if stderr producer produce too much stderr message.
-                        // So we consume stderr stream and rebuild it.
-                        let stderr = child
-                            .stderr
-                            .take()
-                            .map(|mut stderr| {
-                                let mut buf = Vec::new();
-                                stderr.read_to_end(&mut buf).err_span(span)?;
-                                Ok::<_, ShellError>(buf)
-                            })
-                            .transpose()?;
-
-                        let code = child.wait()?.code();
-                        let mut child = ChildProcess::from_raw(None, None, None, span);
-                        if let Some(stderr) = stderr {
-                            child.stderr = Some(ChildPipe::Tee(Box::new(Cursor::new(stderr))));
-                        }
-                        child.set_exit_code(code);
-                        let stream = ByteStream::child(child, span).with_type(type_);
-                        Ok((PipelineData::ByteStream(stream, metadata), code != 0))
-                    } else {
-                        let stream = ByteStream::child(child, span)
-                            .with_type(type_)
-                            .with_known_size(known_size);
-                        Ok((PipelineData::ByteStream(stream, metadata), false))
-                    }
-                }
-                Err(stream) => Ok((PipelineData::ByteStream(stream, metadata), false)),
-            }
-        } else {
-            Ok((self, false))
-        }
-    }
-
     /// Try to convert Value from Value::Range to Value::List.
     /// This is useful to expand Value::Range into array notation, specifically when
     /// converting `to json` or `to nuon`.
@@ -579,7 +499,7 @@ impl PipelineData {
         stack: &mut Stack,
         no_newline: bool,
         to_stderr: bool,
-    ) -> Result<Option<ExitStatus>, ShellError> {
+    ) -> Result<(), ShellError> {
         match self {
             // Print byte streams directly as long as they aren't binary.
             PipelineData::ByteStream(stream, ..) if stream.type_() != ByteStreamType::Binary => {
@@ -609,7 +529,7 @@ impl PipelineData {
         engine_state: &EngineState,
         no_newline: bool,
         to_stderr: bool,
-    ) -> Result<Option<ExitStatus>, ShellError> {
+    ) -> Result<(), ShellError> {
         if let PipelineData::ByteStream(stream, ..) = self {
             // Copy ByteStreams directly
             stream.print(to_stderr)
@@ -633,7 +553,7 @@ impl PipelineData {
                 }
             }
 
-            Ok(None)
+            Ok(())
         }
     }
 

crates/nu-protocol/src/process/child.rs

@@ -187,8 +187,7 @@ impl ChildProcess {
             Vec::new()
         };
 
-        // TODO: check exit_status
+        self.exit_status.wait(self.span)?.check_ok(self.span)?;
-        self.exit_status.wait(self.span)?;
 
         Ok(bytes)
     }

crates/nu-protocol/src/process/exit_status.rs

@@ -1,3 +1,4 @@
+use crate::{ShellError, Span};
 use std::process;
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -18,6 +19,39 @@ impl ExitStatus {
             ExitStatus::Signaled { signal, .. } => -signal,
         }
     }
+
+    pub fn check_ok(self, span: Span) -> Result<(), ShellError> {
+        match self {
+            ExitStatus::Exited(0) => Ok(()),
+            ExitStatus::Exited(exit_code) => Err(ShellError::NonZeroExitCode { exit_code, span }),
+            #[cfg(unix)]
+            ExitStatus::Signaled {
+                signal,
+                core_dumped,
+            } => {
+                use nix::sys::signal::Signal;
+
+                let signal_name = Signal::try_from(signal)
+                    .map(Signal::as_str)
+                    .unwrap_or("unknown signal")
+                    .into();
+
+                Err(if core_dumped {
+                    ShellError::ProcessCoreDumped {
+                        signal_name,
+                        signal,
+                        span,
+                    }
+                } else {
+                    ShellError::ProcessSignaled {
+                        signal_name,
+                        signal,
+                        span,
+                    }
+                })
+            }
+        }
+    }
 }
 
 #[cfg(unix)]

src/run.rs

@@ -85,7 +85,7 @@ pub(crate) fn run_commands(
     engine_state.generate_nu_constant();
 
     let start_time = std::time::Instant::now();
-    if let Err(err) = evaluate_commands(
+    let result = evaluate_commands(
         commands,
         engine_state,
         &mut stack,
@@ -95,11 +95,13 @@ pub(crate) fn run_commands(
             error_style: parsed_nu_cli_args.error_style,
             no_newline: parsed_nu_cli_args.no_newline.is_some(),
         },
-    ) {
+    );
-        report_error_new(engine_state, &err);
-        std::process::exit(1);
-    }
     perf!("evaluate_commands", start_time, use_color);
+
+    if let Err(err) = result {
+        report_error_new(engine_state, &err);
+        std::process::exit(err.exit_code());
+    }
 }
 
 pub(crate) fn run_file(
@@ -158,29 +160,19 @@ pub(crate) fn run_file(
     engine_state.generate_nu_constant();
 
     let start_time = std::time::Instant::now();
-    if let Err(err) = evaluate_file(
+    let result = evaluate_file(
         script_name,
         &args_to_script,
         engine_state,
         &mut stack,
         input,
-    ) {
+    );
-        report_error_new(engine_state, &err);
-        std::process::exit(1);
-    }
     perf!("evaluate_file", start_time, use_color);
 
-    let start_time = std::time::Instant::now();
+    if let Err(err) = result {
-    let last_exit_code = stack.get_env_var(&*engine_state, "LAST_EXIT_CODE");
+        report_error_new(engine_state, &err);
-    if let Some(last_exit_code) = last_exit_code {
+        std::process::exit(err.exit_code());
-        let value = last_exit_code.as_int();
-        if let Ok(value) = value {
-            if value != 0 {
-                std::process::exit(value as i32);
-            }
-        }
     }
-    perf!("get exit code", start_time, use_color);
 }
 
 pub(crate) fn run_repl(