Remove unwraps (#1153)

* Remove a batch of unwraps

* finish another batch

crates/nu-parser/src/hir/tokens_iterator/debug/expand_trace.rs

@@ -254,7 +254,7 @@ pub struct ExpandTracer {
 
 impl ExpandTracer {
     pub fn print(&self, source: Text) -> PrintTracer {
-        let root = self.frame_stack.get(0).unwrap().to_tree_frame(&source);
+        let root = self.frame_stack[0].to_tree_frame(&source);
 
         PrintTracer { root, source }
     }

crates/nu-parser/src/parse/operator.rs

@@ -42,7 +42,11 @@ impl CompareOperator {
 
 impl From<&str> for CompareOperator {
     fn from(input: &str) -> CompareOperator {
-        CompareOperator::from_str(input).unwrap()
+        if let Ok(output) = CompareOperator::from_str(input) {
+            output
+        } else {
+            unreachable!("Internal error: CompareOperator from failed")
+        }
     }
 }
 
@@ -90,7 +94,11 @@ impl EvaluationOperator {
 
 impl From<&str> for EvaluationOperator {
     fn from(input: &str) -> EvaluationOperator {
-        EvaluationOperator::from_str(input).unwrap()
+        if let Ok(output) = EvaluationOperator::from_str(input) {
+            output
+        } else {
+            unreachable!("Internal error: EvaluationOperator 'from' failed")
+        }
     }
 }
 

crates/nu-parser/src/parse/parser.rs

@@ -148,13 +148,21 @@ macro_rules! primitive_decimal {
         $(
             impl From<$ty> for Number {
                 fn from(decimal: $ty) -> Number {
-                    Number::Decimal(BigDecimal::$from(decimal).unwrap())
+                    if let Some(num) = BigDecimal::$from(decimal) {
+                        Number::Decimal(num)
+                    } else {
+                        unreachable!("Internal error: BigDecimal 'from' failed")
+                    }
                 }
             }
 
             impl From<&$ty> for Number {
                 fn from(decimal: &$ty) -> Number {
-                    Number::Decimal(BigDecimal::$from(*decimal).unwrap())
+                    if let Some(num) = BigDecimal::$from(*decimal) {
+                        Number::Decimal(num)
+                    } else {
+                        unreachable!("Internal error: BigDecimal 'from' failed")
+                    }
                 }
             }
         )*
@@ -909,11 +917,13 @@ pub fn module(input: NomSpan) -> IResult<NomSpan, TokenNode> {
 }
 
 fn parse_int<T>(frag: &str, neg: Option<T>) -> i64 {
-    let int = FromStr::from_str(frag).unwrap();
-
-    match neg {
-        None => int,
-        Some(_) => -int,
+    if let Ok(int) = FromStr::from_str(frag) {
+        match neg {
+            None => int,
+            Some(_) => -int,
+        }
+    } else {
+        unreachable!("Internal error: parse_int failed");
     }
 }
 

crates/nu-parser/src/parse/token_tree_builder.rs

@@ -323,10 +323,13 @@ impl TokenTreeBuilder {
 
         let mut input = input.into_iter();
 
-        let head = input.next().unwrap();
-        let tail = input.collect();
+        if let Some(head) = input.next() {
+            let tail = input.collect();
 
-        CallNode::new(Box::new(head), tail).spanned(span.into())
+            CallNode::new(Box::new(head), tail).spanned(span.into())
+        } else {
+            unreachable!("Internal error: spanned_call failed")
+        }
     }
 
     fn consume_delimiter(

crates/nu-parser/src/parse/tokens.rs

@@ -88,9 +88,19 @@ impl RawNumber {
 
     pub(crate) fn to_number(self, source: &Text) -> Number {
         match self {
-            RawNumber::Int(tag) => Number::Int(BigInt::from_str(tag.slice(source)).unwrap()),
+            RawNumber::Int(tag) => {
+                if let Ok(int) = BigInt::from_str(tag.slice(source)) {
+                    Number::Int(int)
+                } else {
+                    unreachable!("Internal error: to_number failed")
+                }
+            }
             RawNumber::Decimal(tag) => {
-                Number::Decimal(BigDecimal::from_str(tag.slice(source)).unwrap())
+                if let Ok(decimal) = BigDecimal::from_str(tag.slice(source)) {
+                    Number::Decimal(decimal)
+                } else {
+                    unreachable!("Internal error: to_number failed")
+                }
             }
         }
     }

crates/nu-plugin/src/test_helpers.rs

@@ -124,14 +124,14 @@ impl CallStub {
         self
     }
 
-    pub fn with_parameter(&mut self, name: &str) -> &mut Self {
+    pub fn with_parameter(&mut self, name: &str) -> Result<&mut Self, ShellError> {
         let fields: Vec<Value> = name
             .split('.')
             .map(|s| UntaggedValue::string(s.to_string()).into_value(Tag::unknown()))
             .collect();
 
-        self.positionals.push(value::column_path(&fields));
-        self
+        self.positionals.push(value::column_path(&fields)?);
+        Ok(self)
     }
 
     pub fn create(&self) -> CallInfo {
@@ -156,7 +156,11 @@ pub fn expect_return_value_at(
         };
 
         if idx == at {
-            return item.raw_value().unwrap();
+            if let Some(value) = item.raw_value() {
+                return value;
+            } else {
+                panic!("Internal error: could not get raw value in expect_return_value_at")
+            }
         }
     }
 
@@ -168,6 +172,7 @@ pub fn expect_return_value_at(
 }
 
 pub mod value {
+    use nu_errors::ShellError;
     use nu_protocol::{Primitive, TaggedDictBuilder, UntaggedValue, Value};
     use nu_source::Tag;
     use nu_value_ext::ValueExt;
@@ -199,10 +204,10 @@ pub mod value {
         UntaggedValue::table(list).into_untagged_value()
     }
 
-    pub fn column_path(paths: &[Value]) -> Value {
-        UntaggedValue::Primitive(Primitive::ColumnPath(
-            table(&paths.to_vec()).as_column_path().unwrap().item,
+    pub fn column_path(paths: &[Value]) -> Result<Value, ShellError> {
+        Ok(UntaggedValue::Primitive(Primitive::ColumnPath(
+            table(&paths.to_vec()).as_column_path()?.item,
         ))
-        .into_untagged_value()
+        .into_untagged_value())
     }
 }

crates/nu-test-support/src/fs.rs

@@ -30,7 +30,11 @@ impl AbsoluteFile {
     }
 
     pub fn dir(&self) -> AbsolutePath {
-        AbsolutePath::new(self.inner.parent().unwrap())
+        AbsolutePath::new(if let Some(parent) = self.inner.parent() {
+            parent
+        } else {
+            unreachable!("Internal error: could not get parent in dir")
+        })
     }
 }
 

crates/nu_plugin_inc/src/nu_plugin_inc/tests.rs

@@ -1,6 +1,7 @@
 mod integration {
     use crate::inc::{Action, SemVerAction};
     use crate::Inc;
+    use nu_errors::ShellError;
     use nu_plugin::test_helpers::value::{column_path, string};
     use nu_plugin::test_helpers::{plugin, CallStub};
 
@@ -52,16 +53,21 @@ mod integration {
     }
 
     #[test]
-    fn picks_up_argument_for_field() {
+    fn picks_up_argument_for_field() -> Result<(), ShellError> {
         plugin(&mut Inc::new())
-            .args(CallStub::new().with_parameter("package.version").create())
+            .args(CallStub::new().with_parameter("package.version")?.create())
             .setup(|plugin, _| {
-                plugin.expect_field(column_path(&[string("package"), string("version")]))
+                //FIXME: this will need to be updated
+                if let Ok(column_path) = column_path(&[string("package"), string("version")]) {
+                    plugin.expect_field(column_path)
+                }
             });
+        Ok(())
     }
 
     mod sem_ver {
         use crate::Inc;
+        use nu_errors::ShellError;
         use nu_plugin::test_helpers::value::{get_data, string, structured_sample_record};
         use nu_plugin::test_helpers::{expect_return_value_at, plugin, CallStub};
 
@@ -70,12 +76,12 @@ mod integration {
         }
 
         #[test]
-        fn major_input_using_the_field_passed_as_parameter() {
+        fn major_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
             let run = plugin(&mut Inc::new())
                 .args(
                     CallStub::new()
                         .with_long_flag("major")
-                        .with_parameter("version")
+                        .with_parameter("version")?
                         .create(),
                 )
                 .input(cargo_sample_record("0.1.3"))
@@ -85,15 +91,16 @@ mod integration {
             let actual = expect_return_value_at(run, 0);
 
             assert_eq!(get_data(actual, "version"), string("1.0.0"));
+            Ok(())
         }
 
         #[test]
-        fn minor_input_using_the_field_passed_as_parameter() {
+        fn minor_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
             let run = plugin(&mut Inc::new())
                 .args(
                     CallStub::new()
                         .with_long_flag("minor")
-                        .with_parameter("version")
+                        .with_parameter("version")?
                         .create(),
                 )
                 .input(cargo_sample_record("0.1.3"))
@@ -103,15 +110,16 @@ mod integration {
             let actual = expect_return_value_at(run, 0);
 
             assert_eq!(get_data(actual, "version"), string("0.2.0"));
+            Ok(())
         }
 
         #[test]
-        fn patch_input_using_the_field_passed_as_parameter() {
+        fn patch_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
             let run = plugin(&mut Inc::new())
                 .args(
                     CallStub::new()
                         .with_long_flag("patch")
-                        .with_parameter("version")
+                        .with_parameter("version")?
                         .create(),
                 )
                 .input(cargo_sample_record("0.1.3"))
@@ -121,6 +129,7 @@ mod integration {
             let actual = expect_return_value_at(run, 0);
 
             assert_eq!(get_data(actual, "version"), string("0.1.4"));
+            Ok(())
         }
     }
 }

crates/nu_plugin_str/src/nu_plugin_str/tests.rs

@@ -1,6 +1,7 @@
 mod integration {
     use crate::strutils::{Action, ReplaceAction};
     use crate::Str;
+    use nu_errors::ShellError;
     use nu_plugin::test_helpers::value::{
         column_path, get_data, int, string, structured_sample_record, table,
         unstructured_sample_record,
@@ -83,16 +84,21 @@ mod integration {
     }
 
     #[test]
-    fn picks_up_argument_for_field() {
+    fn picks_up_argument_for_field() -> Result<(), ShellError> {
         plugin(&mut Str::new())
             .args(
                 CallStub::new()
-                    .with_parameter("package.description")
+                    .with_parameter("package.description")?
                     .create(),
             )
             .setup(|plugin, _| {
-                plugin.expect_field(column_path(&[string("package"), string("description")]))
+                //FIXME: this is possibly not correct
+                if let Ok(column_path) = column_path(&[string("package"), string("description")]) {
+                    plugin.expect_field(column_path)
+                }
             });
+
+        Ok(())
     }
 
     #[test]
@@ -115,12 +121,12 @@ mod integration {
     }
 
     #[test]
-    fn upcases_the_input_using_the_field_passed_as_parameter() {
+    fn upcases_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
         let run = plugin(&mut Str::new())
             .args(
                 CallStub::new()
                     .with_long_flag("upcase")
-                    .with_parameter("name")
+                    .with_parameter("name")?
                     .create(),
             )
             .input(structured_sample_record("name", "jotandrehuda"))
@@ -130,15 +136,16 @@ mod integration {
         let actual = expect_return_value_at(run, 0);
 
         assert_eq!(get_data(actual, "name"), string("JOTANDREHUDA"));
+        Ok(())
     }
 
     #[test]
-    fn downcases_the_input_using_the_field_passed_as_parameter() {
+    fn downcases_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
         let run = plugin(&mut Str::new())
             .args(
                 CallStub::new()
                     .with_long_flag("downcase")
-                    .with_parameter("name")
+                    .with_parameter("name")?
                     .create(),
             )
             .input(structured_sample_record("name", "JOTANDREHUDA"))
@@ -148,15 +155,17 @@ mod integration {
         let actual = expect_return_value_at(run, 0);
 
         assert_eq!(get_data(actual, "name"), string("jotandrehuda"));
+        Ok(())
     }
 
     #[test]
-    fn converts_the_input_to_integer_using_the_field_passed_as_parameter() {
+    fn converts_the_input_to_integer_using_the_field_passed_as_parameter() -> Result<(), ShellError>
+    {
         let run = plugin(&mut Str::new())
             .args(
                 CallStub::new()
                     .with_long_flag("to-int")
-                    .with_parameter("Nu_birthday")
+                    .with_parameter("Nu_birthday")?
                     .create(),
             )
             .input(structured_sample_record("Nu_birthday", "10"))
@@ -166,14 +175,15 @@ mod integration {
         let actual = expect_return_value_at(run, 0);
 
         assert_eq!(get_data(actual, "Nu_birthday"), int(10));
+        Ok(())
     }
 
     #[test]
-    fn replaces_the_input_using_the_field_passed_as_parameter() {
+    fn replaces_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
         let run = plugin(&mut Str::new())
             .args(
                 CallStub::new()
-                    .with_parameter("rustconf")
+                    .with_parameter("rustconf")?
                     .with_named_parameter("replace", string("22nd August 2019"))
                     .create(),
             )
@@ -184,14 +194,15 @@ mod integration {
         let actual = expect_return_value_at(run, 0);
 
         assert_eq!(get_data(actual, "rustconf"), string("22nd August 2019"));
+        Ok(())
     }
 
     #[test]
-    fn find_and_replaces_the_input_using_the_field_passed_as_parameter() {
+    fn find_and_replaces_the_input_using_the_field_passed_as_parameter() -> Result<(), ShellError> {
         let run = plugin(&mut Str::new())
             .args(
                 CallStub::new()
-                    .with_parameter("staff")
+                    .with_parameter("staff")?
                     .with_named_parameter(
                         "find-replace",
                         table(&[string("kittens"), string("jotandrehuda")]),
@@ -205,6 +216,7 @@ mod integration {
         let actual = expect_return_value_at(run, 0);
 
         assert_eq!(get_data(actual, "staff"), string("wyjotandrehuda"));
+        Ok(())
     }
 
     #[test]

src/cli.rs

@@ -384,7 +384,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
             continue;
         }
 
-        let cwd = context.shell_manager.path();
+        let cwd = context.shell_manager.path()?;
 
         rl.set_helper(Some(crate::shell::Helper::new(context.clone())));
 
@@ -452,7 +452,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
 
                 context.with_host(|host| {
                     print_err(err, host, &Text::from(line.clone()));
-                });
+                })?;
 
                 context.maybe_print_errors(Text::from(line.clone()));
             }
@@ -474,7 +474,7 @@ pub async fn cli() -> Result<(), Box<dyn Error>> {
                     let _ = rl.save_history(&History::path());
                     std::process::exit(0);
                 } else {
-                    context.with_host(|host| host.stdout("CTRL-C pressed (again to quit)"));
+                    context.with_host(|host| host.stdout("CTRL-C pressed (again to quit)"))?;
                     ctrlcbreak = true;
                     continue;
                 }
@@ -634,13 +634,13 @@ pub fn classify_pipeline(
     let result = expand_syntax(
         &PipelineShape,
         &mut iterator,
-        &context.expand_context(source),
+        &context.expand_context(source)?,
     )
     .map_err(|err| err.into());
 
     if log_enabled!(target: "nu::expand_syntax", log::Level::Debug) {
         outln!("");
-        ptree::print_tree(&iterator.expand_tracer().print(source.clone())).unwrap();
+        let _ = ptree::print_tree(&iterator.expand_tracer().print(source.clone()));
         outln!("");
     }
 

src/commands/classified/external.rs

@@ -148,7 +148,7 @@ pub(crate) async fn run_external_command(
         }
     }
 
-    process = process.cwd(context.shell_manager.path());
+    process = process.cwd(context.shell_manager.path()?);
 
     trace!(target: "nu::run::external", "cwd = {:?}", context.shell_manager.path());
 

src/commands/classified/internal.rs

@@ -46,11 +46,11 @@ pub(crate) async fn run_internal_command(
             match item {
                 Ok(ReturnSuccess::Action(action)) => match action {
                     CommandAction::ChangePath(path) => {
-                        context.shell_manager.set_path(path);
+                        context.shell_manager.set_path(path)?;
                     }
                     CommandAction::Exit => std::process::exit(0), // TODO: save history.txt
                     CommandAction::Error(err) => {
-                        context.error(err);
+                        context.error(err)?;
                         break;
                     }
                     CommandAction::AutoConvert(tagged_contents, extension) => {
@@ -99,39 +99,43 @@ pub(crate) async fn run_internal_command(
                                 value: UntaggedValue::Primitive(Primitive::String(cmd)),
                                 tag,
                             } => {
-                                context.shell_manager.insert_at_current(Box::new(
+                                let result = context.shell_manager.insert_at_current(Box::new(
                                     HelpShell::for_command(
                                         UntaggedValue::string(cmd).into_value(tag),
                                         &context.registry(),
                                     )?,
                                 ));
+
+                                result?
                             }
                             _ => {
-                                context.shell_manager.insert_at_current(Box::new(
-                                    HelpShell::index(&context.registry()).unwrap(),
+                                let result = context.shell_manager.insert_at_current(Box::new(
+                                    HelpShell::index(&context.registry())?,
                                 ));
+
+                                result?
                             }
                         }
                     }
                     CommandAction::EnterValueShell(value) => {
                         context
                             .shell_manager
-                            .insert_at_current(Box::new(ValueShell::new(value)));
+                            .insert_at_current(Box::new(ValueShell::new(value)))?;
                     }
                     CommandAction::EnterShell(location) => {
                         context.shell_manager.insert_at_current(Box::new(
                             FilesystemShell::with_location(location, context.registry().clone()).unwrap(),
-                        ));
+                        ))?;
                     }
                     CommandAction::PreviousShell => {
-                        context.shell_manager.prev();
+                        context.shell_manager.prev()?;
                     }
                     CommandAction::NextShell => {
-                        context.shell_manager.next();
+                        context.shell_manager.next()?;
                     }
                     CommandAction::LeaveShell => {
-                        context.shell_manager.remove_at_current();
-                        if context.shell_manager.is_empty() {
+                        context.shell_manager.remove_at_current()?;
+                        if context.shell_manager.is_empty()? {
                             std::process::exit(0); // TODO: save history.txt
                         }
                     }
@@ -149,7 +153,7 @@ pub(crate) async fn run_internal_command(
                     let mut buffer = termcolor::Buffer::ansi();
 
                     let _ = doc.render_raw(
-                        context.with_host(|host| host.width() - 5),
+                        context.with_host(|host| host.width() - 5)?,
                         &mut nu_source::TermColored::new(&mut buffer),
                     );
 
@@ -159,7 +163,7 @@ pub(crate) async fn run_internal_command(
                 }
 
                 Err(err) => {
-                    context.error(err);
+                    context.error(err)?;
                     break;
                 }
             }

src/commands/clip.rs

@@ -55,35 +55,52 @@ pub mod clipboard {
     }
 
     async fn inner_clip(input: Vec<Value>, name: Tag) -> OutputStream {
-        let mut clip_context: ClipboardContext = ClipboardProvider::new().unwrap();
-        let mut new_copy_data = String::new();
+        if let Ok(clip_context) = ClipboardProvider::new() {
+            let mut clip_context: ClipboardContext = clip_context;
+            let mut new_copy_data = String::new();
 
-        if !input.is_empty() {
-            let mut first = true;
-            for i in input.iter() {
-                if !first {
-                    new_copy_data.push_str("\n");
-                } else {
-                    first = false;
-                }
-
-                let string: String = match i.as_string() {
-                    Ok(string) => string.to_string(),
-                    Err(_) => {
-                        return OutputStream::one(Err(ShellError::labeled_error(
-                            "Given non-string data",
-                            "expected strings from pipeline",
-                            name,
-                        )))
+            if !input.is_empty() {
+                let mut first = true;
+                for i in input.iter() {
+                    if !first {
+                        new_copy_data.push_str("\n");
+                    } else {
+                        first = false;
                     }
-                };
 
-                new_copy_data.push_str(&string);
+                    let string: String = match i.as_string() {
+                        Ok(string) => string.to_string(),
+                        Err(_) => {
+                            return OutputStream::one(Err(ShellError::labeled_error(
+                                "Given non-string data",
+                                "expected strings from pipeline",
+                                name,
+                            )))
+                        }
+                    };
+
+                    new_copy_data.push_str(&string);
+                }
             }
+
+            match clip_context.set_contents(new_copy_data) {
+                Ok(_) => {}
+                Err(_) => {
+                    return OutputStream::one(Err(ShellError::labeled_error(
+                        "Could not set contents of clipboard",
+                        "could not set contents of clipboard",
+                        name,
+                    )));
+                }
+            }
+
+            OutputStream::empty()
+        } else {
+            OutputStream::one(Err(ShellError::labeled_error(
+                "Could not open clipboard",
+                "could not open clipboard",
+                name,
+            )))
         }
-
-        clip_context.set_contents(new_copy_data).unwrap();
-
-        OutputStream::empty()
     }
 }

src/commands/enter.rs

@@ -72,7 +72,7 @@ impl PerItemCommand for Enter {
                         // If it's a file, attempt to open the file as a value and enter it
                         let cwd = raw_args.shell_manager.path();
 
-                        let full_path = std::path::PathBuf::from(cwd);
+                        let full_path = std::path::PathBuf::from(cwd?);
 
                         let (file_extension, contents, contents_tag) =
                             crate::commands::open::fetch(

src/commands/evaluate_by.rs

@@ -45,7 +45,6 @@ pub fn evaluate_by(
     let stream = async_stream! {
         let values: Vec<Value> = input.values.collect().await;
 
-
         if values.is_empty() {
             yield Err(ShellError::labeled_error(
                     "Expected table from pipeline",
@@ -141,6 +140,7 @@ mod tests {
     use crate::data::value;
     use crate::prelude::*;
     use indexmap::IndexMap;
+    use nu_errors::ShellError;
     use nu_protocol::{UntaggedValue, Value};
     use nu_source::TaggedItem;
 
@@ -160,21 +160,20 @@ mod tests {
         UntaggedValue::table(list).into_untagged_value()
     }
 
-    fn nu_releases_sorted_by_date() -> Value {
+    fn nu_releases_sorted_by_date() -> Result<Value, ShellError> {
         let key = String::from("date");
 
         t_sort(
             Some(key),
             None,
-            &nu_releases_grouped_by_date(),
+            &nu_releases_grouped_by_date()?,
             Tag::unknown(),
         )
-        .unwrap()
     }
 
-    fn nu_releases_grouped_by_date() -> Value {
+    fn nu_releases_grouped_by_date() -> Result<Value, ShellError> {
         let key = String::from("date").tagged_unknown();
-        group(&key, nu_releases_commiters(), Tag::unknown()).unwrap()
+        group(&key, nu_releases_commiters(), Tag::unknown())
     }
 
     fn nu_releases_commiters() -> Vec<Value> {
@@ -230,28 +229,32 @@ mod tests {
     }
 
     #[test]
-    fn evaluates_the_tables() {
+    fn evaluates_the_tables() -> Result<(), ShellError> {
         assert_eq!(
-            evaluate(&nu_releases_sorted_by_date(), None, Tag::unknown()).unwrap(),
+            evaluate(&nu_releases_sorted_by_date()?, None, Tag::unknown())?,
             table(&[table(&[
                 table(&[int(1), int(1), int(1)]),
                 table(&[int(1), int(1), int(1)]),
                 table(&[int(1), int(1), int(1)]),
             ]),])
         );
+
+        Ok(())
     }
 
     #[test]
-    fn evaluates_the_tables_with_custom_evaluator() {
+    fn evaluates_the_tables_with_custom_evaluator() -> Result<(), ShellError> {
         let eval = String::from("name");
 
         assert_eq!(
-            evaluate(&nu_releases_sorted_by_date(), Some(eval), Tag::unknown()).unwrap(),
+            evaluate(&nu_releases_sorted_by_date()?, Some(eval), Tag::unknown())?,
             table(&[table(&[
                 table(&[string("AR"), string("JT"), string("YK")]),
                 table(&[string("AR"), string("YK"), string("JT")]),
                 table(&[string("YK"), string("JT"), string("AR")]),
             ]),])
         );
+
+        Ok(())
     }
 }

src/commands/open.rs

@@ -40,7 +40,7 @@ impl PerItemCommand for Open {
 
 fn run(call_info: &CallInfo, raw_args: &RawCommandArgs) -> Result<OutputStream, ShellError> {
     let shell_manager = &raw_args.shell_manager;
-    let cwd = PathBuf::from(shell_manager.path());
+    let cwd = PathBuf::from(shell_manager.path()?);
     let full_path = cwd;
 
     let path = call_info.args.nth(0).ok_or_else(|| {

src/commands/save.rs

@@ -130,7 +130,7 @@ fn save(
     }: RunnableContext,
     raw_args: RawCommandArgs,
 ) -> Result<OutputStream, ShellError> {
-    let mut full_path = PathBuf::from(shell_manager.path());
+    let mut full_path = PathBuf::from(shell_manager.path()?);
     let name_tag = name.clone();
 
     let stream = async_stream! {

src/commands/shells.rs

@@ -32,7 +32,16 @@ fn shells(args: CommandArgs, _registry: &CommandRegistry) -> Result<OutputStream
     let mut shells_out = VecDeque::new();
     let tag = args.call_info.name_tag;
 
-    for (index, shell) in args.shell_manager.shells.lock().unwrap().iter().enumerate() {
+    for (index, shell) in args
+        .shell_manager
+        .shells
+        .lock()
+        .map_err(|_| {
+            ShellError::labeled_error("Could not list shells", "could not list shells", &tag)
+        })?
+        .iter()
+        .enumerate()
+    {
         let mut dict = TaggedDictBuilder::new(&tag);
 
         if index == (*args.shell_manager.current_shell).load(Ordering::SeqCst) {

src/context.rs

@@ -95,6 +95,13 @@ impl CommandRegistry {
         })?;
         Ok(registry.keys().cloned().collect())
     }
+
+    pub(crate) fn snapshot(&self) -> Result<IndexMap<String, Arc<Command>>, ShellError> {
+        let registry = self.registry.lock().map_err(|_| {
+            ShellError::untagged_runtime_error("Internal error: names could not get mutex")
+        })?;
+        Ok(registry.clone())
+    }
 }
 
 #[derive(Clone)]
@@ -114,12 +121,12 @@ impl Context {
     pub(crate) fn expand_context<'context>(
         &'context self,
         source: &'context Text,
-    ) -> ExpandContext<'context> {
-        ExpandContext::new(
+    ) -> Result<ExpandContext<'context>, ShellError> {
+        Ok(ExpandContext::new(
             Box::new(self.registry.clone()),
             source,
-            self.shell_manager.homedir(),
-        )
+            self.shell_manager.homedir()?,
+        ))
     }
 
     pub(crate) fn basic() -> Result<Context, Box<dyn Error>> {
@@ -133,7 +140,7 @@ impl Context {
         })
     }
 
-    pub(crate) fn error(&mut self, error: ShellError) {
+    pub(crate) fn error(&mut self, error: ShellError) -> Result<(), ShellError> {
         self.with_errors(|errors| errors.push(error))
     }
 
@@ -177,16 +184,30 @@ impl Context {
         result
     }
 
-    pub(crate) fn with_host<T>(&mut self, block: impl FnOnce(&mut dyn Host) -> T) -> T {
-        let mut host = self.host.lock().unwrap();
-
-        block(&mut *host)
+    pub(crate) fn with_host<T>(
+        &mut self,
+        block: impl FnOnce(&mut dyn Host) -> T,
+    ) -> Result<T, ShellError> {
+        if let Ok(mut host) = self.host.lock() {
+            Ok(block(&mut *host))
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock host in with_host",
+            ))
+        }
     }
 
-    pub(crate) fn with_errors<T>(&mut self, block: impl FnOnce(&mut Vec<ShellError>) -> T) -> T {
-        let mut errors = self.current_errors.lock().unwrap();
-
-        block(&mut *errors)
+    pub(crate) fn with_errors<T>(
+        &mut self,
+        block: impl FnOnce(&mut Vec<ShellError>) -> T,
+    ) -> Result<T, ShellError> {
+        if let Ok(mut errors) = self.current_errors.lock() {
+            Ok(block(&mut *errors))
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock host in with_errors",
+            ))
+        }
     }
 
     pub fn add_commands(&mut self, commands: Vec<Arc<Command>>) -> Result<(), ShellError> {

src/data/base/shape.rs

@@ -359,12 +359,14 @@ impl Shapes {
 
     pub fn to_values(&self) -> Vec<Value> {
         if self.shapes.len() == 1 {
-            let shape = self.shapes.keys().nth(0).unwrap();
-
-            let mut tagged_dict = TaggedDictBuilder::new(Tag::unknown());
-            tagged_dict.insert_untagged("type", shape.to_value());
-            tagged_dict.insert_untagged("rows", UntaggedValue::string("all"));
-            vec![tagged_dict.into_value()]
+            if let Some(shape) = self.shapes.keys().nth(0) {
+                let mut tagged_dict = TaggedDictBuilder::new(Tag::unknown());
+                tagged_dict.insert_untagged("type", shape.to_value());
+                tagged_dict.insert_untagged("rows", UntaggedValue::string("all"));
+                vec![tagged_dict.into_value()]
+            } else {
+                unreachable!("Internal error: impossible state in to_values")
+            }
         } else {
             self.shapes
                 .iter()

src/format/entries.rs

@@ -39,10 +39,10 @@ impl RenderView for EntriesView {
             return Ok(());
         }
 
-        let max_name_size: usize = self.entries.iter().map(|(n, _)| n.len()).max().unwrap();
-
-        for (name, value) in &self.entries {
-            outln!("{:width$} : {}", name, value, width = max_name_size)
+        if let Some(max_name_size) = self.entries.iter().map(|(n, _)| n.len()).max() {
+            for (name, value) in &self.entries {
+                outln!("{:width$} : {}", name, value, width = max_name_size)
+            }
         }
 
         Ok(())

src/format/table.rs

@@ -376,7 +376,7 @@ impl RenderView for TableView {
             ));
         }
 
-        table.print_term(&mut *host.out_terminal()).unwrap();
+        table.print_term(&mut *host.out_terminal()).map_err(|_| ShellError::untagged_runtime_error("Internal error: could not print to terminal (for unix systems check to make sure TERM is set)"))?;
 
         Ok(())
     }

src/prelude.rs

@@ -129,7 +129,15 @@ where
 {
     fn to_input_stream(self) -> InputStream {
         InputStream {
-            values: self.map(|item| item.into().unwrap()).boxed(),
+            values: self
+                .map(|item| {
+                    if let Ok(result) = item.into() {
+                        result
+                    } else {
+                        unreachable!("Internal errors: to_input_stream in inconsistent state")
+                    }
+                })
+                .boxed(),
         }
     }
 }

src/shell/help_shell.rs

@@ -25,18 +25,21 @@ impl HelpShell {
         let mut cmds = TaggedDictBuilder::new(Tag::unknown());
         let mut specs = Vec::new();
 
-        for cmd in registry.names()? {
-            let mut spec = TaggedDictBuilder::new(Tag::unknown());
-            let value = command_dict(registry.get_command(&cmd)?.unwrap(), Tag::unknown());
+        let snapshot = registry.snapshot()?;
 
-            spec.insert_untagged("name", cmd);
+        for (name, cmd) in snapshot.iter() {
+            let mut spec = TaggedDictBuilder::new(Tag::unknown());
+            let value = command_dict(cmd.clone(), Tag::unknown());
+
+            spec.insert_untagged("name", name.to_string());
             spec.insert_untagged(
                 "description",
                 value
                     .get_data_by_key("usage".spanned_unknown())
-                    .unwrap()
-                    .as_string()
-                    .unwrap(),
+                    .ok_or_else(|| {
+                        ShellError::untagged_runtime_error("Internal error: expected to find usage")
+                    })?
+                    .as_string()?,
             );
             spec.insert_value("details", value);
 
@@ -77,7 +80,8 @@ impl HelpShell {
             match p {
                 x if x == sep => {}
                 step => {
-                    let value = viewed.get_data_by_key(step.to_str().unwrap().spanned_unknown());
+                    let step: &str = &step.to_string_lossy().to_string();
+                    let value = viewed.get_data_by_key(step.spanned_unknown());
                     if let Some(v) = value {
                         viewed = v.clone();
                     }

src/shell/helper.rs

@@ -39,7 +39,10 @@ impl Completer for Helper {
 
 impl Hinter for Helper {
     fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option<String> {
-        self.context.shell_manager.hint(line, pos, ctx)
+        match self.context.shell_manager.hint(line, pos, ctx) {
+            Ok(output) => output,
+            Err(e) => Some(format!("{}", e)),
+        }
     }
 }
 
@@ -78,31 +81,43 @@ impl Highlighter for Helper {
                 let mut tokens = TokensIterator::all(&tokens[..], Text::from(line), v.span());
 
                 let text = Text::from(line);
-                let expand_context = self.context.expand_context(&text);
+                match self.context.expand_context(&text) {
+                    Ok(expand_context) => {
+                        let shapes = {
+                            // We just constructed a token list that only contains a pipeline, so it can't fail
+                            if let Err(err) =
+                                color_fallible_syntax(&PipelineShape, &mut tokens, &expand_context)
+                            {
+                                let error_msg = format!("{}", err);
+                                return Cow::Owned(error_msg);
+                            }
+                            tokens.with_color_tracer(|_, tracer| tracer.finish());
 
-                let shapes = {
-                    // We just constructed a token list that only contains a pipeline, so it can't fail
-                    color_fallible_syntax(&PipelineShape, &mut tokens, &expand_context).unwrap();
-                    tokens.with_color_tracer(|_, tracer| tracer.finish());
+                            tokens.state().shapes()
+                        };
 
-                    tokens.state().shapes()
-                };
+                        trace!(target: "nu::color_syntax", "{:#?}", tokens.color_tracer());
 
-                trace!(target: "nu::color_syntax", "{:#?}", tokens.color_tracer());
+                        if log_enabled!(target: "nu::color_syntax", log::Level::Debug) {
+                            outln!("");
+                            let _ = ptree::print_tree(
+                                &tokens.color_tracer().clone().print(Text::from(line)),
+                            );
+                            outln!("");
+                        }
 
-                if log_enabled!(target: "nu::color_syntax", log::Level::Debug) {
-                    outln!("");
-                    ptree::print_tree(&tokens.color_tracer().clone().print(Text::from(line)))
-                        .unwrap();
-                    outln!("");
+                        for shape in shapes {
+                            let styled = paint_flat_shape(&shape, line);
+                            out.push_str(&styled);
+                        }
+
+                        Cow::Owned(out)
+                    }
+                    Err(err) => {
+                        let error_msg = format!("{}", err);
+                        Cow::Owned(error_msg)
+                    }
                 }
-
-                for shape in shapes {
-                    let styled = paint_flat_shape(&shape, line);
-                    out.push_str(&styled);
-                }
-
-                Cow::Owned(out)
             }
         }
     }

src/shell/shell_manager.rs

@@ -30,53 +30,95 @@ impl ShellManager {
         })
     }
 
-    pub fn insert_at_current(&mut self, shell: Box<dyn Shell + Send>) {
-        self.shells.lock().unwrap().push(shell);
-        self.current_shell
-            .store(self.shells.lock().unwrap().len() - 1, Ordering::SeqCst);
-        self.set_path(self.path());
+    pub fn insert_at_current(&mut self, shell: Box<dyn Shell + Send>) -> Result<(), ShellError> {
+        if let Ok(mut shells) = self.shells.lock() {
+            shells.push(shell);
+        } else {
+            return Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer",
+            ));
+        }
+
+        let shells_len = if let Ok(shells) = self.shells.lock() {
+            shells.len()
+        } else {
+            return Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer",
+            ));
+        };
+
+        self.current_shell.store(shells_len - 1, Ordering::SeqCst);
+        self.set_path(self.path()?)
     }
 
     pub fn current_shell(&self) -> usize {
         self.current_shell.load(Ordering::SeqCst)
     }
 
-    pub fn remove_at_current(&mut self) {
+    pub fn remove_at_current(&mut self) -> Result<(), ShellError> {
         {
-            let mut shells = self.shells.lock().unwrap();
-            if shells.len() > 0 {
-                if self.current_shell() == shells.len() - 1 {
-                    shells.pop();
-                    let new_len = shells.len();
-                    if new_len > 0 {
-                        self.current_shell.store(new_len - 1, Ordering::SeqCst);
+            if let Ok(mut shells) = self.shells.lock() {
+                if shells.len() > 0 {
+                    if self.current_shell() == shells.len() - 1 {
+                        shells.pop();
+                        let new_len = shells.len();
+                        if new_len > 0 {
+                            self.current_shell.store(new_len - 1, Ordering::SeqCst);
+                        } else {
+                            return Ok(());
+                        }
                     } else {
-                        return;
+                        shells.remove(self.current_shell());
                     }
-                } else {
-                    shells.remove(self.current_shell());
                 }
+            } else {
+                return Err(ShellError::untagged_runtime_error(
+                    "Internal error: could not lock shells ring buffer",
+                ));
             }
         }
-        self.set_path(self.path());
+        self.set_path(self.path()?)
     }
 
-    pub fn is_empty(&self) -> bool {
-        self.shells.lock().unwrap().is_empty()
+    pub fn is_empty(&self) -> Result<bool, ShellError> {
+        if let Ok(shells) = self.shells.lock() {
+            Ok(shells.is_empty())
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (is_empty)",
+            ))
+        }
     }
 
-    pub fn path(&self) -> String {
-        self.shells.lock().unwrap()[self.current_shell()].path()
+    pub fn path(&self) -> Result<String, ShellError> {
+        if let Ok(shells) = self.shells.lock() {
+            Ok(shells[self.current_shell()].path())
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (path)",
+            ))
+        }
     }
 
     pub fn pwd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock().unwrap();
-
-        env[self.current_shell()].pwd(args)
+        if let Ok(shells) = self.shells.lock() {
+            shells[self.current_shell()].pwd(args)
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (pwd)",
+            ))
+        }
     }
 
-    pub fn set_path(&mut self, path: String) {
-        self.shells.lock().unwrap()[self.current_shell()].set_path(path)
+    pub fn set_path(&mut self, path: String) -> Result<(), ShellError> {
+        if let Ok(mut shells) = self.shells.lock() {
+            shells[self.current_shell()].set_path(path);
+            Ok(())
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (set_path)",
+            ))
+        }
     }
 
     pub fn complete(
@@ -85,43 +127,77 @@ impl ShellManager {
         pos: usize,
         ctx: &rustyline::Context<'_>,
     ) -> Result<(usize, Vec<rustyline::completion::Pair>), rustyline::error::ReadlineError> {
-        self.shells.lock().unwrap()[self.current_shell()].complete(line, pos, ctx)
+        if let Ok(shells) = self.shells.lock() {
+            shells[self.current_shell()].complete(line, pos, ctx)
+        } else {
+            Err(rustyline::error::ReadlineError::Io(std::io::Error::new(
+                std::io::ErrorKind::Other,
+                "Internal error: could not lock shells ring buffer (complete)",
+            )))
+        }
     }
 
-    pub fn hint(&self, line: &str, pos: usize, ctx: &rustyline::Context<'_>) -> Option<String> {
-        self.shells.lock().unwrap()[self.current_shell()].hint(line, pos, ctx)
+    pub fn hint(
+        &self,
+        line: &str,
+        pos: usize,
+        ctx: &rustyline::Context<'_>,
+    ) -> Result<Option<String>, ShellError> {
+        if let Ok(shells) = self.shells.lock() {
+            Ok(shells[self.current_shell()].hint(line, pos, ctx))
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (hint)",
+            ))
+        }
     }
 
-    pub fn next(&mut self) {
+    pub fn next(&mut self) -> Result<(), ShellError> {
         {
-            let shell_len = self.shells.lock().unwrap().len();
-            if self.current_shell() == (shell_len - 1) {
-                self.current_shell.store(0, Ordering::SeqCst);
+            if let Ok(shells) = self.shells.lock() {
+                let shell_len = shells.len();
+                if self.current_shell() == (shell_len - 1) {
+                    self.current_shell.store(0, Ordering::SeqCst);
+                } else {
+                    self.current_shell
+                        .store(self.current_shell() + 1, Ordering::SeqCst);
+                }
             } else {
-                self.current_shell
-                    .store(self.current_shell() + 1, Ordering::SeqCst);
+                return Err(ShellError::untagged_runtime_error(
+                    "Internal error: could not lock shells ring buffer (next)",
+                ));
             }
         }
-        self.set_path(self.path());
+        self.set_path(self.path()?)
     }
 
-    pub fn prev(&mut self) {
+    pub fn prev(&mut self) -> Result<(), ShellError> {
         {
-            let shell_len = self.shells.lock().unwrap().len();
-            if self.current_shell() == 0 {
-                self.current_shell.store(shell_len - 1, Ordering::SeqCst);
+            if let Ok(shells) = self.shells.lock() {
+                let shell_len = shells.len();
+                if self.current_shell() == 0 {
+                    self.current_shell.store(shell_len - 1, Ordering::SeqCst);
+                } else {
+                    self.current_shell
+                        .store(self.current_shell() - 1, Ordering::SeqCst);
+                }
             } else {
-                self.current_shell
-                    .store(self.current_shell() - 1, Ordering::SeqCst);
+                return Err(ShellError::untagged_runtime_error(
+                    "Internal error: could not lock shells ring buffer (prev)",
+                ));
             }
         }
-        self.set_path(self.path());
+        self.set_path(self.path()?)
     }
 
-    pub fn homedir(&self) -> Option<PathBuf> {
-        let env = self.shells.lock().unwrap();
-
-        env[self.current_shell()].homedir()
+    pub fn homedir(&self) -> Result<Option<PathBuf>, ShellError> {
+        if let Ok(shells) = self.shells.lock() {
+            Ok(shells[self.current_shell()].homedir())
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (homedir)",
+            ))
+        }
     }
 
     pub fn ls(
@@ -130,15 +206,23 @@ impl ShellManager {
         context: &RunnableContext,
         full: bool,
     ) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock().unwrap();
-
-        env[self.current_shell()].ls(path, context, full)
+        if let Ok(shells) = self.shells.lock() {
+            shells[self.current_shell()].ls(path, context, full)
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (ls)",
+            ))
+        }
     }
 
     pub fn cd(&self, args: EvaluatedWholeStreamCommandArgs) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock().unwrap();
-
-        env[self.current_shell()].cd(args)
+        if let Ok(shells) = self.shells.lock() {
+            shells[self.current_shell()].cd(args)
+        } else {
+            Err(ShellError::untagged_runtime_error(
+                "Internal error: could not lock shells ring buffer (cd)",
+            ))
+        }
     }
 
     pub fn cp(
@@ -146,9 +230,9 @@ impl ShellManager {
         args: CopyArgs,
         context: &RunnablePerItemContext,
     ) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock();
+        let shells = self.shells.lock();
 
-        match env {
+        match shells {
             Ok(x) => {
                 let path = x[self.current_shell()].path();
                 x[self.current_shell()].cp(args, context.name.clone(), &path)
@@ -166,9 +250,9 @@ impl ShellManager {
         args: RemoveArgs,
         context: &RunnablePerItemContext,
     ) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock();
+        let shells = self.shells.lock();
 
-        match env {
+        match shells {
             Ok(x) => {
                 let path = x[self.current_shell()].path();
                 x[self.current_shell()].rm(args, context.name.clone(), &path)
@@ -186,9 +270,9 @@ impl ShellManager {
         args: MkdirArgs,
         context: &RunnablePerItemContext,
     ) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock();
+        let shells = self.shells.lock();
 
-        match env {
+        match shells {
             Ok(x) => {
                 let path = x[self.current_shell()].path();
                 x[self.current_shell()].mkdir(args, context.name.clone(), &path)
@@ -206,9 +290,9 @@ impl ShellManager {
         args: MoveArgs,
         context: &RunnablePerItemContext,
     ) -> Result<OutputStream, ShellError> {
-        let env = self.shells.lock();
+        let shells = self.shells.lock();
 
-        match env {
+        match shells {
             Ok(x) => {
                 let path = x[self.current_shell()].path();
                 x[self.current_shell()].mv(args, context.name.clone(), &path)

src/shell/value_shell.rs

@@ -44,7 +44,8 @@ impl ValueShell {
             match p {
                 x if x == sep => {}
                 step => {
-                    let value = viewed.get_data_by_key(step.to_str().unwrap().spanned_unknown());
+                    let name: &str = &step.to_string_lossy().to_string();
+                    let value = viewed.get_data_by_key(name.spanned_unknown());
                     if let Some(v) = value {
                         viewed = v.clone();
                     }