From f71a45235a14e426410c079cd25347539731de41 Mon Sep 17 00:00:00 2001
From: WindSoilder <WindSoilder@outlook.com>
Date: Fri, 2 Dec 2022 00:58:32 +0800
Subject: [PATCH] fix try for external command runs to failed (#7300)

# Description

Fixes: #7298

So `try .. catch` works better on external command failed.

# User-Facing Changes

```
try {nu --testbin fail} catch {print "fail"}
```

After this pr, it will output "fail"

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- `cargo test --workspace` to check that all tests pass

# After Submitting

If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.

Co-authored-by: JT <547158+jntrnr@users.noreply.github.com>
---
 crates/nu-command/src/core_commands/try_.rs | 61 ++++++++++++++++++++-
 crates/nu-command/tests/commands/try_.rs    | 12 ++++
 2 files changed, 72 insertions(+), 1 deletion(-)

diff --git a/crates/nu-command/src/core_commands/try_.rs b/crates/nu-command/src/core_commands/try_.rs
index 22e2ef63f4..31aae08a23 100644
--- a/crates/nu-command/src/core_commands/try_.rs
+++ b/crates/nu-command/src/core_commands/try_.rs
@@ -1,7 +1,9 @@
 use nu_engine::{eval_block, CallExt};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{Block, Closure, Command, EngineState, Stack};
-use nu_protocol::{Category, Example, PipelineData, Signature, SyntaxShape, Type, Value};
+use nu_protocol::{
+    Category, Example, ListStream, PipelineData, Signature, SyntaxShape, Type, Value,
+};
 
 #[derive(Clone)]
 pub struct Try;
@@ -77,6 +79,63 @@ impl Command for Try {
                     Ok(PipelineData::new(call.head))
                 }
             }
+            // external command may fail to run
+            Ok(PipelineData::ExternalStream {
+                stdout: None,
+                stderr,
+                mut exit_code,
+                span,
+                metadata,
+                trim_end_newline,
+            }) => {
+                let exit_code = exit_code.take();
+                let mut failed_to_run = false;
+                let mut exit_code_stream = None;
+                if let Some(stream) = exit_code {
+                    let ctrlc = stream.ctrlc.clone();
+                    let exit_code: Vec<Value> = stream.into_iter().collect();
+                    if let Some(Value::Int { val: code, .. }) = exit_code.last() {
+                        // if exit_code is not 0, it indicates error occured, return back Err.
+                        if *code != 0 {
+                            failed_to_run = true;
+                        }
+                    }
+                    exit_code_stream = Some(ListStream::from_stream(exit_code.into_iter(), ctrlc));
+                }
+
+                if failed_to_run {
+                    if let Some(catch_block) = catch_block {
+                        let catch_block = engine_state.get_block(catch_block.block_id);
+
+                        if let Some(var) = catch_block.signature.get_positional(0) {
+                            if let Some(var_id) = &var.var_id {
+                                let err_value = Value::nothing(call.head);
+                                stack.add_var(*var_id, err_value);
+                            }
+                        }
+
+                        eval_block(
+                            engine_state,
+                            stack,
+                            catch_block,
+                            PipelineData::new(call.head),
+                            false,
+                            false,
+                        )
+                    } else {
+                        Ok(PipelineData::new(call.head))
+                    }
+                } else {
+                    Ok(PipelineData::ExternalStream {
+                        stdout: None,
+                        stderr,
+                        exit_code: exit_code_stream,
+                        span,
+                        metadata,
+                        trim_end_newline,
+                    })
+                }
+            }
             Ok(output) => Ok(output),
         }
     }
diff --git a/crates/nu-command/tests/commands/try_.rs b/crates/nu-command/tests/commands/try_.rs
index 514e2e7a9c..254bc9a2e3 100644
--- a/crates/nu-command/tests/commands/try_.rs
+++ b/crates/nu-command/tests/commands/try_.rs
@@ -36,3 +36,15 @@ fn catch_can_access_error() {
         assert!(output.err.contains("External command failed"));
     })
 }
+
+#[test]
+fn external_failed_should_be_catched() {
+    Playground::setup("try_catch_test", |dirs, _sandbox| {
+        let output = nu!(
+            cwd: dirs.test(),
+            "try { nu --testbin fail; echo 'success' } catch { echo 'fail' }"
+        );
+
+        assert!(output.out.contains("fail"));
+    })
+}