From 4ed522db934337202fc2e8fb4fd3a4d80e0fe711 Mon Sep 17 00:00:00 2001 From: 132ikl <132@ikl.sh> Date: Tue, 15 Jul 2025 06:42:10 -0400 Subject: [PATCH] Add default error codes (#16166) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit # Description Before this PR, errors without error codes are printed somewhat strangely, with the `×` and the description being printed on the same line as the `Error:` text: Error: × Invalid literal ╭─[entry #1:1:2] 1 │ "\z" · ─┬─ · ╰── unrecognized escape after '\' in string ╰──── This PR adds a default error code for the different error types: Error: nu::parser::error × Invalid literal ╭─[entry #1:1:2] 1 │ "\z" · ─┬─ · ╰── unrecognized escape after '\' in string ╰──── While maybe not as informative as a proper error code, it makes `GenericError`s and other things which don't have error codes look a lot nicer. It would be nicer if we could just set `diagnostic(code: "nu::shell::error")` at the top of `ShellError`, but unfortunately you can't set a "default" at the `enum` level and then override it in the variants. @cptpiepmatz mentioned he might change miette's derive macro to accommodate this, in that case we can switch the approach here. # User-Facing Changes * Errors without error codes now have a default error code corresponding to from which part of Nushell the error occurred (shell, parser, compile, etc) --------- Co-authored-by: Bahex <17417311+Bahex@users.noreply.github.com> --- crates/nu-command/src/filesystem/watch.rs | 17 +-- crates/nu-protocol/src/errors/cli_error.rs | 101 +++++++++++++----- .../nu-protocol/src/errors/shell_error/mod.rs | 4 +- crates/nu-protocol/tests/test_config.rs | 3 +- 4 files changed, 81 insertions(+), 44 deletions(-) diff --git a/crates/nu-command/src/filesystem/watch.rs b/crates/nu-command/src/filesystem/watch.rs index f86c1ec349..081054c6c0 100644 --- a/crates/nu-command/src/filesystem/watch.rs +++ b/crates/nu-command/src/filesystem/watch.rs @@ -6,11 +6,7 @@ use notify_debouncer_full::{ }, }; use nu_engine::{ClosureEval, command_prelude::*}; -use nu_protocol::{ - engine::{Closure, StateWorkingSet}, - format_cli_error, - shell_error::io::IoError, -}; +use nu_protocol::{engine::Closure, report_shell_error, shell_error::io::IoError}; use std::{ path::PathBuf, sync::mpsc::{RecvTimeoutError, channel}, @@ -203,14 +199,9 @@ impl Command for Watch { .run_with_input(PipelineData::Empty); match result { - Ok(val) => { - val.print_table(engine_state, stack, false, false)?; - } - Err(err) => { - let working_set = StateWorkingSet::new(engine_state); - eprintln!("{}", format_cli_error(&working_set, &err)); - } - } + Ok(val) => val.print_table(engine_state, stack, false, false)?, + Err(err) => report_shell_error(engine_state, &err), + }; } Ok(()) diff --git a/crates/nu-protocol/src/errors/cli_error.rs b/crates/nu-protocol/src/errors/cli_error.rs index 7b899d77f0..2617709742 100644 --- a/crates/nu-protocol/src/errors/cli_error.rs +++ b/crates/nu-protocol/src/errors/cli_error.rs @@ -17,11 +17,27 @@ use thiserror::Error; /// This error exists so that we can defer SourceCode handling. It simply /// forwards most methods, except for `.source_code()`, which we provide. #[derive(Error)] -#[error("{0}")] -struct CliError<'src>( - pub &'src dyn miette::Diagnostic, - pub &'src StateWorkingSet<'src>, -); +#[error("{diagnostic}")] +struct CliError<'src> { + diagnostic: &'src dyn miette::Diagnostic, + working_set: &'src StateWorkingSet<'src>, + // error code to use if `diagnostic` doesn't provide one + default_code: Option<&'static str>, +} + +impl<'src> CliError<'src> { + pub fn new( + diagnostic: &'src dyn miette::Diagnostic, + working_set: &'src StateWorkingSet<'src>, + default_code: Option<&'static str>, + ) -> Self { + CliError { + diagnostic, + working_set, + default_code, + } + } +} #[derive(Default)] pub struct ReportLog { @@ -63,45 +79,64 @@ fn should_show_warning(engine_state: &EngineState, warning: &ParseWarning) -> bo } } -pub fn format_cli_error(working_set: &StateWorkingSet, error: &dyn miette::Diagnostic) -> String { - format!("Error: {:?}", CliError(error, working_set)) +pub fn format_cli_error( + working_set: &StateWorkingSet, + error: &dyn miette::Diagnostic, + default_code: Option<&'static str>, +) -> String { + format!( + "Error: {:?}", + CliError::new(error, working_set, default_code) + ) } pub fn report_shell_error(engine_state: &EngineState, error: &ShellError) { if engine_state.config.display_errors.should_show(error) { - report_error(&StateWorkingSet::new(engine_state), error) + let working_set = StateWorkingSet::new(engine_state); + report_error(&working_set, error, "nu::shell::error") } } pub fn report_shell_warning(engine_state: &EngineState, warning: &ShellError) { if engine_state.config.display_errors.should_show(warning) { - report_warning(&StateWorkingSet::new(engine_state), warning) + report_warning( + &StateWorkingSet::new(engine_state), + warning, + "nu::shell::warning", + ) } } pub fn report_parse_error(working_set: &StateWorkingSet, error: &ParseError) { - report_error(working_set, error); + report_error(working_set, error, "nu::parser::error"); } pub fn report_parse_warning(working_set: &StateWorkingSet, warning: &ParseWarning) { if should_show_warning(working_set.permanent(), warning) { - report_warning(working_set, warning); + report_warning(working_set, warning, "nu::parser::warning"); } } pub fn report_compile_error(working_set: &StateWorkingSet, error: &CompileError) { - report_error(working_set, error); + report_error(working_set, error, "nu::compile::error"); } pub fn report_experimental_option_warning( working_set: &StateWorkingSet, warning: &dyn miette::Diagnostic, ) { - report_warning(working_set, warning); + report_warning(working_set, warning, "nu::experimental_option::warning"); } -fn report_error(working_set: &StateWorkingSet, error: &dyn miette::Diagnostic) { - eprintln!("Error: {:?}", CliError(error, working_set)); +fn report_error( + working_set: &StateWorkingSet, + error: &dyn miette::Diagnostic, + default_code: &'static str, +) { + eprintln!( + "Error: {:?}", + CliError::new(error, working_set, Some(default_code)) + ); // reset vt processing, aka ansi because illbehaved externals can break it #[cfg(windows)] { @@ -109,8 +144,15 @@ fn report_error(working_set: &StateWorkingSet, error: &dyn miette::Diagnostic) { } } -fn report_warning(working_set: &StateWorkingSet, warning: &dyn miette::Diagnostic) { - eprintln!("Warning: {:?}", CliError(warning, working_set)); +fn report_warning( + working_set: &StateWorkingSet, + warning: &dyn miette::Diagnostic, + default_code: &'static str, +) { + eprintln!( + "Warning: {:?}", + CliError::new(warning, working_set, Some(default_code)) + ); // reset vt processing, aka ansi because illbehaved externals can break it #[cfg(windows)] { @@ -120,9 +162,9 @@ fn report_warning(working_set: &StateWorkingSet, warning: &dyn miette::Diagnosti impl std::fmt::Debug for CliError<'_> { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let config = self.1.get_config(); + let config = self.working_set.get_config(); - let ansi_support = config.use_ansi_coloring.get(self.1.permanent()); + let ansi_support = config.use_ansi_coloring.get(self.working_set.permanent()); let error_style = &config.error_style; @@ -150,39 +192,42 @@ impl std::fmt::Debug for CliError<'_> { impl miette::Diagnostic for CliError<'_> { fn code<'a>(&'a self) -> Option> { - self.0.code() + self.diagnostic.code().or_else(|| { + self.default_code + .map(|code| Box::new(code) as Box) + }) } fn severity(&self) -> Option { - self.0.severity() + self.diagnostic.severity() } fn help<'a>(&'a self) -> Option> { - self.0.help() + self.diagnostic.help() } fn url<'a>(&'a self) -> Option> { - self.0.url() + self.diagnostic.url() } fn labels<'a>(&'a self) -> Option + 'a>> { - self.0.labels() + self.diagnostic.labels() } // Finally, we redirect the source_code method to our own source. fn source_code(&self) -> Option<&dyn SourceCode> { - if let Some(source_code) = self.0.source_code() { + if let Some(source_code) = self.diagnostic.source_code() { Some(source_code) } else { - Some(&self.1) + Some(&self.working_set) } } fn related<'a>(&'a self) -> Option + 'a>> { - self.0.related() + self.diagnostic.related() } fn diagnostic_source(&self) -> Option<&dyn miette::Diagnostic> { - self.0.diagnostic_source() + self.diagnostic.diagnostic_source() } } diff --git a/crates/nu-protocol/src/errors/shell_error/mod.rs b/crates/nu-protocol/src/errors/shell_error/mod.rs index cc0f76d040..68cc79358e 100644 --- a/crates/nu-protocol/src/errors/shell_error/mod.rs +++ b/crates/nu-protocol/src/errors/shell_error/mod.rs @@ -1418,7 +1418,7 @@ impl ShellError { "msg" => Value::string(self.to_string(), span), "debug" => Value::string(format!("{self:?}"), span), "raw" => Value::error(self.clone(), span), - "rendered" => Value::string(format_cli_error(working_set, &self), span), + "rendered" => Value::string(format_cli_error(working_set, &self, Some("nu::shell::error")), span), "json" => Value::string(serde_json::to_string(&self).expect("Could not serialize error"), span), }; @@ -1431,7 +1431,7 @@ impl ShellError { // TODO: Implement as From trait pub fn wrap(self, working_set: &StateWorkingSet, span: Span) -> ParseError { - let msg = format_cli_error(working_set, &self); + let msg = format_cli_error(working_set, &self, None); ParseError::LabeledError( msg, "Encountered error during parse-time evaluation".into(), diff --git a/crates/nu-protocol/tests/test_config.rs b/crates/nu-protocol/tests/test_config.rs index 49ad1cdec6..632dd1ff47 100644 --- a/crates/nu-protocol/tests/test_config.rs +++ b/crates/nu-protocol/tests/test_config.rs @@ -60,7 +60,7 @@ fn fancy_default_errors() { assert_eq!( actual.err, - "Error: \u{1b}[31m×\u{1b}[0m oh no!\n ╭─[\u{1b}[36;1;4mline2:1:13\u{1b}[0m]\n \u{1b}[2m1\u{1b}[0m │ force_error \"My error\"\n · \u{1b}[35;1m ─────┬────\u{1b}[0m\n · \u{1b}[35;1m╰── \u{1b}[35;1mhere's the error\u{1b}[0m\u{1b}[0m\n ╰────\n\n" + "Error: \u{1b}[31mnu::shell::error\u{1b}[0m\n\n \u{1b}[31m×\u{1b}[0m oh no!\n ╭─[\u{1b}[36;1;4mline2:1:13\u{1b}[0m]\n \u{1b}[2m1\u{1b}[0m │ force_error \"My error\"\n · \u{1b}[35;1m ─────┬────\u{1b}[0m\n · \u{1b}[35;1m╰── \u{1b}[35;1mhere's the error\u{1b}[0m\u{1b}[0m\n ╰────\n\n" ); } @@ -90,6 +90,7 @@ Begin snippet for line2 starting at line 1, column 1 snippet line 1: force_error "my error" label at line 1, columns 13 to 22: here's the error +diagnostic code: nu::shell::error "#,