From 530ff3893e5d470d7231a9960cd5e49f1b143b20 Mon Sep 17 00:00:00 2001
From: WindSoilder <WindSoilder@outlook.com>
Date: Fri, 30 Sep 2022 20:14:02 +0800
Subject: [PATCH] Eval external command result immediately when using `do`
 command with `-c` (#6645)

* make capture error works better in do command

* remove into string test because we have no way to generate Value::Error for now
---
 crates/nu-command/src/core_commands/do_.rs    | 73 ++++++++++++++++++-
 crates/nu-command/tests/commands/do_.rs       | 40 +++++++++-
 .../tests/commands/str_/into_string.rs        | 12 ---
 3 files changed, 108 insertions(+), 17 deletions(-)

diff --git a/crates/nu-command/src/core_commands/do_.rs b/crates/nu-command/src/core_commands/do_.rs
index 13ecb8dbf..80f633425 100644
--- a/crates/nu-command/src/core_commands/do_.rs
+++ b/crates/nu-command/src/core_commands/do_.rs
@@ -2,7 +2,8 @@ use nu_engine::{eval_block, CallExt};
 use nu_protocol::ast::Call;
 use nu_protocol::engine::{CaptureBlock, Command, EngineState, Stack};
 use nu_protocol::{
-    Category, Example, IntoPipelineData, PipelineData, Signature, SyntaxShape, Value,
+    Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, SyntaxShape,
+    Value,
 };
 
 #[derive(Clone)]
@@ -102,9 +103,75 @@ impl Command for Do {
                 Err(_) => Ok(PipelineData::new(call.head)),
             }
         } else if capture_errors {
+            // collect stdout and stderr and check exit code.
+            // if exit code is not 0, return back ShellError.
             match result {
-                Ok(x) => Ok(x),
-                Err(err) => Ok((Value::Error { error: err }).into_pipeline_data()),
+                Ok(PipelineData::ExternalStream {
+                    stdout,
+                    stderr,
+                    exit_code,
+                    span,
+                    metadata,
+                }) => {
+                    // collect all output first.
+                    let mut stderr_ctrlc = None;
+                    let stderr_msg = match stderr {
+                        None => "".to_string(),
+                        Some(stderr_stream) => {
+                            stderr_ctrlc = stderr_stream.ctrlc.clone();
+                            stderr_stream.into_string().map(|s| s.item)?
+                        }
+                    };
+
+                    let mut stdout_ctrlc = None;
+                    let stdout_msg = match stdout {
+                        None => "".to_string(),
+                        Some(stdout_stream) => {
+                            stdout_ctrlc = stdout_stream.ctrlc.clone();
+                            stdout_stream.into_string().map(|s| s.item)?
+                        }
+                    };
+
+                    let mut exit_code_ctrlc = None;
+                    let exit_code: Vec<Value> = match exit_code {
+                        None => vec![],
+                        Some(exit_code_stream) => {
+                            exit_code_ctrlc = exit_code_stream.ctrlc.clone();
+                            exit_code_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 {
+                            return Err(ShellError::ExternalCommand(
+                                "External command runs to failed".to_string(),
+                                stderr_msg,
+                                span,
+                            ));
+                        }
+                    }
+                    // construct pipeline data to our caller
+                    Ok(PipelineData::ExternalStream {
+                        stdout: Some(RawStream::new(
+                            Box::new(vec![Ok(stdout_msg.into_bytes())].into_iter()),
+                            stdout_ctrlc,
+                            span,
+                        )),
+                        stderr: Some(RawStream::new(
+                            Box::new(vec![Ok(stderr_msg.into_bytes())].into_iter()),
+                            stderr_ctrlc,
+                            span,
+                        )),
+                        exit_code: Some(ListStream::from_stream(
+                            exit_code.into_iter(),
+                            exit_code_ctrlc,
+                        )),
+                        span,
+                        metadata,
+                    })
+                }
+                Ok(other) => Ok(other),
+                Err(e) => Err(e),
             }
         } else {
             result
diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs
index 27b4394b2..eb7f7ad15 100644
--- a/crates/nu-command/tests/commands/do_.rs
+++ b/crates/nu-command/tests/commands/do_.rs
@@ -5,11 +5,47 @@ fn capture_errors_works() {
     let actual = nu!(
         cwd: ".", pipeline(
         r#"
-        do -c {$env.use} | describe
+        do -c {$env.use}
         "#
     ));
 
-    assert_eq!(actual.out, "error");
+    assert!(actual.err.contains("column_not_found"));
+}
+
+#[test]
+fn capture_errors_works_for_external() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        do -c {nu --testbin fail}
+        "#
+    ));
+    assert!(actual.err.contains("External command runs to failed"));
+    assert_eq!(actual.out, "");
+}
+
+#[test]
+fn capture_errors_works_for_external_with_pipeline() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        do -c {nu --testbin fail} | echo `text`
+        "#
+    ));
+    assert!(actual.err.contains("External command runs to failed"));
+    assert_eq!(actual.out, "");
+}
+
+#[test]
+fn capture_errors_works_for_external_with_semicolon() {
+    let actual = nu!(
+        cwd: ".", pipeline(
+        r#"
+        do -c {nu --testbin fail}; echo `text`
+        "#
+    ));
+    assert!(actual.err.contains("External command runs to failed"));
+    assert_eq!(actual.out, "");
 }
 
 #[test]
diff --git a/crates/nu-command/tests/commands/str_/into_string.rs b/crates/nu-command/tests/commands/str_/into_string.rs
index a51b0af94..e77988057 100644
--- a/crates/nu-command/tests/commands/str_/into_string.rs
+++ b/crates/nu-command/tests/commands/str_/into_string.rs
@@ -172,18 +172,6 @@ fn from_nothing() {
     assert_eq!(actual.out, "");
 }
 
-#[test]
-fn from_error() {
-    let actual = nu!(
-        cwd: ".", pipeline(
-        r#"
-        do -c {$env.use} | into string
-        "#
-    ));
-
-    assert_eq!(actual.out, "nu::shell::column_not_found");
-}
-
 #[test]
 fn int_into_string() {
     let actual = nu!(