Skip to content

Commit

Permalink
Defensive handling of errors when transposing (nushell#14096)
Browse files Browse the repository at this point in the history
# Description
This PR aims to close nushell#14027, in which it was noticed that the transpose
command "swallows" error messages.

*Note that in exploring the linked issue, [other situations were
identified](nushell#14027 (comment))
which also produce inconsistent behaviour. These have knowingly been
omitted from this PR, to minimize its scope, and since they seem to have
a different cause. It's probably best to make a separate issue/PR in
which to tackle a broader scan of error handling, with a suspected
relation to streams.*

# User-Facing Changes
The user will see errors from deeper in the pipeline, in case the errors
originated there.

# Tests + Formatting
Toolkit PR check was run successfully.

One test was added, covering this exact situation, in order to prevent
regressions.
The bug is relatively obscure, so it may be prone to reappear during
refactorings.
  • Loading branch information
PhotonBursted authored Oct 22, 2024
1 parent 3f75b6b commit 9870c7c
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 0 deletions.
6 changes: 6 additions & 0 deletions crates/nu-command/src/filters/transpose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ pub fn transpose(

let metadata = input.metadata();
let input: Vec<_> = input.into_iter().collect();
// Ensure error values are propagated
for i in input.iter() {
if let Value::Error { .. } = i {
return Ok(i.clone().into_pipeline_data_with_metadata(metadata));
}
}

let descs = get_columns(&input);

Expand Down
12 changes: 12 additions & 0 deletions crates/nu-command/tests/commands/transpose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,15 @@ fn row_but_all() {

assert!(actual.out.contains("foo: [1, 2]"));
}

#[test]
fn throw_inner_error() {
let error_msg = "This message should show up";
let error = format!("(error make {{ msg: \"{}\" }})", error_msg);
let actual = nu!(format!(
"[[key value]; [foo 1] [foo 2] [{} 3]] | transpose",
error
));

assert!(actual.err.contains(error.as_str()));
}

0 comments on commit 9870c7c

Please sign in to comment.