From e76586ede420a2bdb12bc868698572a10217b4b4 Mon Sep 17 00:00:00 2001 From: Solomon Date: Wed, 26 Mar 2025 23:41:16 +0000 Subject: [PATCH] reset argument/redirection state after `eval_call` errors (#15400) Closes #15395 # User-Facing Changes Certain errors no longer leave the argument stack in an unexpected state: ```diff let x: any = 1; try { $x | get path } catch { print caught } -$.path # extra `print` argument from the failed `get` call caught ``` # Description If `eval_call` fails in `check_input_types` or `gather_arguments`, the cleanup code is still executed. --- crates/nu-command/tests/commands/try_.rs | 8 ++ crates/nu-engine/src/eval_ir.rs | 100 ++++++++++++----------- 2 files changed, 59 insertions(+), 49 deletions(-) diff --git a/crates/nu-command/tests/commands/try_.rs b/crates/nu-command/tests/commands/try_.rs index 7e6952d2b9..65a36db6e1 100644 --- a/crates/nu-command/tests/commands/try_.rs +++ b/crates/nu-command/tests/commands/try_.rs @@ -82,6 +82,14 @@ fn catch_block_can_use_error_object() { assert_eq!(output.out, "Division by zero.") } +#[test] +fn catch_input_type_mismatch_and_rethrow() { + let actual = nu!( + "let x: any = 1; try { $x | get 1 } catch {|err| error make { msg: ($err | get msg) } }" + ); + assert!(actual.err.contains("Input type not supported")); +} + // This test is disabled on Windows because they cause a stack overflow in CI (but not locally!). // For reasons we don't understand, the Windows CI runners are prone to stack overflow. // TODO: investigate so we can enable on Windows diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 20a0bda94d..6dfa913931 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -1026,63 +1026,65 @@ fn eval_call( // Set up redirect modes let mut caller_stack = caller_stack.push_redirection(redirect_out.take(), redirect_err.take()); - let result; + let result = (|| { + if let Some(block_id) = decl.block_id() { + // If the decl is a custom command + let block = engine_state.get_block(block_id); - if let Some(block_id) = decl.block_id() { - // If the decl is a custom command - let block = engine_state.get_block(block_id); + // check types after acquiring block to avoid unnecessarily cloning Signature + check_input_types(&input, &block.signature, head)?; - // check types after acquiring block to avoid unnecessarily cloning Signature - check_input_types(&input, &block.signature, head)?; + // Set up a callee stack with the captures and move arguments from the stack into variables + let mut callee_stack = caller_stack.gather_captures(engine_state, &block.captures); - // Set up a callee stack with the captures and move arguments from the stack into variables - let mut callee_stack = caller_stack.gather_captures(engine_state, &block.captures); + gather_arguments( + engine_state, + block, + &mut caller_stack, + &mut callee_stack, + *args_base, + args_len, + head, + )?; - gather_arguments( - engine_state, - block, - &mut caller_stack, - &mut callee_stack, - *args_base, - args_len, - head, - )?; + // Add one to the recursion count, so we don't recurse too deep. Stack overflows are not + // recoverable in Rust. + callee_stack.recursion_count += 1; - // Add one to the recursion count, so we don't recurse too deep. Stack overflows are not - // recoverable in Rust. - callee_stack.recursion_count += 1; + let result = + eval_block_with_early_return::(engine_state, &mut callee_stack, block, input); - result = eval_block_with_early_return::(engine_state, &mut callee_stack, block, input); + // Move environment variables back into the caller stack scope if requested to do so + if block.redirect_env { + redirect_env(engine_state, &mut caller_stack, &callee_stack); + } - // Move environment variables back into the caller stack scope if requested to do so - if block.redirect_env { - redirect_env(engine_state, &mut caller_stack, &callee_stack); + result + } else { + check_input_types(&input, &decl.signature(), head)?; + // FIXME: precalculate this and save it somewhere + let span = Span::merge_many( + std::iter::once(head).chain( + caller_stack + .arguments + .get_args(*args_base, args_len) + .iter() + .flat_map(|arg| arg.span()), + ), + ); + + let call = Call { + decl_id, + head, + span, + args_base: *args_base, + args_len, + }; + + // Run the call + decl.run(engine_state, &mut caller_stack, &(&call).into(), input) } - } else { - check_input_types(&input, &decl.signature(), head)?; - - // FIXME: precalculate this and save it somewhere - let span = Span::merge_many( - std::iter::once(head).chain( - caller_stack - .arguments - .get_args(*args_base, args_len) - .iter() - .flat_map(|arg| arg.span()), - ), - ); - - let call = Call { - decl_id, - head, - span, - args_base: *args_base, - args_len, - }; - - // Run the call - result = decl.run(engine_state, &mut caller_stack, &(&call).into(), input); - }; + })(); drop(caller_stack);