[pbs-devel] [PATCH proxmox-backup] client: change anyhow Error display representation

[pbs-devel] [PATCH proxmox-backup] client: change anyhow Error display representation

[Christian Ebner]

Improve error message output by showing the full Error context, using
the alternate selector '{:#}" [0].
Without this, only the outermost context is displayed, which in case
of pxar extraction errors is mostly not enough to find the underlying
issue.

[0] https://docs.rs/anyhow/1.0.69/anyhow/struct.Error.html#display-representations

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Current error context just gives to little information, as for example
see here https://forum.proxmox.com/threads/136341/

 proxmox-backup-client/src/main.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 1a13291a..9b680f62 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -1453,7 +1453,7 @@ async fn restore(
                 },
                 options,
             )
-            .map_err(|err| format_err!("error extracting archive - {}", err))?;
+            .map_err(|err| format_err!("error extracting archive - {:#}", err))?;
         } else {
             let mut writer = std::fs::OpenOptions::new()
                 .write(true)
-- 
2.39.2

[pbs-devel] applied: [PATCH proxmox-backup] client: change anyhow Error display representation

[Thomas Lamprecht]

Am 25/11/2023 um 11:13 schrieb Christian Ebner:
> Improve error message output by showing the full Error context, using
> the alternate selector '{:#}" [0].
> Without this, only the outermost context is displayed, which in case
> of pxar extraction errors is mostly not enough to find the underlying
> issue.
> 
> [0] https://docs.rs/anyhow/1.0.69/anyhow/struct.Error.html#display-representations
> 
> Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
> ---
> Current error context just gives to little information, as for example
> see here https://forum.proxmox.com/threads/136341/
> 
>  proxmox-backup-client/src/main.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)


applied, thanks!

> diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
> index 1a13291a..9b680f62 100644
> --- a/proxmox-backup-client/src/main.rs
> +++ b/proxmox-backup-client/src/main.rs
> @@ -1453,7 +1453,7 @@ async fn restore(
>                  },
>                  options,
>              )
> -            .map_err(|err| format_err!("error extracting archive - {}", err))?;
> +            .map_err(|err| format_err!("error extracting archive - {:#}", err))?;

Two things though:

First, for touching existing formats you're encouraged to move simple variables
inside the format string directly, e.g.:

format_err!("error extracting archive - {err:#}")

but really not a blocker, so I just applied as is, just try to keep that
in mind for future changes.

Second, could be maybe better to see if we can explicitly get the inner
error and format/present that, but for now this is an improvement and
I did not really checked out the whole context of the call graph here,
so take this just as "our rust error handling story might be improved
in general".

>          } else {
>              let mut writer = std::fs::OpenOptions::new()
>                  .write(true)