[pbs-devel] [PATCH proxmox 1/4] CLI: print fatal errors including causes

[Gabriel Goller]

From: Fabian Grünbichler <f.gruenbichler at proxmox.com>

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>
Signed-off-by: Gabriel Goller <g.goller at proxmox.com>
---
 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 7a26ffb..d5522f1 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.43.0