Improve some errors, streamline internal error handling (#2839)

* Improve some errors, streamline internal error handling

* Fix lints
This commit is contained in:
Jonathan Turner 2021-01-02 08:52:19 +13:00 committed by GitHub
parent 48f535f02e
commit 452d8c06e9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 46 additions and 50 deletions

View File

@ -53,12 +53,12 @@ pub(crate) async fn run_internal_command(
Ok(ReturnSuccess::Action(action)) => match action { Ok(ReturnSuccess::Action(action)) => match action {
CommandAction::ChangePath(path) => { CommandAction::ChangePath(path) => {
context.shell_manager.set_path(path); context.shell_manager.set_path(path);
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
CommandAction::Exit => std::process::exit(0), // TODO: save history.txt CommandAction::Exit => std::process::exit(0), // TODO: save history.txt
CommandAction::Error(err) => { CommandAction::Error(err) => {
context.error(err.clone()); context.error(err);
InputStream::one(UntaggedValue::Error(err).into_untagged_value()) InputStream::empty()
} }
CommandAction::AutoConvert(tagged_contents, extension) => { CommandAction::AutoConvert(tagged_contents, extension) => {
let contents_tag = tagged_contents.tag.clone(); let contents_tag = tagged_contents.tag.clone();
@ -117,8 +117,8 @@ pub(crate) async fn run_internal_command(
futures::stream::iter(output).to_input_stream() futures::stream::iter(output).to_input_stream()
} }
Err(e) => { Err(err) => {
context.add_error(e); context.error(err);
InputStream::empty() InputStream::empty()
} }
} }
@ -138,9 +138,8 @@ pub(crate) async fn run_internal_command(
) { ) {
Ok(v) => v, Ok(v) => v,
Err(err) => { Err(err) => {
return InputStream::one( context.error(err);
UntaggedValue::Error(err).into_untagged_value(), return InputStream::empty();
)
} }
}, },
)); ));
@ -151,9 +150,8 @@ pub(crate) async fn run_internal_command(
match HelpShell::index(&context.scope) { match HelpShell::index(&context.scope) {
Ok(v) => v, Ok(v) => v,
Err(err) => { Err(err) => {
return InputStream::one( context.error(err);
UntaggedValue::Error(err).into_untagged_value(), return InputStream::empty();
)
} }
}, },
)); ));
@ -171,10 +169,8 @@ pub(crate) async fn run_internal_command(
match FilesystemShell::with_location(location) { match FilesystemShell::with_location(location) {
Ok(v) => v, Ok(v) => v,
Err(err) => { Err(err) => {
return InputStream::one( context.error(err.into());
UntaggedValue::Error(err.into()) return InputStream::empty();
.into_untagged_value(),
)
} }
}, },
)); ));
@ -198,12 +194,9 @@ pub(crate) async fn run_internal_command(
.await; .await;
if let Err(err) = result { if let Err(err) = result {
return InputStream::one( context.error(err.into());
UntaggedValue::Error(err.into())
.into_untagged_value(),
);
} }
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
Err(_) => { Err(_) => {
context.error(ShellError::labeled_error( context.error(ShellError::labeled_error(
@ -212,7 +205,7 @@ pub(crate) async fn run_internal_command(
filename.span(), filename.span(),
)); ));
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
} }
} }
@ -228,39 +221,37 @@ pub(crate) async fn run_internal_command(
.collect(), .collect(),
); );
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
Err(reason) => { Err(reason) => {
context.error(reason.clone()); context.error(reason);
InputStream::one( InputStream::empty()
UntaggedValue::Error(reason).into_untagged_value(),
)
} }
} }
} }
CommandAction::PreviousShell => { CommandAction::PreviousShell => {
context.shell_manager.prev(); context.shell_manager.prev();
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
CommandAction::NextShell => { CommandAction::NextShell => {
context.shell_manager.next(); context.shell_manager.next();
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
CommandAction::LeaveShell => { CommandAction::LeaveShell => {
context.shell_manager.remove_at_current(); context.shell_manager.remove_at_current();
if context.shell_manager.is_empty() { if context.shell_manager.is_empty() {
std::process::exit(0); // TODO: save history.txt std::process::exit(0); // TODO: save history.txt
} }
InputStream::from_stream(futures::stream::iter(vec![])) InputStream::empty()
} }
}, },
Ok(ReturnSuccess::Value(Value { Ok(ReturnSuccess::Value(Value {
value: UntaggedValue::Error(err), value: UntaggedValue::Error(err),
tag, ..
})) => { })) => {
context.error(err.clone()); context.error(err);
InputStream::one(UntaggedValue::Error(err).into_value(tag)) InputStream::empty()
} }
Ok(ReturnSuccess::Value(v)) => InputStream::one(v), Ok(ReturnSuccess::Value(v)) => InputStream::one(v),
@ -280,8 +271,8 @@ pub(crate) async fn run_internal_command(
} }
Err(err) => { Err(err) => {
context.error(err.clone()); context.error(err);
InputStream::one(UntaggedValue::Error(err).into_untagged_value()) InputStream::empty()
} }
} }
} }

View File

@ -112,11 +112,12 @@ async fn enter(raw_args: CommandArgs) -> Result<OutputStream, ShellError> {
let cwd = shell_manager.path(); let cwd = shell_manager.path();
let full_path = std::path::PathBuf::from(cwd); let full_path = std::path::PathBuf::from(cwd);
let span = location.span();
let (file_extension, tagged_contents) = crate::commands::open::fetch( let (file_extension, tagged_contents) = crate::commands::open::fetch(
&full_path, &full_path,
&PathBuf::from(location_clone), &PathBuf::from(location_clone),
tag.span, span,
encoding, encoding,
) )
.await?; .await?;

View File

@ -42,7 +42,7 @@ impl WholeStreamCommand for SubCommand {
vec![Example { vec![Example {
description: "Load all plugins in the current directory", description: "Load all plugins in the current directory",
example: "nu plugin --load .", example: "nu plugin --load .",
result: Some(vec![]), result: None,
}] }]
} }

View File

@ -188,12 +188,20 @@ pub async fn fetch(
// TODO: I don't understand the point of this? Maybe for better error reporting // TODO: I don't understand the point of this? Maybe for better error reporting
let mut cwd = cwd.clone(); let mut cwd = cwd.clone();
cwd.push(location); cwd.push(location);
let nice_location = dunce::canonicalize(&cwd).map_err(|e| { let nice_location = dunce::canonicalize(&cwd).map_err(|e| match e.kind() {
ShellError::labeled_error( std::io::ErrorKind::NotFound => ShellError::labeled_error(
format!("Cannot canonicalize file {:?} because {:?}", &cwd, e), format!("Cannot find file {:?}", cwd),
"Cannot canonicalize", "cannot find file",
span, span,
) ),
std::io::ErrorKind::PermissionDenied => {
ShellError::labeled_error("Permission denied", "permission denied", span)
}
_ => ShellError::labeled_error(
format!("Cannot open file {:?} because {:?}", &cwd, e),
"Cannot open",
span,
),
})?; })?;
// The extension may be used in AutoConvert later on // The extension may be used in AutoConvert later on

View File

@ -74,10 +74,6 @@ impl EvaluationContext {
self.current_errors.lock().clone() self.current_errors.lock().clone()
} }
pub(crate) fn add_error(&self, err: ShellError) {
self.current_errors.lock().push(err);
}
pub(crate) fn maybe_print_errors(&self, source: Text) -> bool { pub(crate) fn maybe_print_errors(&self, source: Text) -> bool {
let errors = self.current_errors.clone(); let errors = self.current_errors.clone();
let mut errors = errors.lock(); let mut errors = errors.lock();

View File

@ -49,7 +49,7 @@ pub fn test_examples(cmd: Command) -> Result<(), ShellError> {
for sample_pipeline in examples { for sample_pipeline in examples {
let mut ctx = base_context.clone(); let mut ctx = base_context.clone();
let block = parse_line(sample_pipeline.example, &mut ctx)?; let block = parse_line(sample_pipeline.example, &ctx)?;
println!("{:#?}", block); println!("{:#?}", block);
@ -107,7 +107,7 @@ pub fn test(cmd: impl WholeStreamCommand + 'static) -> Result<(), ShellError> {
for sample_pipeline in examples { for sample_pipeline in examples {
let mut ctx = base_context.clone(); let mut ctx = base_context.clone();
let block = parse_line(sample_pipeline.example, &mut ctx)?; let block = parse_line(sample_pipeline.example, &ctx)?;
if let Some(expected) = &sample_pipeline.result { if let Some(expected) = &sample_pipeline.result {
let result = block_on(evaluate_block(block, &mut ctx))?; let result = block_on(evaluate_block(block, &mut ctx))?;
@ -171,7 +171,7 @@ pub fn test_anchors(cmd: Command) -> Result<(), ShellError> {
let mut ctx = base_context.clone(); let mut ctx = base_context.clone();
let block = parse_line(&pipeline_with_anchor, &mut ctx)?; let block = parse_line(&pipeline_with_anchor, &ctx)?;
let result = block_on(evaluate_block(block, &mut ctx))?; let result = block_on(evaluate_block(block, &mut ctx))?;
ctx.with_errors(|reasons| reasons.iter().cloned().take(1).next()) ctx.with_errors(|reasons| reasons.iter().cloned().take(1).next())

View File

@ -80,6 +80,6 @@ fn errors_if_file_not_found() {
"enter i_dont_exist.csv" "enter i_dont_exist.csv"
); );
assert!(actual.err.contains("Cannot canonicalize")); assert!(actual.err.contains("Cannot find file"));
}) })
} }

View File

@ -222,7 +222,7 @@ fn errors_if_file_not_found() {
cwd: "tests/fixtures/formats", cwd: "tests/fixtures/formats",
"open i_dont_exist.txt" "open i_dont_exist.txt"
); );
let expected = "Cannot canonicalize"; let expected = "Cannot find file";
assert!( assert!(
actual.err.contains(expected), actual.err.contains(expected),
"Error:\n{}\ndoes not contain{}", "Error:\n{}\ndoes not contain{}",