[pbs-devel] [RFC proxmox] CLI: print fatal errors including causes

[Fabian Grünbichler]

as a first step of improving our error handling story, printing context and
causes if the error contains them.

For example, with two calls to `.with_context` when bubbling up errors in
proxmox-offline-mirror:

----8<----
diff --git a/src/bin/proxmox-offline-mirror.rs b/src/bin/proxmox-offline-mirror.rs
index bec366a..403a2f5 100644
--- a/src/bin/proxmox-offline-mirror.rs
+++ b/src/bin/proxmox-offline-mirror.rs
@@ -1,7 +1,7 @@
 use std::fmt::Display;
 use std::path::Path;

-use anyhow::{bail, Error};
+use anyhow::{bail, format_err, Context, Error};
 use serde_json::Value;

 use proxmox_router::cli::{run_cli_command, CliCommand, CliCommandMap, CliEnvironment};
@@ -676,7 +676,8 @@ async fn setup(config: Option<String>, _param: Value) -> Result<(), Error> {
             Action::AddMirror => {
                 for mirror_config in action_add_mirror(&config)? {
                     let id = mirror_config.id.clone();
-                    mirror::init(&mirror_config)?;
+                    mirror::init(&mirror_config)
+                        .with_context(|| format!("Failed to initialize mirror '{id}'"))?;
                     config.set_data(&id, "mirror", mirror_config)?;
                     save_config(&config_file, &config)?;
                     println!("Config entry '{id}' added");
diff --git a/src/pool.rs b/src/pool.rs
index 3da8c08..ecf3f6f 100644
--- a/src/pool.rs
+++ b/src/pool.rs
@@ -7,7 +7,7 @@ use std::{
     path::{Path, PathBuf},
 };

-use anyhow::{bail, format_err, Error};
+use anyhow::{bail, format_err, Context, Error};
 use nix::{unistd, NixPath};

 use proxmox_apt::deb822::CheckSums;
@@ -45,10 +45,12 @@ impl Pool {
         }

         if !pool.exists() {
-            create_path(pool, None, None)?;
+            create_path(pool, None, None)
+                .with_context(|| format!("Failed to create pool dir {pool:?}"))?;
         }

-        create_path(link_dir, None, None)?;
+        create_path(link_dir, None, None)
+            .with_context(|| format!("Failed to create link dir {link_dir:?}"))?;

         Ok(Self {
             pool_dir: pool.to_path_buf(),
---->8----

we'd get the following output:

Error: Failed to initialize mirror 'debian_bullseye_main'

Caused by:
    0: Failed to create pool dir "/var/lib/proxmox-offline-mirror/mirrors//.pool"
    1: EACCES: Permission denied

instead of the original

Error: EACCESS: Permission denied

which is not really helpful without knowing the path.

For non-fatal cases or logging inside tasks, `{:#}` could be used which just
prints the causes/contexts in a single line like this:

Failed to initialize mirror 'debian_bullseye_main': Failed to create pool dir "/var/lib/proxmox-offline-mirror/mirrors//.pool": EACCES: Permission denied

but for that usage, the context should be kept short to avoid the line getting overly long.

One downside to adding context is that the default Display implementation will
*just* print the context, which hides the root cause:

Error: Failed to initialize mirror 'debian_bullseye_main'

When adding context to existing error handling (or when replacing manual
context adding via format_err!), call sites need to be adapted to ensure the
causes are not accidentally hidden.

Signed-off-by: Fabian Grünbichler <f.gruenbichler at proxmox.com>
---
RFC despite the off-list echo being positive - ideally we would combine this
with using thiserror or something similar to also define nice error types at
certain boundaries between our application layer and lower level lib code,
which would help with keeping the error chain length manageable..

 proxmox-router/src/cli/command.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-router/src/cli/command.rs b/proxmox-router/src/cli/command.rs
index 8ae889f..0d43850 100644
--- a/proxmox-router/src/cli/command.rs
+++ b/proxmox-router/src/cli/command.rs
@@ -83,7 +83,7 @@ async fn handle_simple_command_future(
             }
         }
         Err(err) => {
-            eprintln!("Error: {}", err);
+            eprintln!("Error: {:?}", err);
             return Err(err);
         }
     }
@@ -135,7 +135,7 @@ fn handle_simple_command(
             }
         }
         Err(err) => {
-            eprintln!("Error: {}", err);
+            eprintln!("Error: {:?}", err);
             return Err(err);
         }
     }
-- 
2.30.2