diff --git a/crates/nu-command/src/core_commands/do_.rs b/crates/nu-command/src/core_commands/do_.rs index 2aaadf0df2..6958ea09d6 100644 --- a/crates/nu-command/src/core_commands/do_.rs +++ b/crates/nu-command/src/core_commands/do_.rs @@ -1,8 +1,11 @@ +use std::thread; + use nu_engine::{eval_block_with_early_return, CallExt}; use nu_protocol::ast::Call; use nu_protocol::engine::{Closure, Command, EngineState, Stack}; use nu_protocol::{ - Category, Example, ListStream, PipelineData, ShellError, Signature, SyntaxShape, Type, Value, + Category, Example, ListStream, PipelineData, RawStream, ShellError, Signature, SyntaxShape, + Type, Value, }; #[derive(Clone)] @@ -106,7 +109,7 @@ impl Command for Do { block, input, call.redirect_stdout, - capture_errors || ignore_shell_errors || ignore_program_errors, + call.redirect_stdout, ); match result { @@ -118,6 +121,52 @@ impl Command for Do { metadata, trim_end_newline, }) if capture_errors => { + // Use a thread to receive stdout message. + // Or we may get a deadlock if child process sends out too much bytes to stderr. + // + // For example: in normal linux system, stderr pipe's limit is 65535 bytes. + // if child process sends out 65536 bytes, the process will be hanged because no consumer + // consumes the first 65535 bytes + // So we need a thread to receive stdout message, then the current thread can continue to consume + // stderr messages. + let stdout_handler = stdout.map(|stdout_stream| { + thread::spawn(move || { + let ctrlc = stdout_stream.ctrlc.clone(); + let span = stdout_stream.span; + RawStream::new( + Box::new(vec![stdout_stream.into_bytes().map(|s| s.item)].into_iter()), + ctrlc, + span, + ) + }) + }); + + // Intercept stderr so we can return it in the error if the exit code is non-zero. + // The threading issues mentioned above dictate why we also need to intercept stdout. + 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 stdout = if let Some(handle) = stdout_handler { + match handle.join() { + Err(err) => { + return Err(ShellError::ExternalCommand( + "Fail to receive external commands stdout message".to_string(), + format!("{err:?}"), + span, + )); + } + Ok(res) => Some(res), + } + } else { + None + }; + let mut exit_code_ctrlc = None; let exit_code: Vec = match exit_code { None => vec![], @@ -128,11 +177,6 @@ impl Command for Do { }; if let Some(Value::Int { val: code, .. }) = exit_code.last() { if *code != 0 { - let stderr_msg = match stderr { - None => "".to_string(), - Some(stderr_stream) => stderr_stream.into_string().map(|s| s.item)?, - }; - return Err(ShellError::ExternalCommand( "External command failed".to_string(), stderr_msg, @@ -143,7 +187,11 @@ impl Command for Do { Ok(PipelineData::ExternalStream { stdout, - stderr, + 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, @@ -168,10 +216,9 @@ impl Command for Do { metadata, trim_end_newline, }), - Ok(PipelineData::Value(Value::Error { .. }, ..)) if ignore_shell_errors => { + Ok(PipelineData::Value(..)) | Err(_) if ignore_shell_errors => { Ok(PipelineData::empty()) } - Err(_) if ignore_shell_errors => Ok(PipelineData::empty()), r => r, } } diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs index ea022581fd..edb83c647e 100644 --- a/crates/nu-command/tests/commands/do_.rs +++ b/crates/nu-command/tests/commands/do_.rs @@ -100,7 +100,7 @@ fn ignore_error_should_work_for_external_command() { #[test] #[cfg(not(windows))] -fn ignore_error_with_too_much_stderr_not_hang_nushell() { +fn capture_error_with_too_much_stderr_not_hang_nushell() { use nu_test_support::fs::Stub::FileWithContent; use nu_test_support::pipeline; use nu_test_support::playground::Playground; @@ -115,8 +115,8 @@ fn ignore_error_with_too_much_stderr_not_hang_nushell() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - do -i {sh -c "cat a_large_file.txt 1>&2"} | complete | get stderr - "# + do -c {sh -c "cat a_large_file.txt 1>&2"} | complete | get stderr + "#, )); assert_eq!(actual.out, large_file_body); @@ -125,7 +125,7 @@ fn ignore_error_with_too_much_stderr_not_hang_nushell() { #[test] #[cfg(not(windows))] -fn ignore_error_with_too_much_stdout_not_hang_nushell() { +fn capture_error_with_too_much_stdout_not_hang_nushell() { use nu_test_support::fs::Stub::FileWithContent; use nu_test_support::pipeline; use nu_test_support::playground::Playground; @@ -140,8 +140,8 @@ fn ignore_error_with_too_much_stdout_not_hang_nushell() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - do -i {sh -c "cat a_large_file.txt"} | complete | get stdout - "# + do -c {sh -c "cat a_large_file.txt"} | complete | get stdout + "#, )); assert_eq!(actual.out, large_file_body); @@ -150,7 +150,7 @@ fn ignore_error_with_too_much_stdout_not_hang_nushell() { #[test] #[cfg(not(windows))] -fn ignore_error_with_both_stdout_stderr_messages_not_hang_nushell() { +fn capture_error_with_both_stdout_stderr_messages_not_hang_nushell() { use nu_test_support::fs::Stub::FileWithContent; use nu_test_support::playground::Playground; Playground::setup( @@ -172,16 +172,16 @@ fn ignore_error_with_both_stdout_stderr_messages_not_hang_nushell() { let actual = nu!( cwd: dirs.test(), pipeline( r#" - do -i {bash test.sh} | complete | get stdout | str trim - "# + do -c {bash test.sh} | complete | get stdout | str trim + "#, )); assert_eq!(actual.out, expect_body); // check for stderr let actual = nu!( cwd: dirs.test(), pipeline( r#" - do -i {bash test.sh} | complete | get stderr | str trim - "# + do -c {bash test.sh} | complete | get stderr | str trim + "#, )); assert_eq!(actual.out, expect_body); }, diff --git a/crates/nu-command/tests/commands/save.rs b/crates/nu-command/tests/commands/save.rs index 2c42b07782..44f1b5ba35 100644 --- a/crates/nu-command/tests/commands/save.rs +++ b/crates/nu-command/tests/commands/save.rs @@ -95,7 +95,8 @@ fn save_stderr_and_stdout_to_same_file() { r#" let-env FOO = "bar"; let-env BAZ = "ZZZ"; - do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt"#, + do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_5/new-file.txt --stderr save_test_5/new-file.txt + "#, ); let actual = file_contents(expected_file); @@ -118,7 +119,8 @@ fn save_stderr_and_stdout_to_diff_file() { r#" let-env FOO = "bar"; let-env BAZ = "ZZZ"; - do -i {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt"#, + do -c {nu -c 'nu --testbin echo_env FOO; nu --testbin echo_env_stderr BAZ'} | save -r save_test_6/log.txt --stderr save_test_6/err.txt + "#, ); let actual = file_contents(expected_file);