diff --git a/src/cli.rs b/src/cli.rs index f8250ebea..0691fc0f9 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -61,16 +61,16 @@ fn load_plugin(path: &std::path::Path, context: &mut Context) -> Result<(), Shel let name = params.name.clone(); let fname = fname.to_string(); - if context.get_command(&name).is_some() { + if context.get_command(&name)?.is_some() { trace!("plugin {:?} already loaded.", &name); } else if params.is_filter { - context.add_commands(vec![whole_stream_command(PluginCommand::new( - name, fname, params, - ))]); + context.add_commands(vec![whole_stream_command( + PluginCommand::new(name, fname, params), + )])?; } else { context.add_commands(vec![whole_stream_command(PluginSink::new( name, fname, params, - ))]); + ))])?; } Ok(()) } @@ -328,7 +328,7 @@ pub async fn cli() -> Result<(), Box> { whole_stream_command(FromXML), whole_stream_command(FromYAML), whole_stream_command(FromYML), - ]); + ])?; cfg_if::cfg_if! { if #[cfg(data_processing_primitives)] { @@ -337,7 +337,7 @@ pub async fn cli() -> Result<(), Box> { whole_stream_command(EvaluateBy), whole_stream_command(TSortBy), whole_stream_command(MapMaxBy), - ]); + ])?; } } @@ -345,7 +345,7 @@ pub async fn cli() -> Result<(), Box> { { context.add_commands(vec![whole_stream_command( crate::commands::clip::clipboard::Clip, - )]); + )])?; } } diff --git a/src/commands/autoview.rs b/src/commands/autoview.rs index b81a34353..0b728ea45 100644 --- a/src/commands/autoview.rs +++ b/src/commands/autoview.rs @@ -70,7 +70,7 @@ pub fn autoview( } } }; - if let Some(table) = table { + if let Some(table) = table? { let mut new_output_stream: OutputStream = stream.to_output_stream(); let mut finished = false; let mut current_idx = 0; @@ -136,7 +136,7 @@ pub fn autoview( value: UntaggedValue::Primitive(Primitive::String(ref s)), tag: Tag { anchor, span }, } if anchor.is_some() => { - if let Some(text) = text { + if let Some(text) = text? { let mut stream = VecDeque::new(); stream.push_back(UntaggedValue::string(s).into_value(Tag { anchor, span })); let result = text.run(raw.with_input(stream.into()), &context.commands); @@ -155,7 +155,7 @@ pub fn autoview( value: UntaggedValue::Primitive(Primitive::Line(ref s)), tag: Tag { anchor, span }, } if anchor.is_some() => { - if let Some(text) = text { + if let Some(text) = text? { let mut stream = VecDeque::new(); stream.push_back(UntaggedValue::string(s).into_value(Tag { anchor, span })); let result = text.run(raw.with_input(stream.into()), &context.commands); @@ -190,7 +190,7 @@ pub fn autoview( } Value { value: UntaggedValue::Primitive(Primitive::Binary(ref b)), .. } => { - if let Some(binary) = binary { + if let Some(binary) = binary? { let mut stream = VecDeque::new(); stream.push_back(x); let result = binary.run(raw.with_input(stream.into()), &context.commands); @@ -205,7 +205,7 @@ pub fn autoview( yield Err(e); } Value { value: ref item, .. } => { - if let Some(table) = table { + if let Some(table) = table? { let mut stream = VecDeque::new(); stream.push_back(x); let result = table.run(raw.with_input(stream.into()), &context.commands); diff --git a/src/commands/classified/internal.rs b/src/commands/classified/internal.rs index 46b9da251..0fe5d6d41 100644 --- a/src/commands/classified/internal.rs +++ b/src/commands/classified/internal.rs @@ -57,7 +57,7 @@ pub(crate) async fn run_internal_command( let contents_tag = tagged_contents.tag.clone(); let command_name = format!("from-{}", extension); let command = command.clone(); - if let Some(converter) = context.registry.get_command(&command_name) { + if let Some(converter) = context.registry.get_command(&command_name)? { let new_args = RawCommandArgs { host: context.host.clone(), ctrl_c: context.ctrl_c.clone(), @@ -103,7 +103,7 @@ pub(crate) async fn run_internal_command( HelpShell::for_command( UntaggedValue::string(cmd).into_value(tag), &context.registry(), - ).unwrap(), + )?, )); } _ => { diff --git a/src/commands/command.rs b/src/commands/command.rs index d90e21cec..184c3fee5 100644 --- a/src/commands/command.rs +++ b/src/commands/command.rs @@ -227,7 +227,7 @@ pub struct RunnableContext { } impl RunnableContext { - pub fn get_command(&self, name: &str) -> Option> { + pub fn get_command(&self, name: &str) -> Result>, ShellError> { self.commands.get_command(name) } } diff --git a/src/commands/enter.rs b/src/commands/enter.rs index d49989664..1a0670d25 100644 --- a/src/commands/enter.rs +++ b/src/commands/enter.rs @@ -51,7 +51,7 @@ impl PerItemCommand for Enter { let (_, command) = (spec[0], spec[1]); - if registry.has(command) { + if registry.has(command)? { Ok(vec![Ok(ReturnSuccess::Action(CommandAction::EnterHelpShell( UntaggedValue::string(command).into_value(Tag::unknown()), )))] @@ -88,7 +88,7 @@ impl PerItemCommand for Enter { if let Some(extension) = file_extension { let command_name = format!("from-{}", extension); if let Some(converter) = - registry.get_command(&command_name) + registry.get_command(&command_name)? { let new_args = RawCommandArgs { host: raw_args.host, diff --git a/src/commands/from_ssv.rs b/src/commands/from_ssv.rs index 81ef56e28..476d63afa 100644 --- a/src/commands/from_ssv.rs +++ b/src/commands/from_ssv.rs @@ -391,7 +391,7 @@ mod tests { } #[test] - fn it_trims_remaining_separator_space() { + fn it_trims_remaining_separator_space() -> Result<(), ShellError> { let input = r#" colA colB colC val1 val2 val3 @@ -399,14 +399,17 @@ mod tests { let trimmed = |s: &str| s.trim() == s; - let result = string_to_table(input, false, true, 2).unwrap(); + let result = string_to_table(input, false, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; assert!(result .iter() - .all(|row| row.iter().all(|(a, b)| trimmed(a) && trimmed(b)))) + .all(|row| row.iter().all(|(a, b)| trimmed(a) && trimmed(b)))); + + Ok(()) } #[test] - fn it_keeps_empty_columns() { + fn it_keeps_empty_columns() -> Result<(), ShellError> { let input = r#" colA col B col C val2 val3 @@ -414,7 +417,8 @@ mod tests { val7 val8 "#; - let result = string_to_table(input, false, true, 2).unwrap(); + let result = string_to_table(input, false, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; assert_eq!( result, vec![ @@ -434,35 +438,39 @@ mod tests { owned("col C", "val8") ], ] - ) + ); + Ok(()) } #[test] - fn it_uses_the_full_final_column() { + fn it_uses_the_full_final_column() -> Result<(), ShellError> { let input = r#" colA col B val1 val2 trailing value that should be included "#; - let result = string_to_table(input, false, true, 2).unwrap(); + let result = string_to_table(input, false, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; assert_eq!( result, vec![vec![ owned("colA", "val1"), owned("col B", "val2 trailing value that should be included"), ],] - ) + ); + Ok(()) } #[test] - fn it_handles_empty_values_when_headerless_and_aligned_columns() { + fn it_handles_empty_values_when_headerless_and_aligned_columns() -> Result<(), ShellError> { let input = r#" a multi-word value b d 1 3-3 4 last "#; - let result = string_to_table(input, true, true, 2).unwrap(); + let result = string_to_table(input, true, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; assert_eq!( result, vec![ @@ -488,22 +496,28 @@ mod tests { owned("Column5", "last") ], ] - ) + ); + Ok(()) } #[test] - fn input_is_parsed_correctly_if_either_option_works() { + fn input_is_parsed_correctly_if_either_option_works() -> Result<(), ShellError> { let input = r#" docker-registry docker-registry=default docker-registry=default 172.30.78.158 5000/TCP kubernetes component=apiserver,provider=kubernetes 172.30.0.2 443/TCP kubernetes-ro component=apiserver,provider=kubernetes 172.30.0.1 80/TCP "#; - let aligned_columns_headerless = string_to_table(input, true, true, 2).unwrap(); - let separator_headerless = string_to_table(input, true, false, 2).unwrap(); - let aligned_columns_with_headers = string_to_table(input, false, true, 2).unwrap(); - let separator_with_headers = string_to_table(input, false, false, 2).unwrap(); + let aligned_columns_headerless = string_to_table(input, true, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; + let separator_headerless = string_to_table(input, true, false, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; + let aligned_columns_with_headers = string_to_table(input, false, true, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; + let separator_with_headers = string_to_table(input, false, false, 2) + .ok_or_else(|| ShellError::unexpected("table couldn't be parsed"))?; assert_eq!(aligned_columns_headerless, separator_headerless); assert_eq!(aligned_columns_with_headers, separator_with_headers); + Ok(()) } } diff --git a/src/commands/help.rs b/src/commands/help.rs index 888f451bf..17b63efce 100644 --- a/src/commands/help.rs +++ b/src/commands/help.rs @@ -40,12 +40,12 @@ impl PerItemCommand for Help { }) => { let mut help = VecDeque::new(); if document == "commands" { - let mut sorted_names = registry.names(); + let mut sorted_names = registry.names()?; sorted_names.sort(); for cmd in sorted_names { let mut short_desc = TaggedDictBuilder::new(tag.clone()); let value = command_dict( - registry.get_command(&cmd).ok_or_else(|| { + registry.get_command(&cmd)?.ok_or_else(|| { ShellError::labeled_error( format!("Could not load {}", cmd), "could not load command", @@ -71,7 +71,7 @@ impl PerItemCommand for Help { help.push_back(ReturnSuccess::value(short_desc.into_value())); } - } else if let Some(command) = registry.get_command(document) { + } else if let Some(command) = registry.get_command(document)? { let mut long_desc = String::new(); long_desc.push_str(&command.usage()); diff --git a/src/commands/plugin.rs b/src/commands/plugin.rs index 9bb693bd0..31ee17797 100644 --- a/src/commands/plugin.rs +++ b/src/commands/plugin.rs @@ -106,14 +106,26 @@ pub fn filter_plugin( let mut reader = BufReader::new(stdout); let request = JsonRpc::new("begin_filter", call_info.clone()); - let request_raw = serde_json::to_string(&request).unwrap(); - match stdin.write(format!("{}\n", request_raw).as_bytes()) { - Ok(_) => {} - Err(err) => { + let request_raw = serde_json::to_string(&request); + + match request_raw { + Err(_) => { let mut result = VecDeque::new(); - result.push_back(Err(ShellError::unexpected(format!("{}", err)))); + result.push_back(Err(ShellError::labeled_error( + "Could not load json from plugin", + "could not load json from plugin", + &call_info.name_tag, + ))); return result; } + Ok(request_raw) => match stdin.write(format!("{}\n", request_raw).as_bytes()) { + Ok(_) => {} + Err(err) => { + let mut result = VecDeque::new(); + result.push_back(Err(ShellError::unexpected(format!("{}", err)))); + return result; + } + }, } let mut input = String::new(); @@ -179,8 +191,20 @@ pub fn filter_plugin( Ok(params) => { let request: JsonRpc> = JsonRpc::new("quit", vec![]); - let request_raw = serde_json::to_string(&request).unwrap(); - let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error + let request_raw = serde_json::to_string(&request); + match request_raw { + Ok(request_raw) => { + let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error + } + Err(e) => { + let mut result = VecDeque::new(); + result.push_back(Err(ShellError::untagged_runtime_error(format!( + "Error while processing begin_filter response: {:?} {}", + e, input + )))); + return result; + } + } params } @@ -221,8 +245,20 @@ pub fn filter_plugin( let mut reader = BufReader::new(stdout); let request = JsonRpc::new("filter", v); - let request_raw = serde_json::to_string(&request).unwrap(); - let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error + let request_raw = serde_json::to_string(&request); + match request_raw { + Ok(request_raw) => { + let _ = stdin.write(format!("{}\n", request_raw).as_bytes()); // TODO: Handle error + } + Err(e) => { + let mut result = VecDeque::new(); + result.push_back(Err(ShellError::untagged_runtime_error(format!( + "Error while processing filter response: {:?}", + e + )))); + return result; + } + } let mut input = String::new(); match reader.read_line(&mut input) { @@ -304,21 +340,31 @@ pub fn sink_plugin( let input: Vec = args.input.values.collect().await; let request = JsonRpc::new("sink", (call_info.clone(), input)); - let request_raw = serde_json::to_string(&request).unwrap(); - let mut tmpfile = tempfile::NamedTempFile::new().unwrap(); - let _ = writeln!(tmpfile, "{}", request_raw); - let _ = tmpfile.flush(); + let request_raw = serde_json::to_string(&request); + if let Ok(request_raw) = request_raw { + if let Ok(mut tmpfile) = tempfile::NamedTempFile::new() { + let _ = writeln!(tmpfile, "{}", request_raw); + let _ = tmpfile.flush(); - let mut child = std::process::Command::new(path) - .arg(tmpfile.path()) - .spawn() - .expect("Failed to spawn child process"); + let mut child = std::process::Command::new(path) + .arg(tmpfile.path()) + .spawn(); - let _ = child.wait(); + if let Ok(mut child) = child { + let _ = child.wait(); - // Needed for async_stream to type check - if false { - yield ReturnSuccess::value(UntaggedValue::nothing().into_untagged_value()); + // Needed for async_stream to type check + if false { + yield ReturnSuccess::value(UntaggedValue::nothing().into_untagged_value()); + } + } else { + yield Err(ShellError::untagged_runtime_error("Could not create process for sink command")); + } + } else { + yield Err(ShellError::untagged_runtime_error("Could not open file to send sink command message")); + } + } else { + yield Err(ShellError::untagged_runtime_error("Could not create message to sink command")); } }; Ok(OutputStream::new(stream)) diff --git a/src/commands/save.rs b/src/commands/save.rs index 1c1d06cae..85b8314c2 100644 --- a/src/commands/save.rs +++ b/src/commands/save.rs @@ -179,7 +179,7 @@ fn save( break if !save_raw { if let Some(extension) = full_path.extension() { let command_name = format!("to-{}", extension.to_string_lossy()); - if let Some(converter) = registry.get_command(&command_name) { + if let Some(converter) = registry.get_command(&command_name)? { let new_args = RawCommandArgs { host, ctrl_c, diff --git a/src/commands/which_.rs b/src/commands/which_.rs index 012b52a8f..dc8b80978 100644 --- a/src/commands/which_.rs +++ b/src/commands/which_.rs @@ -95,7 +95,7 @@ fn which( } } - let builtin = commands.has(&item); + let builtin = commands.has(&item)?; if builtin { yield ReturnSuccess::value(entry_builtin!(item, application.tag.clone())); } @@ -128,7 +128,7 @@ fn which( if let Ok(path) = ichwh::which(&item).await { yield ReturnSuccess::value(entry_path!(item, path.into(), application.tag.clone())); } - } else if commands.has(&item) { + } else if commands.has(&item)? { yield ReturnSuccess::value(entry_builtin!(item, application.tag.clone())); } else if let Ok(path) = ichwh::which(&item).await { yield ReturnSuccess::value(entry_path!(item, path.into(), application.tag.clone())); diff --git a/src/context.rs b/src/context.rs index 73645a2b1..040cc8dc1 100644 --- a/src/context.rs +++ b/src/context.rs @@ -54,32 +54,46 @@ impl CommandRegistry { } } - pub(crate) fn get_command(&self, name: &str) -> Option> { - let registry = self.registry.lock().unwrap(); + pub(crate) fn get_command(&self, name: &str) -> Result>, ShellError> { + let registry = self.registry.lock().map_err(|_| { + ShellError::untagged_runtime_error("Internal error: get_command could not get mutex") + })?; - registry.get(name).cloned() + Ok(registry.get(name).cloned()) } pub(crate) fn expect_command(&self, name: &str) -> Result, ShellError> { - self.get_command(name).ok_or_else(|| { + self.get_command(name)?.ok_or_else(|| { ShellError::untagged_runtime_error(format!("Could not load command: {}", name)) }) } - pub(crate) fn has(&self, name: &str) -> bool { - let registry = self.registry.lock().unwrap(); + pub(crate) fn has(&self, name: &str) -> Result { + let registry = self.registry.lock().map_err(|_| { + ShellError::untagged_runtime_error("Internal error: has could not get mutex") + })?; - registry.contains_key(name) + Ok(registry.contains_key(name)) } - pub(crate) fn insert(&mut self, name: impl Into, command: Arc) { - let mut registry = self.registry.lock().unwrap(); + pub(crate) fn insert( + &mut self, + name: impl Into, + command: Arc, + ) -> Result<(), ShellError> { + let mut registry = self.registry.lock().map_err(|_| { + ShellError::untagged_runtime_error("Internal error: insert could not get mutex") + })?; + registry.insert(name.into(), command); + Ok(()) } - pub(crate) fn names(&self) -> Vec { - let registry = self.registry.lock().unwrap(); - registry.keys().cloned().collect() + pub(crate) fn names(&self) -> Result, ShellError> { + let registry = self.registry.lock().map_err(|_| { + ShellError::untagged_runtime_error("Internal error: names could not get mutex") + })?; + Ok(registry.keys().cloned().collect()) } } @@ -175,13 +189,15 @@ impl Context { block(&mut *errors) } - pub fn add_commands(&mut self, commands: Vec>) { + pub fn add_commands(&mut self, commands: Vec>) -> Result<(), ShellError> { for command in commands { - self.registry.insert(command.name().to_string(), command); + self.registry.insert(command.name().to_string(), command)?; } + + Ok(()) } - pub(crate) fn get_command(&self, name: &str) -> Option> { + pub(crate) fn get_command(&self, name: &str) -> Result>, ShellError> { self.registry.get_command(name) } diff --git a/src/data/base.rs b/src/data/base.rs index ad90a9e39..d9c88641d 100644 --- a/src/data/base.rs +++ b/src/data/base.rs @@ -43,7 +43,7 @@ impl EvaluateTrait for Block { return Ok(UntaggedValue::nothing().into_value(&self.tag)); } - let mut last = None; + let mut last = Ok(UntaggedValue::nothing().into_value(&self.tag)); trace!( "EXPRS = {:?}", @@ -54,15 +54,10 @@ impl EvaluateTrait for Block { ); for expr in self.expressions.iter() { - last = Some(evaluate_baseline_expr( - &expr, - &CommandRegistry::empty(), - &scope, - &self.source, - )?) + last = evaluate_baseline_expr(&expr, &CommandRegistry::empty(), &scope, &self.source) } - Ok(last.unwrap()) + last } fn clone_box(&self) -> Evaluate { @@ -382,7 +377,7 @@ mod tests { let actual = sample .into_untagged_value() .replace_data_at_column_path(&field_path?.item, replacement) - .unwrap(); + .ok_or_else(|| ShellError::untagged_runtime_error("Could not replace column"))?; assert_eq!(actual, row(indexmap! {"amigos".into() => string("jonas")})); @@ -411,9 +406,15 @@ mod tests { let tag = replacement.tag.clone(); let actual = sample - .into_value(tag.clone()) + .into_value(&tag) .replace_data_at_column_path(&field_path?.item, replacement.clone()) - .unwrap(); + .ok_or_else(|| { + ShellError::labeled_error( + "Could not replace column", + "could not replace column", + &tag, + ) + })?; assert_eq!( actual, @@ -467,7 +468,13 @@ mod tests { let actual = sample .into_value(tag.clone()) .replace_data_at_column_path(&field_path?.item, replacement.clone()) - .unwrap(); + .ok_or_else(|| { + ShellError::labeled_error( + "Could not replace column", + "could not replace column", + &tag, + ) + })?; assert_eq!( actual, diff --git a/src/lib.rs b/src/lib.rs index 30de3cbe8..61a12f888 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,4 @@ -#![recursion_limit = "1024"] +#![recursion_limit = "2048"] #[cfg(test)] #[macro_use] diff --git a/src/shell/completer.rs b/src/shell/completer.rs index 84627568b..a4bed043f 100644 --- a/src/shell/completer.rs +++ b/src/shell/completer.rs @@ -17,7 +17,7 @@ impl NuCompleter { pos: usize, context: &rustyline::Context, ) -> rustyline::Result<(usize, Vec)> { - let commands: Vec = self.commands.names(); + let commands: Vec = self.commands.names().unwrap_or_else(|_| vec![]); let line_chars: Vec<_> = line[..pos].chars().collect(); diff --git a/src/shell/filesystem_shell.rs b/src/shell/filesystem_shell.rs index bd0bbf77e..ead246d37 100644 --- a/src/shell/filesystem_shell.rs +++ b/src/shell/filesystem_shell.rs @@ -1047,7 +1047,13 @@ impl Shell for FilesystemShell { } if trash.item { - SendToTrash::remove(path).unwrap(); + SendToTrash::remove(path).map_err(|_| { + ShellError::labeled_error( + "Could not move file to trash", + "could not move to trash", + target.tag(), + ) + })?; } else if path.is_dir() { std::fs::remove_dir_all(&path)?; } else if path.is_file() { diff --git a/src/shell/help_shell.rs b/src/shell/help_shell.rs index d615629ae..46e315351 100644 --- a/src/shell/help_shell.rs +++ b/src/shell/help_shell.rs @@ -21,13 +21,13 @@ pub struct HelpShell { } impl HelpShell { - pub fn index(registry: &CommandRegistry) -> Result { + pub fn index(registry: &CommandRegistry) -> Result { let mut cmds = TaggedDictBuilder::new(Tag::unknown()); let mut specs = Vec::new(); - for cmd in registry.names() { + for cmd in registry.names()? { let mut spec = TaggedDictBuilder::new(Tag::unknown()); - let value = command_dict(registry.get_command(&cmd).unwrap(), Tag::unknown()); + let value = command_dict(registry.get_command(&cmd)?.unwrap(), Tag::unknown()); spec.insert_untagged("name", cmd); spec.insert_untagged( @@ -51,10 +51,7 @@ impl HelpShell { }) } - pub fn for_command( - cmd: Value, - registry: &CommandRegistry, - ) -> Result { + pub fn for_command(cmd: Value, registry: &CommandRegistry) -> Result { let mut sh = HelpShell::index(®istry)?; if let Value { diff --git a/tests/commands/cd.rs b/tests/commands/cd.rs index 7bfcabc40..24ac6eb2a 100644 --- a/tests/commands/cd.rs +++ b/tests/commands/cd.rs @@ -96,7 +96,7 @@ fn filesystem_change_to_home_directory() { "# ); - assert_eq!(PathBuf::from(actual), dirs::home_dir().unwrap()); + assert_eq!(Some(PathBuf::from(actual)), dirs::home_dir()); }) }