From ca322b761aa0ac385ca752d530df58b67c184b28 Mon Sep 17 00:00:00 2001 From: Yuya Nishihara Date: Wed, 31 Jan 2024 12:33:23 +0900 Subject: [PATCH] cli: print source chain of internal error in a similar way to anyhow Multi-line output should be easier to follow than lengthy line separated by colon. --- cli/src/cli_util.rs | 34 +++++++++++++++++++++++++++++++--- cli/tests/test_edit_command.rs | 6 +++++- cli/tests/test_global_opts.rs | 29 ++++++++++++++++++++++------- 3 files changed, 58 insertions(+), 11 deletions(-) diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index d84c302d1..356911f62 100644 --- a/cli/src/cli_util.rs +++ b/cli/src/cli_util.rs @@ -105,7 +105,7 @@ pub enum CommandError { /// Wraps error with user-visible message. #[derive(Debug, Error)] -#[error("{message}: {source}")] +#[error("{message}")] struct ErrorWithMessage { message: String, source: Box, @@ -160,6 +160,33 @@ fn format_similarity_hint>(candidates: &[S]) -> Option { } } +fn print_error_sources(ui: &Ui, source: Option<&dyn std::error::Error>) -> io::Result<()> { + let Some(err) = source else { + return Ok(()); + }; + if err.source().is_none() { + writeln!(ui.stderr(), "Caused by: {err}")?; + } else { + writeln!(ui.stderr(), "Caused by:")?; + for (i, err) in iter::successors(Some(err), |err| err.source()).enumerate() { + let message = strip_error_source(err); + writeln!(ui.stderr(), "{n}: {message}", n = i + 1)?; + } + } + Ok(()) +} + +// TODO: remove ": {source}" from error types and drop this hack +fn strip_error_source(err: &dyn std::error::Error) -> String { + let mut message = err.to_string(); + if let Some(source) = err.source() { + if let Some(s) = message.strip_suffix(&format!(": {source}")) { + message.truncate(s.len()); + } + } + message +} + impl From for CommandError { fn from(err: std::io::Error) -> Self { if err.kind() == std::io::ErrorKind::BrokenPipe { @@ -2793,7 +2820,7 @@ pub fn handle_command_result( ui: &mut Ui, result: Result<(), CommandError>, ) -> std::io::Result { - match result { + match &result { Ok(()) => Ok(ExitCode::SUCCESS), Err(CommandError::UserError { message, hint }) => { writeln!(ui.error(), "Error: {message}")?; @@ -2846,7 +2873,8 @@ pub fn handle_command_result( Ok(ExitCode::from(BROKEN_PIPE_EXIT_CODE)) } Err(CommandError::InternalError(err)) => { - writeln!(ui.error(), "Internal error: {err}")?; + writeln!(ui.error(), "Internal error: {}", strip_error_source(err))?; + print_error_sources(ui, err.source())?; Ok(ExitCode::from(255)) } } diff --git a/cli/tests/test_edit_command.rs b/cli/tests/test_edit_command.rs index b3cb418d1..422e4b442 100644 --- a/cli/tests/test_edit_command.rs +++ b/cli/tests/test_edit_command.rs @@ -104,7 +104,11 @@ fn test_edit_current_wc_commit_missing() { .assert() .code(255); insta::assert_snapshot!(get_stderr_string(&assert), @r###" - Internal error: Failed to edit a commit: Current working-copy commit not found: Object 69542c1984c1f9d91f7c6c9c9e6941782c944bd9 of type commit not found: An object with id 69542c1984c1f9d91f7c6c9c9e6941782c944bd9 could not be found + Internal error: Failed to edit a commit + Caused by: + 1: Current working-copy commit not found + 2: Object 69542c1984c1f9d91f7c6c9c9e6941782c944bd9 of type commit not found + 3: An object with id 69542c1984c1f9d91f7c6c9c9e6941782c944bd9 could not be found "###); } diff --git a/cli/tests/test_global_opts.rs b/cli/tests/test_global_opts.rs index 93f01f3f1..20114956b 100644 --- a/cli/tests/test_global_opts.rs +++ b/cli/tests/test_global_opts.rs @@ -219,15 +219,18 @@ fn test_broken_repo_structure() { // Test the error message when the git repository can't be located. std::fs::remove_file(store_path.join("git_target")).unwrap(); let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); - insta::assert_snapshot!(stderr, @r###" - Internal error: The repository appears broken or inaccessible: Cannot access $TEST_ENV/repo/.jj/repo/store/git_target + insta::assert_snapshot!(strip_last_line(&stderr), @r###" + Internal error: The repository appears broken or inaccessible + Caused by: + 1: Cannot access $TEST_ENV/repo/.jj/repo/store/git_target "###); // Test the error message when the commit backend is of unknown type. std::fs::write(&store_type_path, "unknown").unwrap(); let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); insta::assert_snapshot!(stderr, @r###" - Internal error: This version of the jj binary doesn't support this type of repo: Unsupported commit backend type 'unknown' + Internal error: This version of the jj binary doesn't support this type of repo + Caused by: Unsupported commit backend type 'unknown' "###); // Test the error message when the file indicating the commit backend type @@ -235,8 +238,11 @@ fn test_broken_repo_structure() { std::fs::remove_file(&store_type_path).unwrap(); std::fs::create_dir(&store_type_path).unwrap(); let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); - insta::assert_snapshot!(stderr, @r###" - Internal error: The repository appears broken or inaccessible: Failed to read commit backend type: Cannot access $TEST_ENV/repo/.jj/repo/store/type + insta::assert_snapshot!(strip_last_line(&stderr), @r###" + Internal error: The repository appears broken or inaccessible + Caused by: + 1: Failed to read commit backend type + 2: Cannot access $TEST_ENV/repo/.jj/repo/store/type "###); // Test when the .jj directory is empty. The error message is identical to @@ -244,8 +250,11 @@ fn test_broken_repo_structure() { std::fs::remove_dir_all(repo_path.join(".jj")).unwrap(); std::fs::create_dir(repo_path.join(".jj")).unwrap(); let stderr = test_env.jj_cmd_internal_error(&repo_path, &["log"]); - insta::assert_snapshot!(stderr, @r###" - Internal error: The repository appears broken or inaccessible: Failed to read commit backend type: Cannot access $TEST_ENV/repo/.jj/repo/store/type + insta::assert_snapshot!(strip_last_line(&stderr), @r###" + Internal error: The repository appears broken or inaccessible + Caused by: + 1: Failed to read commit backend type + 2: Cannot access $TEST_ENV/repo/.jj/repo/store/type "###); } @@ -456,3 +465,9 @@ fn test_verbose_logging_enabled() { // Luckily, insta will print this in colour when reviewing. insta::assert_snapshot!(log_line, @" INFO jj_cli::cli_util: verbose logging enabled"); } + +fn strip_last_line(s: &str) -> &str { + s.trim_end_matches('\n') + .rsplit_once('\n') + .map_or(s, |(h, _)| &s[..h.len() + 1]) +}