Merge 494d65d95e into 802bfed173

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/repl.rs

@@ -97,7 +97,7 @@ pub fn evaluate_repl(
         Value::string("0823", Span::unknown()),
     );
 
-    unique_stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, Span::unknown()));
+    unique_stack.set_last_exit_code(0, Span::unknown());
 
     let mut line_editor = get_line_editor(engine_state, nushell_path, use_color)?;
     let temp_file = temp_dir().join(format!("{}.nu", uuid::Uuid::new_v4()));
@@ -837,7 +837,7 @@ fn do_auto_cd(
         "NUSHELL_LAST_SHELL".into(),
         Value::int(last_shell as i64, span),
     );
-    stack.add_env_var("LAST_EXIT_CODE".into(), Value::int(0, Span::unknown()));
+    stack.set_last_exit_code(0, Span::unknown());
 }
 
 ///

crates/nu-cli/src/util.rs

@@ -210,18 +210,19 @@ 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 = failed.into();
+            stack.set_last_exit_code(code, Span::unknown());
+            code
+        }
         Err(err) => {
             report_error_new(engine_state, &err);
-            1
+            let code = err.exit_code();
+            stack.set_last_error(&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 +245,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 +260,7 @@ 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() {
@@ -278,10 +279,9 @@ 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 {
+    } else if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
-        if let Some(hook) = engine_state.get_config().hooks.display_output.clone() {
         let pipeline = eval_hook(
             engine_state,
             stack,
@@ -293,10 +293,9 @@ fn evaluate_source(
         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()?;
-                            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

@@ -47,6 +47,7 @@ impl Command for Try {
         call: &Call,
         input: PipelineData,
     ) -> Result<PipelineData, ShellError> {
+        let head = call.head;
         // This is compiled specially by the IR compiler. The code here is never used when
         // running in IR mode.
         let call = call.assert_ast_call()?;
@@ -61,30 +62,15 @@ impl Command for Try {
         let try_block = engine_state.get_block(try_block);
         let eval_block = get_eval_block(engine_state);
 
-        match eval_block(engine_state, stack, try_block, input) {
+        let result = eval_block(engine_state, stack, try_block, input)
-            Err(error) => {
+            .and_then(|pipeline| pipeline.write_to_out_dests(engine_state, stack));
-                let error = intercept_block_control(error)?;
+
-                let err_record = err_to_record(error, call.head);
+        match result {
-                handle_catch(err_record, catch_block, engine_state, stack, eval_block)
+            Err(err) => run_catch(err, head, catch_block, engine_state, stack, eval_block),
-            }
             Ok(PipelineData::Value(Value::Error { error, .. }, ..)) => {
-                let error = intercept_block_control(*error)?;
+                run_catch(*error, head, catch_block, engine_state, stack, eval_block)
-                let err_record = err_to_record(error, call.head);
-                handle_catch(err_record, catch_block, engine_state, stack, eval_block)
-            }
-            // 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)
-                }
             }
+            Ok(pipeline) => Ok(pipeline),
         }
     }
 
@@ -109,28 +95,33 @@ 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 {
+        stack.set_last_error(&error);
+        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())
@@ -143,23 +134,28 @@ fn handle_catch(
 /// `Err` when flow control to bubble up with `?`
 fn intercept_block_control(error: ShellError) -> Result<ShellError, ShellError> {
     match error {
-        nu_protocol::ShellError::Break { .. } => Err(error),
+        ShellError::Break { .. } => Err(error),
-        nu_protocol::ShellError::Continue { .. } => Err(error),
+        ShellError::Continue { .. } => Err(error),
-        nu_protocol::ShellError::Return { .. } => Err(error),
+        ShellError::Return { .. } => Err(error),
         _ => Ok(error),
     }
 }
 
 /// Convert from `error` to [`Value::Record`] so the error information can be easily accessed in catch.
 fn err_to_record(error: ShellError, head: Span) -> Value {
-    Value::record(
+    let exit_code = error.external_exit_code();
-        record! {
+
+    let mut record = record! {
         "msg" => Value::string(error.to_string(), head),
         "debug" => Value::string(format!("{error:?}"), head),
         "raw" => Value::error(error, head),
-        },
+    };
-        head,
+
-    )
+    if let Some(code) = exit_code {
+        record.push("exit_code", Value::int(code.item.into(), code.span));
+    }
+
+    Value::record(record, head)
 }
 
 #[cfg(test)]

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-command/tests/commands/do_.rs

@@ -12,21 +12,21 @@ fn capture_errors_works() {
 #[test]
 fn capture_errors_works_for_external() {
     let actual = nu!("do -c {nu --testbin fail}");
-    assert!(actual.err.contains("External command failed"));
+    assert!(actual.err.contains("exited with code"));
     assert_eq!(actual.out, "");
 }
 
 #[test]
 fn capture_errors_works_for_external_with_pipeline() {
     let actual = nu!("do -c {nu --testbin fail} | echo `text`");
-    assert!(actual.err.contains("External command failed"));
+    assert!(actual.err.contains("exited with code"));
     assert_eq!(actual.out, "");
 }
 
 #[test]
 fn capture_errors_works_for_external_with_semicolon() {
     let actual = nu!(r#"do -c {nu --testbin fail}; echo `text`"#);
-    assert!(actual.err.contains("External command failed"));
+    assert!(actual.err.contains("exited with code"));
     assert_eq!(actual.out, "");
 }
 

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>(
@@ -509,7 +490,7 @@ pub fn eval_block_with_early_return<D: DebugContext>(
     }
 }
 
-pub fn eval_block<D: DebugContext>(
+fn eval_block_inner<D: DebugContext>(
     engine_state: &EngineState,
     stack: &mut Stack,
     block: &Block,
@@ -520,8 +501,6 @@ pub fn eval_block<D: DebugContext>(
         return eval_ir_block::<D>(engine_state, stack, block, input);
     }
 
-    D::enter_block(engine_state, block);
-
     let num_pipelines = block.len();
 
     for (pipeline_idx, pipeline) in block.pipelines.iter().enumerate() {
@@ -542,14 +521,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 +532,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,40 +541,41 @@ 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_error(&err);
-                        stack.add_env_var(
+                        return Err(err);
-                            "LAST_EXIT_CODE".into(),
+                    } else {
-                            Value::int(status.code().into(), span),
+                        stack.set_last_exit_code(0, span);
-                        );
-                        if status.code() != 0 {
-                            break;
                     }
                 }
-                }
+                PipelineData::ListStream(stream, ..) => stream.drain()?,
-                PipelineData::ListStream(stream, ..) => {
-                    stream.drain()?;
-                }
                 PipelineData::Value(..) | PipelineData::Empty => {}
             }
+            input = PipelineData::Empty;
         }
     }
 
-    D::leave_block(engine_state, block);
-
     Ok(input)
 }
 
+pub fn eval_block<D: DebugContext>(
+    engine_state: &EngineState,
+    stack: &mut Stack,
+    block: &Block,
+    input: PipelineData,
+) -> Result<PipelineData, ShellError> {
+    D::enter_block(engine_state, block);
+    let result = eval_block_inner::<D>(engine_state, stack, block, input);
+    D::leave_block(engine_state, block);
+    if let Err(err) = &result {
+        stack.set_last_error(err);
+    }
+    result
+}
+
 pub fn eval_collect<D: DebugContext>(
     engine_state: &EngineState,
     stack: &mut Stack,
@@ -639,8 +606,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,21 +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_error(&err);
-        );
+                return Err(err);
-        if exit_status.code() == 0 {
-            Ok(Continue)
             } else {
-            Ok(ExitCode(exit_status.code()))
+                ctx.stack.set_last_exit_code(0, span);
+            }
+        }
+        PipelineData::ListStream(stream, ..) => stream.drain()?,
+        PipelineData::Value(..) | PipelineData::Empty => {}
     }
-    } else {
     Ok(Continue)
 }
-}
 
 enum RedirectionStream {
     Out,

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

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

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

@@ -1,6 +1,6 @@
 use miette::Diagnostic;
 use serde::{Deserialize, Serialize};
-use std::io;
+use std::{io, num::NonZeroI32};
 use thiserror::Error;
 
 use crate::{
@@ -639,6 +639,49 @@ pub enum ShellError {
         span: Span,
     },
 
+    /// An external command exited with a non-zero exit code.
+    ///
+    /// ## Resolution
+    ///
+    /// Check the external command's error message.
+    #[error("External command had a non-zero exit code")]
+    #[diagnostic(code(nu::shell::non_zero_exit_code))]
+    NonZeroExitCode {
+        exit_code: NonZeroI32,
+        #[label("exited with code {exit_code}")]
+        span: Span,
+    },
+
+    #[cfg(unix)]
+    /// An external command exited due to a signal.
+    ///
+    /// ## Resolution
+    ///
+    /// Check why the signal was sent or triggered.
+    #[error("External command was terminated by a signal")]
+    #[diagnostic(code(nu::shell::terminated_by_signal))]
+    TerminatedBySignal {
+        signal_name: String,
+        signal: i32,
+        #[label("terminated by {signal_name} ({signal})")]
+        span: Span,
+    },
+
+    #[cfg(unix)]
+    /// An external command core dumped.
+    ///
+    /// ## Resolution
+    ///
+    /// Check why the core dumped was triggered.
+    #[error("External command core dumped")]
+    #[diagnostic(code(nu::shell::core_dumped))]
+    CoreDumped {
+        signal_name: String,
+        signal: i32,
+        #[label("core dumped with {signal_name} ({signal})")]
+        span: Span,
+    },
+
     /// An operation was attempted with an input unsupported for some reason.
     ///
     /// ## Resolution
@@ -876,21 +919,6 @@ pub enum ShellError {
         span: Span,
     },
 
-    #[cfg(unix)]
-    /// An I/O operation failed.
-    ///
-    /// ## Resolution
-    ///
-    /// This is a generic error. Refer to the specific error message for further details.
-    #[error("program coredump error")]
-    #[diagnostic(code(nu::shell::coredump_error))]
-    CoredumpErrorSpanned {
-        msg: String,
-        signal: i32,
-        #[label("{msg}")]
-        span: Span,
-    },
-
     /// Tried to `cd` to a path that isn't a directory.
     ///
     /// ## Resolution
@@ -1395,8 +1423,22 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"#
     },
 }
 
-// TODO: Implement as From trait
 impl ShellError {
+    pub fn external_exit_code(&self) -> Option<Spanned<i32>> {
+        let (item, span) = match *self {
+            Self::NonZeroExitCode { exit_code, span } => (exit_code.into(), span),
+            Self::TerminatedBySignal { signal, span, .. }
+            | Self::CoreDumped { signal, span, .. } => (-signal, span),
+            _ => return None,
+        };
+        Some(Spanned { item, span })
+    }
+
+    pub fn exit_code(&self) -> i32 {
+        self.external_exit_code().map(|e| e.item).unwrap_or(1)
+    }
+
+    // TODO: Implement as From trait
     pub fn wrap(self, working_set: &StateWorkingSet, span: Span) -> ParseError {
         let msg = format_error(working_set, &self);
         ParseError::LabeledError(

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(),
         }
     }
 
@@ -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,26 +604,25 @@ impl ByteStream {
                         }
                     }
                 }
-                Ok(Some(child.wait()?))
+                child.wait()?;
             }
         }
+        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::Inherit => {
                     copy_with_signals(file, io::stdout(), span, signals)?;
@@ -633,9 +630,7 @@ impl ByteStream {
                 OutDest::File(f) => {
                     copy_with_signals(file, f.as_ref(), span, signals)?;
                 }
-                }
+            },
-                Ok(None)
-            }
             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()?;
             }
         }
+
+        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,
@@ -156,14 +145,10 @@ impl PipelineData {
         stack: &mut Stack,
     ) -> Result<PipelineData, ShellError> {
         match (self, stack.stdout()) {
-            (PipelineData::ByteStream(stream, ..), stdout) => {
-                if let Some(exit_status) = 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::ByteStream(stream, ..), stdout) => {
+                stream.write_to_out_dests(stdout, stack.stderr())?;
+            }
             (PipelineData::Empty, ..) => {}
             (PipelineData::Value(..), OutDest::Null) => {}
             (PipelineData::ListStream(stream, ..), OutDest::Null) => {
@@ -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

@@ -23,21 +23,16 @@ impl ExitStatusFuture {
             ExitStatusFuture::Finished(Err(err)) => Err(err.as_ref().clone()),
             ExitStatusFuture::Running(receiver) => {
                 let code = match receiver.recv() {
-                    Ok(Ok(status)) => {
                     #[cfg(unix)]
-                        if let ExitStatus::Signaled {
+                    Ok(Ok(
-                            signal,
+                        status @ ExitStatus::Signaled {
-                            core_dumped: true,
+                            core_dumped: true, ..
-                        } = status
+                        },
-                        {
+                    )) => {
-                            return Err(ShellError::CoredumpErrorSpanned {
+                        status.check_ok(span)?;
-                                msg: format!("coredump detected. received signal: {signal}"),
-                                signal,
-                                span,
-                            });
-                        }
                         Ok(status)
                     }
+                    Ok(Ok(status)) => Ok(status),
                     Ok(Err(err)) => Err(ShellError::IOErrorSpanned {
                         msg: format!("failed to get exit code: {err:?}"),
                         span,
@@ -187,13 +182,12 @@ 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)
     }
 
-    pub fn wait(mut self) -> Result<ExitStatus, ShellError> {
+    pub fn wait(mut self) -> Result<(), ShellError> {
         if let Some(stdout) = self.stdout.take() {
             let stderr = self
                 .stderr
@@ -229,7 +223,7 @@ impl ChildProcess {
             consume_pipe(stderr).err_span(self.span)?;
         }
 
-        self.exit_status.wait(self.span)
+        self.exit_status.wait(self.span)?.check_ok(self.span)
     }
 
     pub fn try_wait(&mut self) -> Result<Option<ExitStatus>, ShellError> {

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,47 @@ impl ExitStatus {
             ExitStatus::Signaled { signal, .. } => -signal,
         }
     }
+
+    pub fn check_ok(self, span: Span) -> Result<(), ShellError> {
+        match self {
+            ExitStatus::Exited(exit_code) => {
+                if let Ok(exit_code) = exit_code.try_into() {
+                    Err(ShellError::NonZeroExitCode { exit_code, span })
+                } else {
+                    Ok(())
+                }
+            }
+            #[cfg(unix)]
+            ExitStatus::Signaled {
+                signal,
+                core_dumped,
+            } => {
+                use nix::sys::signal::Signal;
+
+                let sig = Signal::try_from(signal);
+
+                if sig == Ok(Signal::SIGPIPE) {
+                    // Processes often exit with SIGPIPE, but this is not an error condition.
+                    Ok(())
+                } else {
+                    let signal_name = sig.map(Signal::as_str).unwrap_or("unknown signal").into();
+                    Err(if core_dumped {
+                        ShellError::CoreDumped {
+                            signal_name,
+                            signal,
+                            span,
+                        }
+                    } else {
+                        ShellError::TerminatedBySignal {
+                            signal_name,
+                            signal,
+                            span,
+                        }
+                    })
+                }
+            }
+        }
+    }
 }
 
 #[cfg(unix)]

crates/nu-protocol/tests/test_config.rs

@@ -52,7 +52,7 @@ fn filesize_format_auto_metric_false() {
 
 #[test]
 fn fancy_default_errors() {
-    let actual = nu!(nu_repl_code(&[
+    let code = nu_repl_code(&[
         r#"def force_error [x] {
         error make {
             msg: "oh no!"
@@ -62,8 +62,10 @@ fn fancy_default_errors() {
             }
         }
     }"#,
-        r#"force_error "My error""#
+        r#"force_error "My error""#,
-    ]));
+    ]);
+
+    let actual = nu!(format!("try {{ {code} }}"));
 
     assert_eq!(
         actual.err,
@@ -73,7 +75,7 @@ fn fancy_default_errors() {
 
 #[test]
 fn narratable_errors() {
-    let actual = nu!(nu_repl_code(&[
+    let code = nu_repl_code(&[
         r#"$env.config = { error_style: "plain" }"#,
         r#"def force_error [x] {
         error make {
@@ -85,7 +87,9 @@ fn narratable_errors() {
         }
     }"#,
         r#"force_error "my error""#,
-    ]));
+    ]);
+
+    let actual = nu!(format!("try {{ {code} }}"));
 
     assert_eq!(
         actual.err,

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,30 +160,20 @@ 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(
     engine_state: &mut EngineState,

tests/shell/pipeline/commands/external.rs

@@ -173,13 +173,6 @@ fn basic_outerr_pipe_works(#[case] redirection: &str) {
     assert_eq!(actual.out, "7");
 }
 
-#[test]
-fn err_pipe_with_failed_external_works() {
-    let actual =
-        nu!(r#"with-env { FOO: "bar" } { nu --testbin echo_env_stderr_fail FOO e>| str length }"#);
-    assert_eq!(actual.out, "3");
-}
-
 #[test]
 fn dont_run_glob_if_pass_variable_to_external() {
     Playground::setup("dont_run_glob", |dirs, sandbox| {