diff --git a/crates/nu-cli/src/config_files.rs b/crates/nu-cli/src/config_files.rs index 701db7ac65..427f503a6f 100644 --- a/crates/nu-cli/src/config_files.rs +++ b/crates/nu-cli/src/config_files.rs @@ -2,10 +2,10 @@ use crate::util::eval_source; #[cfg(feature = "plugin")] use nu_path::canonicalize_with; #[cfg(feature = "plugin")] -use nu_protocol::{engine::StateWorkingSet, report_error, ParseError, PluginRegistryFile, Spanned}; +use nu_protocol::{engine::StateWorkingSet, ParseError, PluginRegistryFile, Spanned}; use nu_protocol::{ engine::{EngineState, Stack}, - report_error_new, HistoryFileFormat, PipelineData, + report_shell_error, HistoryFileFormat, PipelineData, }; #[cfg(feature = "plugin")] use nu_utils::perf; @@ -36,7 +36,7 @@ pub fn read_plugin_file( .and_then(|p| Path::new(&p.item).extension()) .is_some_and(|ext| ext == "nu") { - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: "Wrong plugin file format".into(), @@ -81,7 +81,7 @@ pub fn read_plugin_file( return; } } else { - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: format!( @@ -113,7 +113,7 @@ pub fn read_plugin_file( Ok(contents) => contents, Err(err) => { log::warn!("Failed to read plugin registry file: {err:?}"); - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: format!( @@ -146,7 +146,7 @@ pub fn read_plugin_file( nu_plugin_engine::load_plugin_file(&mut working_set, &contents, span); if let Err(err) = engine_state.merge_delta(working_set.render()) { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); return; } @@ -166,7 +166,7 @@ pub fn add_plugin_file( ) { use std::path::Path; - let working_set = StateWorkingSet::new(engine_state); + use nu_protocol::report_parse_error; if let Ok(cwd) = engine_state.cwd_as_string(None) { if let Some(plugin_file) = plugin_file { @@ -181,8 +181,8 @@ pub fn add_plugin_file( engine_state.plugin_path = Some(path) } else { // It's an error if the directory for the plugin file doesn't exist. - report_error( - &working_set, + report_parse_error( + &StateWorkingSet::new(engine_state), &ParseError::FileNotFound( path_dir.to_string_lossy().into_owned(), plugin_file.span, @@ -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, @@ -230,11 +231,11 @@ pub fn eval_config_contents( match engine_state.cwd(Some(stack)) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } Err(e) => { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } } @@ -280,7 +281,7 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) - let old_contents = match std::fs::read(&old_plugin_file_path) { Ok(old_contents) => old_contents, Err(err) => { - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: "Can't read old plugin file to migrate".into(), @@ -349,7 +350,7 @@ pub fn migrate_old_plugin_file(engine_state: &EngineState, storage_path: &str) - .map_err(|e| e.into()) .and_then(|file| contents.write_to(file, None)) { - report_error_new( + report_shell_error( &engine_state, &ShellError::GenericError { error: "Failed to save migrated plugin file".into(), diff --git a/crates/nu-cli/src/eval_cmds.rs b/crates/nu-cli/src/eval_cmds.rs index 0a9136bc51..f0bebfc884 100644 --- a/crates/nu-cli/src/eval_cmds.rs +++ b/crates/nu-cli/src/eval_cmds.rs @@ -2,9 +2,10 @@ use log::info; use nu_engine::{convert_env_values, eval_block}; use nu_parser::parse; use nu_protocol::{ + cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, - report_error, PipelineData, ShellError, Spanned, Value, + report_parse_error, report_parse_warning, PipelineData, ShellError, Spanned, Value, }; use std::sync::Arc; @@ -61,16 +62,16 @@ pub fn evaluate_commands( let output = parse(&mut working_set, None, commands.item.as_bytes(), false); if let Some(warning) = working_set.parse_warnings.first() { - report_error(&working_set, warning); + report_parse_warning(&working_set, warning); } if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); std::process::exit(1); } if let Some(err) = working_set.compile_errors.first() { - report_error(&working_set, err); + report_compile_error(&working_set, err); // Not a fatal error, for now } @@ -92,11 +93,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)? { - if status.code() != 0 { - std::process::exit(status.code()) - } - } + pipeline.print(engine_state, stack, no_newline, false)?; info!("evaluate {}:{}:{}", file!(), line!(), column!()); diff --git a/crates/nu-cli/src/eval_file.rs b/crates/nu-cli/src/eval_file.rs index bac4378d1b..df75f7e4f8 100644 --- a/crates/nu-cli/src/eval_file.rs +++ b/crates/nu-cli/src/eval_file.rs @@ -4,9 +4,10 @@ use nu_engine::{convert_env_values, eval_block}; use nu_parser::parse; use nu_path::canonicalize_with; use nu_protocol::{ + cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, - report_error, PipelineData, ShellError, Span, Value, + report_parse_error, report_parse_warning, PipelineData, ShellError, Span, Value, }; use std::sync::Arc; @@ -77,17 +78,17 @@ pub fn evaluate_file( let block = parse(&mut working_set, Some(file_path_str), &file, false); if let Some(warning) = working_set.parse_warnings.first() { - report_error(&working_set, warning); + report_parse_warning(&working_set, warning); } // If any parse errors were found, report the first error and exit. if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); std::process::exit(1); } if let Some(err) = working_set.compile_errors.first() { - report_error(&working_set, err); + report_compile_error(&working_set, err); // Not a fatal error, for now } @@ -118,11 +119,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)? { - if status.code() != 0 { - std::process::exit(status.code()) - } - } + pipeline.print(engine_state, stack, true, false)?; // Invoke the main command with arguments. // Arguments with whitespaces are quoted, thus can be safely concatenated by whitespace. @@ -140,7 +137,7 @@ pub fn evaluate_file( }; if exit_code != 0 { - std::process::exit(exit_code) + std::process::exit(exit_code); } info!("evaluate {}:{}:{}", file!(), line!(), column!()); diff --git a/crates/nu-cli/src/prompt_update.rs b/crates/nu-cli/src/prompt_update.rs index dd99b6f338..d96bf40b8f 100644 --- a/crates/nu-cli/src/prompt_update.rs +++ b/crates/nu-cli/src/prompt_update.rs @@ -3,7 +3,7 @@ use log::trace; use nu_engine::ClosureEvalOnce; use nu_protocol::{ engine::{EngineState, Stack}, - report_error_new, Config, PipelineData, Value, + report_shell_error, Config, PipelineData, Value, }; use reedline::Prompt; @@ -80,7 +80,7 @@ fn get_prompt_string( result .map_err(|err| { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); }) .ok() } diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 95fb000be0..0e734571c9 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -27,7 +27,7 @@ use nu_parser::{lex, parse, trim_quotes_str}; use nu_protocol::{ config::NuCursorShape, engine::{EngineState, Stack, StateWorkingSet}, - report_error_new, HistoryConfig, HistoryFileFormat, PipelineData, ShellError, Span, Spanned, + report_shell_error, HistoryConfig, HistoryFileFormat, PipelineData, ShellError, Span, Spanned, Value, }; use nu_utils::{ @@ -88,7 +88,7 @@ pub fn evaluate_repl( let start_time = std::time::Instant::now(); // Translate environment variables from Strings to Values if let Err(e) = convert_env_values(engine_state, &unique_stack) { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } perf!("translate env vars", start_time, use_color); @@ -98,7 +98,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())); @@ -286,7 +286,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { // Before doing anything, merge the environment from the previous REPL iteration into the // permanent state. if let Err(err) = engine_state.merge_env(&mut stack, cwd) { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); } // Check whether $env.NU_USE_IR is set, so that the user can change it in the REPL // Temporary while IR eval is optional @@ -302,7 +302,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { // fire the "pre_prompt" hook if let Some(hook) = engine_state.get_config().hooks.pre_prompt.clone() { if let Err(err) = eval_hook(engine_state, &mut stack, None, vec![], &hook, "pre_prompt") { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); } } perf!("pre-prompt hook", start_time, use_color); @@ -312,7 +312,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { // fire the "env_change" hook let env_change = engine_state.get_config().hooks.env_change.clone(); if let Err(error) = hook::eval_env_change_hook(env_change, engine_state, &mut stack) { - report_error_new(engine_state, &error) + report_shell_error(engine_state, &error) } perf!("env-change hook", start_time, use_color); @@ -386,7 +386,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { trace!("adding menus"); line_editor = add_menus(line_editor, engine_reference, &stack_arc, config).unwrap_or_else(|e| { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); Reedline::create() }); @@ -506,7 +506,7 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) { &hook, "pre_execution", ) { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); } } @@ -808,7 +808,7 @@ fn do_auto_cd( ) { let path = { if !path.exists() { - report_error_new( + report_shell_error( engine_state, &ShellError::DirectoryNotFound { dir: path.to_string_lossy().to_string(), @@ -820,7 +820,7 @@ fn do_auto_cd( }; if let PermissionResult::PermissionDenied(reason) = have_permission(path.clone()) { - report_error_new( + report_shell_error( engine_state, &ShellError::IOError { msg: format!("Cannot change directory to {path}: {reason}"), @@ -834,7 +834,7 @@ fn do_auto_cd( //FIXME: this only changes the current scope, but instead this environment variable //should probably be a block that loads the information from the state in the overlay if let Err(err) = stack.set_cwd(&path) { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); return; }; let cwd = Value::string(cwd, span); @@ -867,7 +867,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()); } /// @@ -1141,7 +1141,7 @@ fn setup_keybindings(engine_state: &EngineState, line_editor: Reedline) -> Reedl } }, Err(e) => { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); line_editor } }; diff --git a/crates/nu-cli/src/util.rs b/crates/nu-cli/src/util.rs index e912bceb5f..038d0d0312 100644 --- a/crates/nu-cli/src/util.rs +++ b/crates/nu-cli/src/util.rs @@ -2,9 +2,11 @@ use nu_cmd_base::hook::eval_hook; use nu_engine::{eval_block, eval_block_with_early_return}; use nu_parser::{escape_quote_string, lex, parse, unescape_unquote_string, Token, TokenContents}; use nu_protocol::{ + cli_error::report_compile_error, debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, - report_error, report_error_new, PipelineData, ShellError, Span, Value, + report_parse_error, report_parse_warning, report_shell_error, PipelineData, ShellError, Span, + Value, }; #[cfg(windows)] use nu_utils::enable_vt_processing; @@ -39,7 +41,7 @@ fn gather_env_vars( init_cwd: &Path, ) { fn report_capture_error(engine_state: &EngineState, env_str: &str, msg: &str) { - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: format!("Environment variable was not captured: {env_str}"), @@ -70,7 +72,7 @@ fn gather_env_vars( } None => { // Could not capture current working directory - report_error_new( + report_shell_error( engine_state, &ShellError::GenericError { error: "Current directory is not a valid utf-8 path".into(), @@ -210,18 +212,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 + report_shell_error(engine_state, &err); + 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 +247,7 @@ fn evaluate_source( fname: &str, input: PipelineData, allow_return: bool, -) -> Result, ShellError> { +) -> Result { let (block, delta) = { let mut working_set = StateWorkingSet::new(engine_state); let output = parse( @@ -254,16 +257,16 @@ fn evaluate_source( false, ); if let Some(warning) = working_set.parse_warnings.first() { - report_error(&working_set, warning); + report_parse_warning(&working_set, warning); } if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); - return Ok(Some(1)); + report_parse_error(&working_set, err); + return Ok(true); } if let Some(err) = working_set.compile_errors.first() { - report_error(&working_set, err); + report_compile_error(&working_set, err); // Not a fatal error, for now } @@ -278,25 +281,23 @@ fn evaluate_source( eval_block::(engine_state, stack, &block, input) }?; - let status = if let PipelineData::ByteStream(..) = pipeline { - pipeline.print(engine_state, stack, false, false)? + if let PipelineData::ByteStream(..) = pipeline { + 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() { - 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) - }? - }; + pipeline.print(engine_state, stack, true, false) + }?; - Ok(status.map(|status| status.code())) + Ok(false) } #[cfg(test)] diff --git a/crates/nu-cmd-base/src/hook.rs b/crates/nu-cmd-base/src/hook.rs index cef5348618..6eba273f3d 100644 --- a/crates/nu-cmd-base/src/hook.rs +++ b/crates/nu-cmd-base/src/hook.rs @@ -3,7 +3,7 @@ use miette::Result; use nu_engine::{eval_block, eval_block_with_early_return}; use nu_parser::parse; use nu_protocol::{ - cli_error::{report_error, report_error_new}, + cli_error::{report_parse_error, report_shell_error}, debugger::WithoutDebug, engine::{Closure, EngineState, Stack, StateWorkingSet}, PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId, @@ -91,7 +91,7 @@ pub fn eval_hook( false, ); if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); return Err(ShellError::UnsupportedConfigValue { expected: "valid source code".into(), @@ -123,7 +123,7 @@ pub fn eval_hook( output = pipeline_data; } Err(err) => { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); } } @@ -223,7 +223,7 @@ pub fn eval_hook( false, ); if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); return Err(ShellError::UnsupportedConfigValue { expected: "valid source code".into(), @@ -251,7 +251,7 @@ pub fn eval_hook( output = pipeline_data; } Err(err) => { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); } } diff --git a/crates/nu-cmd-lang/src/core_commands/do_.rs b/crates/nu-cmd-lang/src/core_commands/do_.rs index 61c9cf09a0..6c7c36e7d1 100644 --- a/crates/nu-cmd-lang/src/core_commands/do_.rs +++ b/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) { - return Err(ShellError::ExternalCommand { - label: "External command failed".to_string(), - help: stderr_msg, - span, - }); - } + child.wait()?; let mut child = ChildProcess::from_raw(None, None, None, span); if let Some(stdout) = stdout { diff --git a/crates/nu-cmd-lang/src/core_commands/for_.rs b/crates/nu-cmd-lang/src/core_commands/for_.rs index 16e1364225..912d738524 100644 --- a/crates/nu-cmd-lang/src/core_commands/for_.rs +++ b/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 { .. }) => { - break; - } - Err(ShellError::Continue { .. }) => { - 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), - ); - } - } - } + Err(ShellError::Break { .. }) => break, + Err(ShellError::Continue { .. }) => continue, + Err(err) => return Err(err), + Ok(data) => data.drain()?, } } } @@ -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 { .. }) => { - break; - } - Err(ShellError::Continue { .. }) => { - 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), - ); - } - } - } + Err(ShellError::Break { .. }) => break, + Err(ShellError::Continue { .. }) => continue, + Err(err) => return Err(err), + Ok(data) => data.drain()?, } } } diff --git a/crates/nu-cmd-lang/src/core_commands/if_.rs b/crates/nu-cmd-lang/src/core_commands/if_.rs index 3b18b23879..e40bb13b82 100644 --- a/crates/nu-cmd-lang/src/core_commands/if_.rs +++ b/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()) diff --git a/crates/nu-cmd-lang/src/core_commands/loop_.rs b/crates/nu-cmd-lang/src/core_commands/loop_.rs index 1ef3706a6e..b799d32141 100644 --- a/crates/nu-cmd-lang/src/core_commands/loop_.rs +++ b/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 { .. }) => { - break; - } - Err(ShellError::Continue { .. }) => { - 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)); - } - } - } + Err(ShellError::Break { .. }) => break, + Err(ShellError::Continue { .. }) => continue, + Err(err) => return Err(err), + Ok(data) => data.drain()?, } } Ok(PipelineData::empty()) diff --git a/crates/nu-cmd-lang/src/core_commands/match_.rs b/crates/nu-cmd-lang/src/core_commands/match_.rs index 71ec8ebaec..a7bebeb857 100644 --- a/crates/nu-cmd-lang/src/core_commands/match_.rs +++ b/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 { diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index fd2d3bd3cc..25d6fe59a3 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -47,6 +47,7 @@ impl Command for Try { call: &Call, input: PipelineData, ) -> Result { + 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) { - Err(error) => { - let error = intercept_block_control(error)?; - let err_record = err_to_record(error, call.head); - handle_catch(err_record, catch_block, engine_state, stack, eval_block) - } + let result = eval_block(engine_state, stack, try_block, input) + .and_then(|pipeline| pipeline.write_to_out_dests(engine_state, stack)); + + match result { + 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)?; - 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) - } + run_catch(*error, head, catch_block, engine_state, stack, eval_block) } + Ok(pipeline) => Ok(pipeline), } } @@ -109,28 +95,33 @@ impl Command for Try { } } -fn handle_catch( - err_value: Value, - catch_block: Option, +fn run_catch( + error: ShellError, + span: Span, + catch: Option, engine_state: &EngineState, stack: &mut Stack, - eval_block_fn: EvalBlockFn, + eval_block: EvalBlockFn, ) -> Result { - if let Some(catch_block) = catch_block { - let catch_block = engine_state.get_block(catch_block.block_id); + let error = intercept_block_control(error)?; + + if let Some(catch) = catch { + stack.set_last_error(&error); + let error = error.into_value(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,25 +134,13 @@ fn handle_catch( /// `Err` when flow control to bubble up with `?` fn intercept_block_control(error: ShellError) -> Result { match error { - nu_protocol::ShellError::Break { .. } => Err(error), - nu_protocol::ShellError::Continue { .. } => Err(error), - nu_protocol::ShellError::Return { .. } => Err(error), + ShellError::Break { .. } => Err(error), + ShellError::Continue { .. } => 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( - record! { - "msg" => Value::string(error.to_string(), head), - "debug" => Value::string(format!("{error:?}"), head), - "raw" => Value::error(error, head), - }, - head, - ) -} - #[cfg(test)] mod test { use super::*; diff --git a/crates/nu-cmd-lang/src/core_commands/while_.rs b/crates/nu-cmd-lang/src/core_commands/while_.rs index acdac7edef..e9578ae6f5 100644 --- a/crates/nu-cmd-lang/src/core_commands/while_.rs +++ b/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 { .. }) => { - break; - } - Err(ShellError::Continue { .. }) => { - 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, - ), - ); - } - } - } + Err(ShellError::Break { .. }) => break, + Err(ShellError::Continue { .. }) => continue, + Err(err) => return Err(err), + Ok(data) => data.drain()?, } } else { break; diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index 70ee7a9b0f..9ae3808480 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -4,7 +4,7 @@ use nu_protocol::{ ast::Block, debugger::WithoutDebug, engine::{StateDelta, StateWorkingSet}, - report_error_new, Range, + report_shell_error, Range, }; use std::{ sync::Arc, @@ -125,7 +125,7 @@ pub fn eval_block( nu_engine::eval_block::(engine_state, &mut stack, &block, input) .and_then(|data| data.into_value(Span::test_data())) .unwrap_or_else(|err| { - report_error_new(engine_state, &err); + report_shell_error(engine_state, &err); panic!("test eval error in `{}`: {:?}", "TODO", err) }) } diff --git a/crates/nu-command/src/debug/timeit.rs b/crates/nu-command/src/debug/timeit.rs index 08de95dcd2..7bb492769c 100644 --- a/crates/nu-command/src/debug/timeit.rs +++ b/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() diff --git a/crates/nu-command/src/filesystem/rm.rs b/crates/nu-command/src/filesystem/rm.rs index 3475c1a595..3d2c712155 100644 --- a/crates/nu-command/src/filesystem/rm.rs +++ b/crates/nu-command/src/filesystem/rm.rs @@ -3,7 +3,7 @@ use super::util::{get_rest_for_glob_pattern, try_interaction}; use nu_engine::{command_prelude::*, env::current_dir}; use nu_glob::MatchOptions; use nu_path::expand_path_with; -use nu_protocol::{report_error_new, NuGlob}; +use nu_protocol::{report_shell_error, NuGlob}; #[cfg(unix)] use std::os::unix::prelude::FileTypeExt; use std::{ @@ -452,7 +452,7 @@ fn rm( match result { Ok(None) => {} Ok(Some(msg)) => eprintln!("{msg}"), - Err(err) => report_error_new(engine_state, &err), + Err(err) => report_shell_error(engine_state, &err), } } diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index 5138af354e..6f022acca8 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -8,7 +8,7 @@ use notify_debouncer_full::{ use nu_engine::{command_prelude::*, ClosureEval}; use nu_protocol::{ engine::{Closure, StateWorkingSet}, - format_error, + format_shell_error, }; use std::{ path::PathBuf, @@ -198,7 +198,7 @@ impl Command for Watch { } Err(err) => { let working_set = StateWorkingSet::new(engine_state); - eprintln!("{}", format_error(&working_set, &err)); + eprintln!("{}", format_shell_error(&working_set, &err)); } } } diff --git a/crates/nu-command/src/filters/group.rs b/crates/nu-command/src/filters/group.rs index 5327cad610..34b70f697a 100644 --- a/crates/nu-command/src/filters/group.rs +++ b/crates/nu-command/src/filters/group.rs @@ -1,5 +1,5 @@ use nu_engine::command_prelude::*; -use nu_protocol::{report_warning_new, ParseWarning, ValueIterator}; +use nu_protocol::{report_shell_warning, ValueIterator}; #[derive(Clone)] pub struct Group; @@ -55,9 +55,9 @@ impl Command for Group { ) -> Result { let head = call.head; - report_warning_new( + report_shell_warning( engine_state, - &ParseWarning::DeprecatedWarning { + &ShellError::Deprecated { old_command: "group".into(), new_suggestion: "the new `chunks` command".into(), span: head, diff --git a/crates/nu-command/src/filters/tee.rs b/crates/nu-command/src/filters/tee.rs index db29d31e5f..65c0aba546 100644 --- a/crates/nu-command/src/filters/tee.rs +++ b/crates/nu-command/src/filters/tee.rs @@ -1,6 +1,6 @@ use nu_engine::{command_prelude::*, get_eval_block_with_early_return}; use nu_protocol::{ - byte_stream::copy_with_signals, engine::Closure, process::ChildPipe, report_error_new, + byte_stream::copy_with_signals, engine::Closure, process::ChildPipe, report_shell_error, ByteStream, ByteStreamSource, OutDest, PipelineMetadata, Signals, }; use std::{ @@ -353,7 +353,7 @@ fn tee_once( ) -> Result, std::io::Error> { thread::Builder::new().name("tee".into()).spawn(move || { if let Err(err) = on_thread() { - report_error_new(&engine_state, &err); + report_shell_error(&engine_state, &err); } }) } diff --git a/crates/nu-command/src/network/url/join.rs b/crates/nu-command/src/network/url/join.rs index 2bbd68bbcb..82f9e21914 100644 --- a/crates/nu-command/src/network/url/join.rs +++ b/crates/nu-command/src/network/url/join.rs @@ -279,7 +279,7 @@ impl UrlComponents { ..self }), _ => { - nu_protocol::report_error_new( + nu_protocol::report_shell_error( engine_state, &ShellError::GenericError { error: format!("'{key}' is not a valid URL field"), diff --git a/crates/nu-command/src/strings/encode_decode/decode_base64.rs b/crates/nu-command/src/strings/encode_decode/decode_base64.rs index 455aec915c..f3c2b7fcde 100644 --- a/crates/nu-command/src/strings/encode_decode/decode_base64.rs +++ b/crates/nu-command/src/strings/encode_decode/decode_base64.rs @@ -1,6 +1,6 @@ use super::base64::{operate, ActionType, Base64CommandArguments, CHARACTER_SET_DESC}; use nu_engine::command_prelude::*; -use nu_protocol::{report_warning_new, ParseWarning}; +use nu_protocol::report_shell_warning; #[derive(Clone)] pub struct DecodeBase64Old; @@ -78,9 +78,9 @@ impl Command for DecodeBase64Old { call: &Call, input: PipelineData, ) -> Result { - report_warning_new( + report_shell_warning( engine_state, - &ParseWarning::DeprecatedWarning { + &ShellError::Deprecated { old_command: "decode base64".into(), new_suggestion: "the new `decode new-base64` version".into(), span: call.head, diff --git a/crates/nu-command/src/strings/encode_decode/encode_base64.rs b/crates/nu-command/src/strings/encode_decode/encode_base64.rs index a484eeb974..0181f94aef 100644 --- a/crates/nu-command/src/strings/encode_decode/encode_base64.rs +++ b/crates/nu-command/src/strings/encode_decode/encode_base64.rs @@ -1,6 +1,6 @@ use super::base64::{operate, ActionType, Base64CommandArguments, CHARACTER_SET_DESC}; use nu_engine::command_prelude::*; -use nu_protocol::{report_warning_new, ParseWarning}; +use nu_protocol::report_shell_warning; #[derive(Clone)] pub struct EncodeBase64Old; @@ -82,9 +82,9 @@ impl Command for EncodeBase64Old { call: &Call, input: PipelineData, ) -> Result { - report_warning_new( + report_shell_warning( engine_state, - &ParseWarning::DeprecatedWarning { + &ShellError::Deprecated { old_command: "encode base64".into(), new_suggestion: "the new `encode new-base64` version".into(), span: call.head, diff --git a/crates/nu-command/src/strings/str_/contains.rs b/crates/nu-command/src/strings/str_/contains.rs index e64ed8c72b..de91768e76 100644 --- a/crates/nu-command/src/strings/str_/contains.rs +++ b/crates/nu-command/src/strings/str_/contains.rs @@ -78,7 +78,7 @@ impl Command for SubCommand { input: PipelineData, ) -> Result { if call.has_flag_const(working_set, "not")? { - nu_protocol::report_error_new( + nu_protocol::report_shell_error( working_set.permanent(), &ShellError::GenericError { error: "Deprecated option".into(), diff --git a/crates/nu-command/tests/commands/do_.rs b/crates/nu-command/tests/commands/do_.rs index 5f46b02c17..645421c15f 100644 --- a/crates/nu-command/tests/commands/do_.rs +++ b/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, ""); } diff --git a/crates/nu-command/tests/commands/try_.rs b/crates/nu-command/tests/commands/try_.rs index c4fc9269d0..264b20796f 100644 --- a/crates/nu-command/tests/commands/try_.rs +++ b/crates/nu-command/tests/commands/try_.rs @@ -95,7 +95,13 @@ fn can_catch_infinite_recursion() { } #[test] -fn exit_code_available_in_catch() { - let actual = nu!("try { nu -c 'exit 42' } catch { $env.LAST_EXIT_CODE }"); +fn exit_code_available_in_catch_env() { + let actual = nu!("try { nu -c 'exit 42'; null } catch { $env.LAST_EXIT_CODE }"); + assert_eq!(actual.out, "42"); +} + +#[test] +fn exit_code_available_in_catch() { + let actual = nu!("try { nu -c 'exit 42'; null } catch { |e| $e.exit_code }"); assert_eq!(actual.out, "42"); } diff --git a/crates/nu-engine/src/compile/builder.rs b/crates/nu-engine/src/compile/builder.rs index 2a443b7bde..655ffbddc5 100644 --- a/crates/nu-engine/src/compile/builder.rs +++ b/crates/nu-engine/src/compile/builder.rs @@ -278,7 +278,6 @@ impl BlockBuilder { Instruction::OnError { index: _ } => Ok(()), Instruction::OnErrorInto { index: _, dst } => allocate(&[], &[*dst]), Instruction::PopErrorHandler => Ok(()), - Instruction::CheckExternalFailed { dst, src } => allocate(&[*src], &[*dst, *src]), Instruction::ReturnEarly { src } => allocate(&[*src], &[]), Instruction::Return { src } => allocate(&[*src], &[]), }; @@ -499,6 +498,7 @@ impl BlockBuilder { } /// Mark an unreachable code path. Produces an error at runtime if executed. + #[allow(dead_code)] // currently unused, but might be used in the future. pub(crate) fn unreachable(&mut self, span: Span) -> Result<(), CompileError> { self.push(Instruction::Unreachable.into_spanned(span)) } diff --git a/crates/nu-engine/src/compile/keyword.rs b/crates/nu-engine/src/compile/keyword.rs index 12f4a54c10..8fe939d638 100644 --- a/crates/nu-engine/src/compile/keyword.rs +++ b/crates/nu-engine/src/compile/keyword.rs @@ -362,12 +362,8 @@ pub(crate) fn compile_try( // // on-error-into ERR, %io_reg // or without // %io_reg <- <...block...> <- %io_reg - // check-external-failed %failed_reg, %io_reg - // branch-if %failed_reg, FAIL // pop-error-handler // jump END - // FAIL: drain %io_reg - // unreachable // ERR: clone %err_reg, %io_reg // store-variable $err_var, %err_reg // or without // %io_reg <- <...catch block...> <- %io_reg // set to empty if no catch block @@ -378,12 +374,8 @@ pub(crate) fn compile_try( // %closure_reg <- // on-error-into ERR, %io_reg // %io_reg <- <...block...> <- %io_reg - // check-external-failed %failed_reg, %io_reg - // branch-if %failed_reg, FAIL // pop-error-handler // jump END - // FAIL: drain %io_reg - // unreachable // ERR: clone %err_reg, %io_reg // push-positional %closure_reg // push-positional %err_reg @@ -405,7 +397,6 @@ pub(crate) fn compile_try( let catch_span = catch_expr.map(|e| e.span).unwrap_or(call.head); let err_label = builder.label(None); - let failed_label = builder.label(None); let end_label = builder.label(None); // We have two ways of executing `catch`: if it was provided as a literal, we can inline it. @@ -470,32 +461,13 @@ pub(crate) fn compile_try( io_reg, )?; - // Check for external command exit code failure, and also redirect that to the catch handler - let failed_reg = builder.next_register()?; - builder.push( - Instruction::CheckExternalFailed { - dst: failed_reg, - src: io_reg, - } - .into_spanned(catch_span), - )?; - builder.branch_if(failed_reg, failed_label, catch_span)?; - // Successful case: pop the error handler builder.push(Instruction::PopErrorHandler.into_spanned(call.head))?; // Jump over the failure case builder.jump(end_label, catch_span)?; - // Set up an error handler preamble for failed external. - // Draining the %io_reg results in the error handler being called with Empty, and sets - // $env.LAST_EXIT_CODE - builder.set_label(failed_label, builder.here())?; - builder.drain(io_reg, catch_span)?; - builder.add_comment("branches to err"); - builder.unreachable(catch_span)?; - - // This is the real error handler + // This is the error handler builder.set_label(err_label, builder.here())?; // Mark out register as likely not clean - state in error handler is not well defined diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 56a85e91e7..ccb494e68e 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -250,7 +250,7 @@ pub fn eval_expression_with_input( stack: &mut Stack, expr: &Expression, mut input: PipelineData, -) -> Result<(PipelineData, bool), ShellError> { +) -> Result { match &expr.expr { Expr::Call(call) => { input = eval_call::(engine_state, stack, call, input)?; @@ -298,16 +298,7 @@ pub fn eval_expression_with_input( } }; - // If input an external command, - // 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() - } + Ok(input) } fn eval_redirection( @@ -401,9 +392,8 @@ fn eval_element_with_input_inner( stack: &mut Stack, element: &PipelineElement, input: PipelineData, -) -> Result<(PipelineData, bool), ShellError> { - let (data, failed) = - eval_expression_with_input::(engine_state, stack, &element.expr, input)?; +) -> Result { + let data = eval_expression_with_input::(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( PipelineData::Empty => PipelineData::Empty, }; - Ok((data, failed)) + Ok(data) } fn eval_element_with_input( @@ -481,20 +471,11 @@ fn eval_element_with_input( stack: &mut Stack, element: &PipelineElement, input: PipelineData, -) -> Result<(PipelineData, bool), ShellError> { +) -> Result { D::enter_element(engine_state, element); - match eval_element_with_input_inner::(engine_state, stack, element, input) { - Ok((data, failed)) => { - let res = Ok(data); - 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)) - } - } + let result = eval_element_with_input_inner::(engine_state, stack, element, input); + D::leave_element(engine_state, element, &result); + result } pub fn eval_block_with_early_return( @@ -509,7 +490,7 @@ pub fn eval_block_with_early_return( } } -pub fn eval_block( +fn eval_block_inner( engine_state: &EngineState, stack: &mut Stack, block: &Block, @@ -520,8 +501,6 @@ pub fn eval_block( return eval_ir_block::(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( (next_out.or(Some(OutDest::Pipe)), next_err), )?; let stack = &mut stack.push_redirection(stdout, stderr); - let (output, failed) = - eval_element_with_input::(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; + input = eval_element_with_input::(engine_state, stack, element, input)?; } if last_pipeline { @@ -560,13 +532,7 @@ pub fn eval_block( (stack.pipe_stdout().cloned(), stack.pipe_stderr().cloned()), )?; let stack = &mut stack.push_redirection(stdout, stderr); - let (output, failed) = eval_element_with_input::(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; + input = eval_element_with_input::(engine_state, stack, last, input)?; } else { let (stdout, stderr) = eval_element_redirection::( engine_state, @@ -575,40 +541,41 @@ pub fn eval_block( (None, None), )?; let stack = &mut stack.push_redirection(stdout, stderr); - let (output, failed) = eval_element_with_input::(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 { + match eval_element_with_input::(engine_state, stack, last, input)? { PipelineData::ByteStream(stream, ..) => { let span = stream.span(); - let status = stream.drain()?; - if let Some(status) = status { - stack.add_env_var( - "LAST_EXIT_CODE".into(), - Value::int(status.code().into(), span), - ); - if status.code() != 0 { - break; - } + if let Err(err) = stream.drain() { + stack.set_last_error(&err); + return Err(err); + } else { + stack.set_last_exit_code(0, span); } } - 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( + engine_state: &EngineState, + stack: &mut Stack, + block: &Block, + input: PipelineData, +) -> Result { + D::enter_block(engine_state, block); + let result = eval_block_inner::(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( engine_state: &EngineState, stack: &mut Stack, @@ -639,8 +606,7 @@ pub fn eval_collect( 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); diff --git a/crates/nu-engine/src/eval_helpers.rs b/crates/nu-engine/src/eval_helpers.rs index 65ebc6b61d..baa7bf432b 100644 --- a/crates/nu-engine/src/eval_helpers.rs +++ b/crates/nu-engine/src/eval_helpers.rs @@ -25,12 +25,8 @@ pub type EvalBlockWithEarlyReturnFn = pub type EvalExpressionFn = fn(&EngineState, &mut Stack, &Expression) -> Result; /// Type of eval_expression_with_input() function -pub type EvalExpressionWithInputFn = fn( - &EngineState, - &mut Stack, - &Expression, - PipelineData, -) -> Result<(PipelineData, bool), ShellError>; +pub type EvalExpressionWithInputFn = + fn(&EngineState, &mut Stack, &Expression, PipelineData) -> Result; /// Type of eval_subexpression() function pub type EvalSubexpressionFn = diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 8550cbab99..34ec541a14 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -6,9 +6,9 @@ use nu_protocol::{ debugger::DebugContext, engine::{Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack}, ir::{Call, DataSlice, Instruction, IrAstRef, IrBlock, Literal, RedirectMode}, - record, ByteStreamSource, DataSource, DeclId, ErrSpan, Flag, IntoPipelineData, IntoSpanned, - ListStream, OutDest, PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, - ShellError, Signals, Signature, Span, Spanned, Type, Value, VarId, ENV_VARIABLE_ID, + ByteStreamSource, DataSource, DeclId, ErrSpan, Flag, IntoPipelineData, IntoSpanned, ListStream, + OutDest, PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, + Signals, Signature, Span, Spanned, Type, Value, VarId, ENV_VARIABLE_ID, }; use nu_utils::IgnoreCaseExt; @@ -207,18 +207,6 @@ fn eval_ir_block_impl( 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 { .. } @@ -259,15 +247,10 @@ fn prepare_error_handler( if let Some(reg_id) = error_handler.error_register { if let Some(error) = error { // Create the error value and put it in the register - let value = Value::record( - record! { - "msg" => Value::string(format!("{}", error.item), error.span), - "debug" => Value::string(format!("{:?}", error.item), error.span), - "raw" => Value::error(error.item, error.span), - }, - error.span, + ctx.put_reg( + reg_id, + error.item.into_value(error.span).into_pipeline_data(), ); - ctx.put_reg(reg_id, PipelineData::Value(value, None)); } else { // Set the register to empty ctx.put_reg(reg_id, PipelineData::Empty); @@ -281,7 +264,6 @@ enum InstructionResult { Continue, Branch(usize), Return(RegId), - ExitCode(i32), } /// Perform an instruction @@ -788,13 +770,6 @@ fn eval_instruction( ctx.stack.error_handlers.pop(ctx.error_handler_base); Ok(Continue) } - Instruction::CheckExternalFailed { dst, src } => { - let data = ctx.take_reg(*src); - let (data, failed) = data.check_external_failed()?; - ctx.put_reg(*src, data); - ctx.put_reg(*dst, Value::bool(failed, *span).into_pipeline_data()); - Ok(Continue) - } Instruction::ReturnEarly { src } => { let val = ctx.collect_reg(*src, *span)?; Err(ShellError::Return { @@ -1362,23 +1337,23 @@ fn collect(data: PipelineData, fallback_span: Span) -> Result, data: PipelineData) -> Result { use self::InstructionResult::*; - let span = data.span().unwrap_or(Span::unknown()); - if let Some(exit_status) = data.drain()? { - ctx.stack.add_env_var( - "LAST_EXIT_CODE".into(), - Value::int(exit_status.code() as i64, span), - ); - if exit_status.code() == 0 { - Ok(Continue) - } else { - Ok(ExitCode(exit_status.code())) + match data { + PipelineData::ByteStream(stream, ..) => { + let span = stream.span(); + if let Err(err) = stream.drain() { + ctx.stack.set_last_error(&err); + return Err(err); + } else { + ctx.stack.set_last_exit_code(0, span); + } } - } else { - Ok(Continue) + PipelineData::ListStream(stream, ..) => stream.drain()?, + PipelineData::Value(..) | PipelineData::Empty => {} } + Ok(Continue) } enum RedirectionStream { diff --git a/crates/nu-plugin-engine/src/init.rs b/crates/nu-plugin-engine/src/init.rs index 44fa4f031f..67ca57245c 100644 --- a/crates/nu-plugin-engine/src/init.rs +++ b/crates/nu-plugin-engine/src/init.rs @@ -15,7 +15,7 @@ use nu_plugin_core::{ ServerCommunicationIo, }; use nu_protocol::{ - engine::StateWorkingSet, report_error_new, PluginIdentity, PluginRegistryFile, + engine::StateWorkingSet, report_shell_error, PluginIdentity, PluginRegistryFile, PluginRegistryItem, PluginRegistryItemData, RegisteredPlugin, ShellError, Span, }; @@ -225,7 +225,7 @@ pub fn load_plugin_file( for plugin in &plugin_registry_file.plugins { // Any errors encountered should just be logged. if let Err(err) = load_plugin_registry_item(working_set, plugin, span) { - report_error_new(working_set.permanent_state, &err) + report_shell_error(working_set.permanent_state, &err) } } } diff --git a/crates/nu-plugin-test-support/src/plugin_test.rs b/crates/nu-plugin-test-support/src/plugin_test.rs index 5e3334c78b..d414887e84 100644 --- a/crates/nu-plugin-test-support/src/plugin_test.rs +++ b/crates/nu-plugin-test-support/src/plugin_test.rs @@ -10,7 +10,7 @@ use nu_plugin_protocol::PluginCustomValue; use nu_protocol::{ debugger::WithoutDebug, engine::{EngineState, Stack, StateWorkingSet}, - report_error_new, CustomValue, Example, IntoSpanned as _, LabeledError, PipelineData, + report_shell_error, CustomValue, Example, IntoSpanned as _, LabeledError, PipelineData, ShellError, Signals, Span, Value, }; @@ -252,7 +252,7 @@ impl PluginTest { Err(err) => { // Report the error failed_header(); - report_error_new(&self.engine_state, &err); + report_shell_error(&self.engine_state, &err); } } } diff --git a/crates/nu-protocol/src/config/display_errors.rs b/crates/nu-protocol/src/config/display_errors.rs new file mode 100644 index 0000000000..6dbad874d2 --- /dev/null +++ b/crates/nu-protocol/src/config/display_errors.rs @@ -0,0 +1,29 @@ +use super::prelude::*; +use crate as nu_protocol; +use crate::ShellError; + +#[derive(Clone, Copy, Debug, IntoValue, PartialEq, Eq, Serialize, Deserialize)] +pub struct DisplayErrors { + pub exit_code: bool, + pub termination_signal: bool, +} + +impl DisplayErrors { + pub fn should_show(&self, error: &ShellError) -> bool { + match error { + ShellError::NonZeroExitCode { .. } => self.exit_code, + #[cfg(unix)] + ShellError::TerminatedBySignal { .. } => self.termination_signal, + _ => true, + } + } +} + +impl Default for DisplayErrors { + fn default() -> Self { + Self { + exit_code: true, + termination_signal: true, + } + } +} diff --git a/crates/nu-protocol/src/config/mod.rs b/crates/nu-protocol/src/config/mod.rs index b6f98ed883..e30892474c 100644 --- a/crates/nu-protocol/src/config/mod.rs +++ b/crates/nu-protocol/src/config/mod.rs @@ -12,6 +12,7 @@ pub use self::completer::{ CompleterConfig, CompletionAlgorithm, CompletionSort, ExternalCompleterConfig, }; pub use self::datetime_format::DatetimeFormatConfig; +pub use self::display_errors::DisplayErrors; pub use self::filesize::FilesizeConfig; pub use self::helper::extract_value; pub use self::history::{HistoryConfig, HistoryFileFormat}; @@ -28,6 +29,7 @@ pub use self::table::{FooterMode, TableConfig, TableIndexMode, TableMode, TrimSt mod completer; mod datetime_format; +mod display_errors; mod filesize; mod helper; mod history; @@ -67,6 +69,7 @@ pub struct Config { pub cursor_shape: CursorShapeConfig, pub datetime_format: DatetimeFormatConfig, pub error_style: ErrorStyle, + pub display_errors: DisplayErrors, pub use_kitty_protocol: bool, pub highlight_resolved_externals: bool, /// Configuration for plugins. @@ -121,6 +124,7 @@ impl Default for Config { keybindings: Vec::new(), error_style: ErrorStyle::Fancy, + display_errors: DisplayErrors::default(), use_kitty_protocol: false, highlight_resolved_externals: false, @@ -609,6 +613,29 @@ impl Value { "show_banner" => { process_bool_config(value, &mut errors, &mut config.show_banner); } + "display_errors" => { + if let Value::Record { val, .. } = value { + val.to_mut().retain_mut(|key2, value| { + let span = value.span(); + match key2 { + "exit_code" => { + process_bool_config(value, &mut errors, &mut config.display_errors.exit_code); + } + "termination_signal" => { + process_bool_config(value, &mut errors, &mut config.display_errors.termination_signal); + } + _ => { + report_invalid_key(&[key, key2], span, &mut errors); + return false; + } + }; + true + }); + } else { + report_invalid_value("should be a record", span, &mut errors); + *value = config.display_errors.into_value(span); + } + } "render_right_prompt_on_last_line" => { process_bool_config(value, &mut errors, &mut config.render_right_prompt_on_last_line); } diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 86420caf50..5819f1134b 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/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 { self.active_overlays .last() diff --git a/crates/nu-protocol/src/errors/cli_error.rs b/crates/nu-protocol/src/errors/cli_error.rs index 9ce6829b7b..8335f787fb 100644 --- a/crates/nu-protocol/src/errors/cli_error.rs +++ b/crates/nu-protocol/src/errors/cli_error.rs @@ -3,7 +3,7 @@ //! Relies on the `miette` crate for pretty layout use crate::{ engine::{EngineState, StateWorkingSet}, - ErrorStyle, + CompileError, ErrorStyle, ParseError, ParseWarning, ShellError, }; use miette::{ LabeledSpan, MietteHandlerOpts, NarratableReportHandler, ReportHandler, RgbColors, Severity, @@ -20,14 +20,35 @@ pub struct CliError<'src>( pub &'src StateWorkingSet<'src>, ); -pub fn format_error( - working_set: &StateWorkingSet, - error: &(dyn miette::Diagnostic + Send + Sync + 'static), -) -> String { +pub fn format_shell_error(working_set: &StateWorkingSet, error: &ShellError) -> String { format!("Error: {:?}", CliError(error, working_set)) } -pub fn report_error( +pub fn report_shell_error(engine_state: &EngineState, error: &ShellError) { + if engine_state.config.display_errors.should_show(error) { + report_error(&StateWorkingSet::new(engine_state), error) + } +} + +pub fn report_shell_warning(engine_state: &EngineState, error: &ShellError) { + if engine_state.config.display_errors.should_show(error) { + report_warning(&StateWorkingSet::new(engine_state), error) + } +} + +pub fn report_parse_error(working_set: &StateWorkingSet, error: &ParseError) { + report_error(working_set, error); +} + +pub fn report_parse_warning(working_set: &StateWorkingSet, error: &ParseWarning) { + report_warning(working_set, error); +} + +pub fn report_compile_error(working_set: &StateWorkingSet, error: &CompileError) { + report_error(working_set, error); +} + +fn report_error( working_set: &StateWorkingSet, error: &(dyn miette::Diagnostic + Send + Sync + 'static), ) { @@ -39,15 +60,7 @@ pub fn report_error( } } -pub fn report_error_new( - engine_state: &EngineState, - error: &(dyn miette::Diagnostic + Send + Sync + 'static), -) { - let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, error); -} - -pub fn report_warning( +fn report_warning( working_set: &StateWorkingSet, error: &(dyn miette::Diagnostic + Send + Sync + 'static), ) { @@ -59,14 +72,6 @@ pub fn report_warning( } } -pub fn report_warning_new( - engine_state: &EngineState, - error: &(dyn miette::Diagnostic + Send + Sync + 'static), -) { - let working_set = StateWorkingSet::new(engine_state); - report_error(&working_set, error); -} - impl std::fmt::Debug for CliError<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let config = self.1.get_config(); diff --git a/crates/nu-protocol/src/errors/mod.rs b/crates/nu-protocol/src/errors/mod.rs index 59369cd54f..17ec6bb813 100644 --- a/crates/nu-protocol/src/errors/mod.rs +++ b/crates/nu-protocol/src/errors/mod.rs @@ -6,7 +6,8 @@ mod parse_warning; mod shell_error; pub use cli_error::{ - format_error, report_error, report_error_new, report_warning, report_warning_new, + format_shell_error, report_parse_error, report_parse_warning, report_shell_error, + report_shell_warning, }; pub use compile_error::CompileError; pub use labeled_error::{ErrorLabel, LabeledError}; diff --git a/crates/nu-protocol/src/errors/shell_error.rs b/crates/nu-protocol/src/errors/shell_error.rs index b4224ae05c..0784c91fb4 100644 --- a/crates/nu-protocol/src/errors/shell_error.rs +++ b/crates/nu-protocol/src/errors/shell_error.rs @@ -1,11 +1,11 @@ use miette::Diagnostic; use serde::{Deserialize, Serialize}; -use std::io; +use std::{io, num::NonZeroI32}; use thiserror::Error; use crate::{ - ast::Operator, engine::StateWorkingSet, format_error, LabeledError, ParseError, Span, Spanned, - Value, + ast::Operator, engine::StateWorkingSet, format_shell_error, record, LabeledError, ParseError, + Span, Spanned, Value, }; /// The fundamental error type for the evaluation engine. These cases represent different kinds of errors @@ -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 unsupported body input was used for the respective application body type in 'http' command /// /// ## Resolution @@ -885,21 +928,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 @@ -1285,6 +1313,16 @@ This is an internal Nushell error, please file an issue https://github.com/nushe span: Span, }, + #[error("Deprecated: {old_command}")] + #[diagnostic(help("for more info see {url}"))] + Deprecated { + old_command: String, + new_suggestion: String, + #[label("`{old_command}` is deprecated and will be removed in a future release. Please {new_suggestion} instead.")] + span: Span, + url: String, + }, + /// Invalid glob pattern /// /// ## Resolution @@ -1404,10 +1442,41 @@ On Windows, this would be %USERPROFILE%\AppData\Roaming"# }, } -// TODO: Implement as From trait impl ShellError { + pub fn external_exit_code(&self) -> Option> { + let (item, span) = match *self { + Self::NonZeroExitCode { exit_code, span } => (exit_code.into(), span), + #[cfg(unix)] + 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) + } + + pub fn into_value(self, span: Span) -> Value { + let exit_code = self.external_exit_code(); + + let mut record = record! { + "msg" => Value::string(self.to_string(), span), + "debug" => Value::string(format!("{self:?}"), span), + "raw" => Value::error(self, span), + }; + + if let Some(code) = exit_code { + record.push("exit_code", Value::int(code.item.into(), code.span)); + } + + Value::record(record, span) + } + + // TODO: Implement as From trait pub fn wrap(self, working_set: &StateWorkingSet, span: Span) -> ParseError { - let msg = format_error(working_set, &self); + let msg = format_shell_error(working_set, &self); ParseError::LabeledError( msg, "Encountered error during parse-time evaluation".into(), diff --git a/crates/nu-protocol/src/ir/display.rs b/crates/nu-protocol/src/ir/display.rs index 0026e15eab..af521e162f 100644 --- a/crates/nu-protocol/src/ir/display.rs +++ b/crates/nu-protocol/src/ir/display.rs @@ -255,9 +255,6 @@ impl<'a> fmt::Display for FmtInstruction<'a> { Instruction::PopErrorHandler => { write!(f, "{:WIDTH$}", "pop-error-handler") } - Instruction::CheckExternalFailed { dst, src } => { - write!(f, "{:WIDTH$} {dst}, {src}", "check-external-failed") - } Instruction::ReturnEarly { src } => { write!(f, "{:WIDTH$} {src}", "return-early") } diff --git a/crates/nu-protocol/src/ir/mod.rs b/crates/nu-protocol/src/ir/mod.rs index bdded28c0e..214632b44d 100644 --- a/crates/nu-protocol/src/ir/mod.rs +++ b/crates/nu-protocol/src/ir/mod.rs @@ -252,9 +252,6 @@ pub enum Instruction { /// Pop an error handler. This is not necessary when control flow is directed to the error /// handler due to an error. PopErrorHandler, - /// Check if an external command failed. Boolean value into `dst`. `src` is preserved, but it - /// does require waiting for the command to exit. - CheckExternalFailed { dst: RegId, src: RegId }, /// Return early from the block, raising a `ShellError::Return` instead. /// /// Collecting the value is unavoidable. @@ -330,7 +327,6 @@ impl Instruction { Instruction::OnError { .. } => None, Instruction::OnErrorInto { .. } => None, Instruction::PopErrorHandler => None, - Instruction::CheckExternalFailed { dst, .. } => Some(dst), Instruction::ReturnEarly { .. } => None, Instruction::Return { .. } => None, } diff --git a/crates/nu-protocol/src/pipeline/byte_stream.rs b/crates/nu-protocol/src/pipeline/byte_stream.rs index bd384e88c2..6811b7b89b 100644 --- a/crates/nu-protocol/src/pipeline/byte_stream.rs +++ b/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}; @@ -548,25 +548,19 @@ impl ByteStream { } /// Consume and drop all bytes of the [`ByteStream`]. - /// - /// If the source of the [`ByteStream`] is [`ByteStreamSource::Child`], - /// then the [`ExitStatus`] of the [`ChildProcess`] is returned. - pub fn drain(self) -> Result, 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::Child(child) => Ok(Some(child.wait()?)), + ByteStreamSource::File(_) => Ok(()), + ByteStreamSource::Child(child) => child.wait(), } } /// Print all bytes of the [`ByteStream`] to stdout or stderr. - /// - /// 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, ShellError> { + pub fn print(self, to_stderr: bool) -> Result<(), ShellError> { if to_stderr { self.write_to(&mut io::stderr()) } else { @@ -575,20 +569,15 @@ impl ByteStream { } /// Write all bytes of the [`ByteStream`] to `dest`. - /// - /// 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, 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 +595,33 @@ impl ByteStream { } } } - Ok(Some(child.wait()?)) + child.wait()?; } } + Ok(()) } pub(crate) fn write_to_out_dests( self, stdout: &OutDest, stderr: &OutDest, - ) -> Result, 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) => { - match stdout { - OutDest::Pipe | OutDest::Capture | OutDest::Null => {} - OutDest::Inherit => { - copy_with_signals(file, io::stdout(), span, signals)?; - } - OutDest::File(f) => { - copy_with_signals(file, f.as_ref(), span, signals)?; - } + ByteStreamSource::File(file) => match stdout { + OutDest::Pipe | OutDest::Capture | OutDest::Null => {} + OutDest::Inherit => { + copy_with_signals(file, io::stdout(), 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 +668,11 @@ impl ByteStream { } (None, None) => {} } - Ok(Some(child.wait()?)) + child.wait()?; } } + + Ok(()) } } diff --git a/crates/nu-protocol/src/pipeline/pipeline_data.rs b/crates/nu-protocol/src/pipeline/pipeline_data.rs index 85789b4f37..da0fea3213 100644 --- a/crates/nu-protocol/src/pipeline/pipeline_data.rs +++ b/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, ErrSpan, ListStream, OutDest, PipelineMetadata, Range, - ShellError, Signals, Span, Type, Value, + ByteStream, ByteStreamType, Config, ListStream, OutDest, PipelineMetadata, Range, 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 { match self { PipelineData::Empty => None, @@ -182,22 +171,17 @@ impl PipelineData { /// without consuming input and without writing anything. /// /// For the other [`OutDest`]s, the given `PipelineData` will be completely consumed - /// and `PipelineData::Empty` will be returned, unless the data is from an external stream, - /// in which case an external stream containing only that exit code will be returned. + /// and `PipelineData::Empty` will be returned (assuming no errors). pub fn write_to_out_dests( self, engine_state: &EngineState, stack: &mut Stack, ) -> Result { 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) => { @@ -227,15 +211,12 @@ impl PipelineData { Ok(PipelineData::Empty) } - pub fn drain(self) -> Result, 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::ListStream(stream, ..) => { - stream.drain()?; - Ok(None) - } + PipelineData::Value(..) => Ok(()), + PipelineData::ListStream(stream, ..) => stream.drain(), PipelineData::ByteStream(stream, ..) => stream.drain(), } } @@ -496,68 +477,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`. @@ -613,7 +532,7 @@ impl PipelineData { stack: &mut Stack, no_newline: bool, to_stderr: bool, - ) -> Result, ShellError> { + ) -> Result<(), ShellError> { match self { // Print byte streams directly as long as they aren't binary. PipelineData::ByteStream(stream, ..) if stream.type_() != ByteStreamType::Binary => { @@ -650,14 +569,14 @@ impl PipelineData { engine_state: &EngineState, no_newline: bool, to_stderr: bool, - ) -> Result, ShellError> { + ) -> Result<(), ShellError> { if let PipelineData::Value(Value::Binary { val: bytes, .. }, _) = self { if to_stderr { stderr_write_all_and_flush(bytes)?; } else { stdout_write_all_and_flush(bytes)?; } - Ok(None) + Ok(()) } else { self.write_all_and_flush(engine_state, no_newline, to_stderr) } @@ -668,7 +587,7 @@ impl PipelineData { engine_state: &EngineState, no_newline: bool, to_stderr: bool, - ) -> Result, ShellError> { + ) -> Result<(), ShellError> { if let PipelineData::ByteStream(stream, ..) = self { // Copy ByteStreams directly stream.print(to_stderr) @@ -692,7 +611,7 @@ impl PipelineData { } } - Ok(None) + Ok(()) } } diff --git a/crates/nu-protocol/src/process/child.rs b/crates/nu-protocol/src/process/child.rs index 08da7e704c..320427a819 100644 --- a/crates/nu-protocol/src/process/child.rs +++ b/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 { - signal, - core_dumped: true, - } = status - { - return Err(ShellError::CoredumpErrorSpanned { - msg: format!("coredump detected. received signal: {signal}"), - signal, - span, - }); - } + #[cfg(unix)] + Ok(Ok( + status @ ExitStatus::Signaled { + core_dumped: true, .. + }, + )) => { + status.check_ok(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)?; + self.exit_status.wait(self.span)?.check_ok(self.span)?; Ok(bytes) } - pub fn wait(mut self) -> Result { + 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, ShellError> { diff --git a/crates/nu-protocol/src/process/exit_status.rs b/crates/nu-protocol/src/process/exit_status.rs index 8f3794c44f..0ea6b0e72b 100644 --- a/crates/nu-protocol/src/process/exit_status.rs +++ b/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)] diff --git a/crates/nu-protocol/tests/test_config.rs b/crates/nu-protocol/tests/test_config.rs index bee944d0a2..ed44b36581 100644 --- a/crates/nu-protocol/tests/test_config.rs +++ b/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}; null }}")); 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}; null }}")); assert_eq!( actual.err, diff --git a/crates/nu-std/src/lib.rs b/crates/nu-std/src/lib.rs index ff6910e4ba..9944d9bf21 100644 --- a/crates/nu-std/src/lib.rs +++ b/crates/nu-std/src/lib.rs @@ -5,7 +5,7 @@ use nu_parser::parse; use nu_protocol::{ debugger::WithoutDebug, engine::{FileStack, Stack, StateWorkingSet, VirtualPath}, - report_error, PipelineData, + report_parse_error, PipelineData, }; use std::path::PathBuf; @@ -85,7 +85,7 @@ use std pwd working_set.files.pop(); if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); } (block, working_set.render()) diff --git a/crates/nu-utils/src/sample_config/default_config.nu b/crates/nu-utils/src/sample_config/default_config.nu index 2165dc5288..1498423c45 100644 --- a/crates/nu-utils/src/sample_config/default_config.nu +++ b/crates/nu-utils/src/sample_config/default_config.nu @@ -174,6 +174,14 @@ $env.config = { error_style: "fancy" # "fancy" or "plain" for screen reader-friendly error messages + # Whether an error message should be printed if an error of a certain kind is triggered. + display_errors: { + exit_code: false # assume the external command prints an error message + # Core dump errors are always printed, and SIGPIPE never triggers an error. + # The setting below controls message printing for termination by all other signals. + termination_signal: true + } + # datetime_format determines what a datetime rendered in the shell would look like. # Behavior without this configuration point will be to "humanize" the datetime display, # showing something like "a day ago." diff --git a/src/command.rs b/src/command.rs index 57ecd98edc..2ba817ac6b 100644 --- a/src/command.rs +++ b/src/command.rs @@ -3,7 +3,7 @@ use nu_parser::{escape_for_script_arg, escape_quote_string, parse}; use nu_protocol::{ ast::{Expr, Expression}, engine::StateWorkingSet, - report_error, + report_parse_error, }; use nu_utils::stdout_write_all_and_flush; @@ -68,7 +68,7 @@ pub(crate) fn parse_commandline_args( let output = parse(&mut working_set, None, commandline_args.as_bytes(), false); if let Some(err) = working_set.parse_errors.first() { - report_error(&working_set, err); + report_parse_error(&working_set, err); std::process::exit(1); } diff --git a/src/config_files.rs b/src/config_files.rs index 565cd0579a..e2d0af8131 100644 --- a/src/config_files.rs +++ b/src/config_files.rs @@ -5,7 +5,7 @@ use nu_cli::{eval_config_contents, eval_source}; use nu_path::canonicalize_with; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, - report_error, report_error_new, Config, ParseError, PipelineData, Spanned, + report_parse_error, report_shell_error, Config, ParseError, PipelineData, Spanned, }; use nu_utils::{get_default_config, get_default_env}; use std::{ @@ -34,19 +34,17 @@ pub(crate) fn read_config_file( ); // Load config startup file if let Some(file) = config_file { - let working_set = StateWorkingSet::new(engine_state); - match engine_state.cwd_as_string(Some(stack)) { Ok(cwd) => { if let Ok(path) = canonicalize_with(&file.item, cwd) { eval_config_contents(path, engine_state, stack); } else { let e = ParseError::FileNotFound(file.item, file.span); - report_error(&working_set, &e); + report_parse_error(&StateWorkingSet::new(engine_state), &e); } } Err(e) => { - report_error(&working_set, &e); + report_shell_error(engine_state, &e); } } } else if let Some(mut config_path) = nu_path::config_dir() { @@ -168,11 +166,11 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut match engine_state.cwd(Some(stack)) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } Err(e) => { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } } @@ -254,11 +252,11 @@ fn eval_default_config( match engine_state.cwd(Some(stack)) { Ok(cwd) => { if let Err(e) = engine_state.merge_env(stack, cwd) { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } Err(e) => { - report_error_new(engine_state, &e); + report_shell_error(engine_state, &e); } } } diff --git a/src/ide.rs b/src/ide.rs index 948539d839..85046ef0d7 100644 --- a/src/ide.rs +++ b/src/ide.rs @@ -3,7 +3,7 @@ use nu_cli::NuCompleter; use nu_parser::{flatten_block, parse, FlatShape}; use nu_protocol::{ engine::{EngineState, Stack, StateWorkingSet}, - report_error_new, DeclId, ShellError, Span, Value, VarId, + report_shell_error, DeclId, ShellError, Span, Value, VarId, }; use reedline::Completer; use serde_json::{json, Value as JsonValue}; @@ -55,7 +55,7 @@ fn read_in_file<'a>( let file = std::fs::read(file_path) .into_diagnostic() .unwrap_or_else(|e| { - report_error_new( + report_shell_error( engine_state, &ShellError::FileNotFoundCustom { msg: format!("Could not read file '{}': {:?}", file_path, e.to_string()), diff --git a/src/main.rs b/src/main.rs index a5c5c5d8ef..303fb50e9f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -25,7 +25,7 @@ use nu_cmd_base::util::get_init_cwd; use nu_lsp::LanguageServer; use nu_path::canonicalize_with; use nu_protocol::{ - engine::EngineState, report_error_new, ByteStream, PipelineData, ShellError, Span, Spanned, + engine::EngineState, report_shell_error, ByteStream, PipelineData, ShellError, Span, Spanned, Value, }; use nu_std::load_standard_library; @@ -67,7 +67,7 @@ fn main() -> Result<()> { }; if let Err(err) = engine_state.merge_delta(delta) { - report_error_new(&engine_state, &err); + report_shell_error(&engine_state, &err); } // TODO: make this conditional in the future @@ -92,7 +92,7 @@ fn main() -> Result<()> { .unwrap_or(PathBuf::from(&xdg_config_home)) .join("nushell") { - report_error_new( + report_shell_error( &engine_state, &ShellError::InvalidXdgConfig { xdg: xdg_config_home, @@ -164,7 +164,7 @@ fn main() -> Result<()> { let (args_to_nushell, script_name, args_to_script) = gather_commandline_args(); let parsed_nu_cli_args = parse_commandline_args(&args_to_nushell.join(" "), &mut engine_state) .unwrap_or_else(|err| { - report_error_new(&engine_state, &err); + report_shell_error(&engine_state, &err); std::process::exit(1) }); diff --git a/src/run.rs b/src/run.rs index 10a5043b25..89e70b8f0f 100644 --- a/src/run.rs +++ b/src/run.rs @@ -10,7 +10,7 @@ use nu_cli::read_plugin_file; use nu_cli::{evaluate_commands, evaluate_file, evaluate_repl, EvaluateCommandsOpts}; use nu_protocol::{ engine::{EngineState, Stack}, - report_error_new, PipelineData, Spanned, + report_shell_error, PipelineData, Spanned, }; use nu_utils::perf; @@ -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_shell_error(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(); - let last_exit_code = stack.get_env_var(&*engine_state, "LAST_EXIT_CODE"); - if let Some(last_exit_code) = last_exit_code { - let value = last_exit_code.as_int(); - if let Ok(value) = value { - if value != 0 { - std::process::exit(value as i32); - } - } + if let Err(err) = result { + report_shell_error(engine_state, &err); + std::process::exit(err.exit_code()); } - perf!("get exit code", start_time, use_color); } pub(crate) fn run_repl( diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs index 5b9f116626..9a19f4b3a7 100644 --- a/tests/shell/pipeline/commands/external.rs +++ b/tests/shell/pipeline/commands/external.rs @@ -182,13 +182,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| { @@ -626,3 +619,24 @@ mod external_command_arguments { assert_eq!(actual.out, r#"expression=-r\" -w"#); } } + +#[test] +fn exit_code_stops_execution_closure() { + let actual = nu!("[1 2] | each {|x| nu -c $'exit ($x)'; print $x }"); + assert!(actual.out.is_empty()); + assert!(actual.err.contains("exited with code 1")); +} + +#[test] +fn exit_code_stops_execution_custom_command() { + let actual = nu!("def cmd [] { nu -c 'exit 42'; 'ok1' }; cmd; print 'ok2'"); + assert!(actual.out.is_empty()); + assert!(actual.err.contains("exited with code 42")); +} + +#[test] +fn exit_code_stops_execution_for_loop() { + let actual = nu!("for x in [0 1] { nu -c 'exit 42'; print $x }"); + assert!(actual.out.is_empty()); + assert!(actual.err.contains("exited with code 42")); +}