Merge pull request #111 from jonathandturner/more_error_checking

Add more error checking

src/cli.rs

@@ -292,7 +292,9 @@ async fn process_line(readline: Result<String, ReadlineError>, ctx: &mut Context
 
                     (Some(ClassifiedCommand::Sink(left)), None) => {
                         let input_vec: Vec<Value> = input.objects.collect().await;
-                        left.run(ctx, input_vec)?;
+                        if let Err(err) = left.run(ctx, input_vec) {
+                            return LineResult::Error(line.clone(), err);
+                        }
                         break;
                     }
 

src/commands/clip.rs

@@ -13,7 +13,16 @@ pub fn clip(args: SinkCommandArgs) -> Result<(), ShellError> {
             } else {
                 first = false;
             }
-            new_copy_data.push_str(&i.as_string().unwrap());
+            match i.as_string() {
+                Ok(s) => new_copy_data.push_str(&s),
+                Err(_) => {
+                    return Err(ShellError::maybe_labeled_error(
+                        "Given non-string data",
+                        "expected strings from pipeline",
+                        args.name_span,
+                    ))
+                }
+            }
         }
     }
     clip_context.set_contents(new_copy_data).unwrap();

src/commands/enter.rs

@@ -14,6 +14,8 @@ pub fn enter(args: CommandArgs) -> Result<OutputStream, ShellError> {
         ));
     }
 
+    let span = args.name_span;
+
     let cwd = args
         .env
         .lock()
@@ -118,27 +120,67 @@ pub fn enter(args: CommandArgs) -> Result<OutputStream, ShellError> {
     match file_extension {
         Some(x) if x == "toml" => {
             stream.push_back(ReturnValue::Action(CommandAction::Enter(
-                crate::commands::from_toml::from_toml_string_to_value(contents),
+                crate::commands::from_toml::from_toml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not load as TOML",
+                            "could not load as TOML",
+                            span,
+                        )
+                    },
+                )?,
             )));
         }
         Some(x) if x == "json" => {
             stream.push_back(ReturnValue::Action(CommandAction::Enter(
-                crate::commands::from_json::from_json_string_to_value(contents),
+                crate::commands::from_json::from_json_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not load as JSON",
+                            "could not load as JSON",
+                            span,
+                        )
+                    },
+                )?,
             )));
         }
         Some(x) if x == "xml" => {
             stream.push_back(ReturnValue::Action(CommandAction::Enter(
-                crate::commands::from_xml::from_xml_string_to_value(contents),
+                crate::commands::from_xml::from_xml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not load as XML",
+                            "could not load as XML",
+                            span,
+                        )
+                    },
+                )?,
             )));
         }
         Some(x) if x == "yml" => {
             stream.push_back(ReturnValue::Action(CommandAction::Enter(
-                crate::commands::from_yaml::from_yaml_string_to_value(contents),
+                crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not load as YAML",
+                            "could not load as YAML",
+                            span,
+                        )
+                    },
+                )?,
             )));
         }
         Some(x) if x == "yaml" => {
             stream.push_back(ReturnValue::Action(CommandAction::Enter(
-                crate::commands::from_yaml::from_yaml_string_to_value(contents),
+                crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not load as YAML",
+                            "could not load as YAML",
+                            span,
+                        )
+                    },
+                )?,
             )));
         }
         _ => {

src/commands/from_json.rs

@@ -28,19 +28,31 @@ fn convert_json_value_to_nu_value(v: &serde_hjson::Value) -> Value {
     }
 }
 
-pub fn from_json_string_to_value(s: String) -> Value {
-    let v: serde_hjson::Value = serde_hjson::from_str(&s).unwrap();
-    convert_json_value_to_nu_value(&v)
+pub fn from_json_string_to_value(s: String) -> serde_hjson::Result<Value> {
+    let v: serde_hjson::Value = serde_hjson::from_str(&s)?;
+    Ok(convert_json_value_to_nu_value(&v))
 }
 
 pub fn from_json(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| match a {
-            Value::Primitive(Primitive::String(s)) => {
-                ReturnValue::Value(from_json_string_to_value(s))
-            }
-            _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))),
+        .map(move |a| match a {
+            Value::Primitive(Primitive::String(s)) => match from_json_string_to_value(s) {
+                Ok(x) => ReturnValue::Value(x),
+                Err(_) => {
+                    ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                        "Could not parse as JSON",
+                        "piped data failed JSON parse",
+                        span,
+                    ))))
+                }
+            },
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
+                span,
+            )))),
         })
         .boxed())
 }

src/commands/from_toml.rs

@@ -1,5 +1,5 @@
-use crate::object::{Primitive, Value, Dictionary, DataDescriptor};
 use crate::object::base::OF64;
+use crate::object::{DataDescriptor, Dictionary, Primitive, Value};
 use crate::prelude::*;
 
 fn convert_toml_value_to_nu_value(v: &toml::Value) -> Value {
@@ -8,31 +8,50 @@ fn convert_toml_value_to_nu_value(v: &toml::Value) -> Value {
         toml::Value::Integer(n) => Value::Primitive(Primitive::Int(*n)),
         toml::Value::Float(n) => Value::Primitive(Primitive::Float(OF64::from(*n))),
         toml::Value::String(s) => Value::Primitive(Primitive::String(s.clone())),
-        toml::Value::Array(a) => Value::List(a.iter().map(|x| convert_toml_value_to_nu_value(x)).collect()),
+        toml::Value::Array(a) => Value::List(
+            a.iter()
+                .map(|x| convert_toml_value_to_nu_value(x))
+                .collect(),
+        ),
         toml::Value::Datetime(dt) => Value::Primitive(Primitive::String(dt.to_string())),
         toml::Value::Table(t) => {
             let mut collected = Dictionary::default();
             for (k, v) in t.iter() {
-                collected.add(DataDescriptor::from(k.clone()), convert_toml_value_to_nu_value(v));
+                collected.add(
+                    DataDescriptor::from(k.clone()),
+                    convert_toml_value_to_nu_value(v),
+                );
             }
             Value::Object(collected)
         }
     }
 }
 
-pub fn from_toml_string_to_value(s: String) -> Value {
-    let v: toml::Value = s.parse::<toml::Value>().unwrap();
-    convert_toml_value_to_nu_value(&v)
+pub fn from_toml_string_to_value(s: String) -> Result<Value, Box<dyn std::error::Error>> {
+    let v: toml::Value = s.parse::<toml::Value>()?;
+    Ok(convert_toml_value_to_nu_value(&v))
 }
 
 pub fn from_toml(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| match a {
-            Value::Primitive(Primitive::String(s)) => {
-                ReturnValue::Value(from_toml_string_to_value(s))
-            }
-            _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))),
+        .map(move |a| match a {
+            Value::Primitive(Primitive::String(s)) => match from_toml_string_to_value(s) {
+                Ok(x) => ReturnValue::Value(x),
+                Err(_) => {
+                    ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                        "Could not parse as TOML",
+                        "piped data failed TOML parse",
+                        span,
+                    ))))
+                }
+            },
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
+                span,
+            )))),
         })
         .boxed())
 }

src/commands/from_xml.rs

@@ -46,13 +46,9 @@ fn from_document_to_value(d: &roxmltree::Document) -> Value {
     from_node_to_value(&d.root_element())
 }
 
-pub fn from_xml_string_to_value(s: String) -> Value {
-    match roxmltree::Document::parse(&s) {
-        Ok(doc) => from_document_to_value(&doc),
-        Err(_) => Value::Error(Box::new(ShellError::string(
-            "Can't convert string from xml".to_string(),
-        ))),
-    }
+pub fn from_xml_string_to_value(s: String) -> Result<Value, Box<dyn std::error::Error>> {
+    let parsed = roxmltree::Document::parse(&s)?;
+    Ok(from_document_to_value(&parsed))
 }
 
 pub fn from_xml(args: CommandArgs) -> Result<OutputStream, ShellError> {
@@ -60,12 +56,19 @@ pub fn from_xml(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let span = args.name_span;
     Ok(out
         .map(move |a| match a {
-            Value::Primitive(Primitive::String(s)) => {
-                ReturnValue::Value(from_xml_string_to_value(s))
-            }
+            Value::Primitive(Primitive::String(s)) => match from_xml_string_to_value(s) {
+                Ok(x) => ReturnValue::Value(x),
+                Err(_) => {
+                    ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                        "Could not parse as XML",
+                        "piped data failed XML parse",
+                        span,
+                    ))))
+                }
+            },
             _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
-                "Trying to convert XML from non-string".to_string(),
-                "given non-string",
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
                 span,
             )))),
         })

src/commands/from_yaml.rs

@@ -36,19 +36,31 @@ fn convert_yaml_value_to_nu_value(v: &serde_yaml::Value) -> Value {
     }
 }
 
-pub fn from_yaml_string_to_value(s: String) -> Value {
-    let v: serde_yaml::Value = serde_yaml::from_str(&s).unwrap();
-    convert_yaml_value_to_nu_value(&v)
+pub fn from_yaml_string_to_value(s: String) -> serde_yaml::Result<Value> {
+    let v: serde_yaml::Value = serde_yaml::from_str(&s)?;
+    Ok(convert_yaml_value_to_nu_value(&v))
 }
 
 pub fn from_yaml(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| match a {
-            Value::Primitive(Primitive::String(s)) => {
-                ReturnValue::Value(from_yaml_string_to_value(s))
-            }
-            _ => ReturnValue::Value(Value::Primitive(Primitive::String("".to_string()))),
+        .map(move |a| match a {
+            Value::Primitive(Primitive::String(s)) => match from_yaml_string_to_value(s) {
+                Ok(x) => ReturnValue::Value(x),
+                Err(_) => {
+                    ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                        "Could not parse as YAML",
+                        "piped data failed YAML parse",
+                        span,
+                    ))))
+                }
+            },
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
+                span,
+            )))),
         })
         .boxed())
 }

src/commands/open.rs

@@ -13,6 +13,8 @@ pub fn open(args: CommandArgs) -> Result<OutputStream, ShellError> {
         ));
     }
 
+    let span = args.name_span;
+
     let cwd = args
         .env
         .lock()
@@ -117,27 +119,67 @@ pub fn open(args: CommandArgs) -> Result<OutputStream, ShellError> {
     match file_extension {
         Some(x) if x == "toml" => {
             stream.push_back(ReturnValue::Value(
-                crate::commands::from_toml::from_toml_string_to_value(contents),
+                crate::commands::from_toml::from_toml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not open as TOML",
+                            "could not open as TOML",
+                            span,
+                        )
+                    },
+                )?,
             ));
         }
         Some(x) if x == "json" => {
             stream.push_back(ReturnValue::Value(
-                crate::commands::from_json::from_json_string_to_value(contents),
+                crate::commands::from_json::from_json_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not open as JSON",
+                            "could not open as JSON",
+                            span,
+                        )
+                    },
+                )?,
             ));
         }
         Some(x) if x == "xml" => {
             stream.push_back(ReturnValue::Value(
-                crate::commands::from_xml::from_xml_string_to_value(contents),
+                crate::commands::from_xml::from_xml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not open as XML",
+                            "could not open as XML",
+                            span,
+                        )
+                    },
+                )?,
             ));
         }
         Some(x) if x == "yml" => {
             stream.push_back(ReturnValue::Value(
-                crate::commands::from_yaml::from_yaml_string_to_value(contents),
+                crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not open as YAML",
+                            "could not open as YAML",
+                            span,
+                        )
+                    },
+                )?,
             ));
         }
         Some(x) if x == "yaml" => {
             stream.push_back(ReturnValue::Value(
-                crate::commands::from_yaml::from_yaml_string_to_value(contents),
+                crate::commands::from_yaml::from_yaml_string_to_value(contents).map_err(
+                    move |_| {
+                        ShellError::maybe_labeled_error(
+                            "Could not open as YAML",
+                            "could not open as YAML",
+                            span,
+                        )
+                    },
+                )?,
             ));
         }
         _ => {

src/commands/split_column.rs

@@ -7,6 +7,7 @@ use log::trace;
 
 pub fn split_column(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let input = args.input;
+    let span = args.name_span;
     let args = args.positional;
 
     Ok(input
@@ -53,7 +54,11 @@ pub fn split_column(args: CommandArgs) -> Result<OutputStream, ShellError> {
                     ReturnValue::Value(Value::Object(dict))
                 }
             }
-            _ => ReturnValue::Value(Value::Object(crate::object::Dictionary::default())),
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
+                span,
+            )))),
         })
         .boxed())
 }

src/commands/split_row.rs

@@ -7,6 +7,7 @@ use log::trace;
 
 pub fn split_row(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let input = args.input;
+    let span = args.name_span;
     let args = args.positional;
 
     let stream = input
@@ -27,7 +28,14 @@ pub fn split_row(args: CommandArgs) -> Result<OutputStream, ShellError> {
                 result
             }
             _ => {
-                let result = VecDeque::new();
+                let mut result = VecDeque::new();
+                result.push_back(ReturnValue::Value(Value::Error(Box::new(
+                    ShellError::maybe_labeled_error(
+                        "Expected string values from pipeline",
+                        "expects strings from pipeline",
+                        span,
+                    ),
+                ))));
                 result
             }
         })

src/commands/to_json.rs

@@ -3,7 +3,15 @@ use crate::prelude::*;
 
 pub fn to_json(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| ReturnValue::Value(Value::Primitive(Primitive::String(serde_json::to_string(&a).unwrap()))))
+        .map(move |a| match serde_json::to_string(&a) {
+            Ok(x) => ReturnValue::Value(Value::Primitive(Primitive::String(x))),
+            Err(_) => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Can not convert to JSON string",
+                "can not convert piped data to JSON string",
+                span,
+            )))),
+        })
         .boxed())
 }

src/commands/to_toml.rs

@@ -3,7 +3,15 @@ use crate::prelude::*;
 
 pub fn to_toml(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let out = args.input;
+    let span = args.name_span;
     Ok(out
-        .map(|a| ReturnValue::Value(Value::Primitive(Primitive::String(toml::to_string(&a).unwrap()))))
+        .map(move |a| match toml::to_string(&a) {
+            Ok(x) => ReturnValue::Value(Value::Primitive(Primitive::String(x))),
+            Err(_) => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Can not convert to TOML string",
+                "can not convert piped data to TOML string",
+                span,
+            )))),
+        })
         .boxed())
 }

src/commands/trim.rs

@@ -6,13 +6,18 @@ use crate::prelude::*;
 
 pub fn trim(args: CommandArgs) -> Result<OutputStream, ShellError> {
     let input = args.input;
+    let span = args.name_span;
 
     Ok(input
         .map(move |v| match v {
             Value::Primitive(Primitive::String(s)) => {
                 ReturnValue::Value(Value::Primitive(Primitive::String(s.trim().to_string())))
             }
-            x => ReturnValue::Value(x),
+            _ => ReturnValue::Value(Value::Error(Box::new(ShellError::maybe_labeled_error(
+                "Expected string values from pipeline",
+                "expects strings from pipeline",
+                span,
+            )))),
         })
         .boxed())
 }