[pbs-devel] [PATCH v2 proxmox{, -backup} 0/4] output full anyhow context in client

[pbs-devel] [PATCH v2 proxmox{, -backup} 0/4] output full anyhow context in client

[Gabriel Goller]

Instead of using format_err! and format! to create error messages, we
want to move to anyhow::Context and add information to already
existing anyhow::Error's. 
Before we start to gradually phase out the format! error calls, we
need to print the whole context + error, because the default 
anyhow::Error Display implementation doesn't print the actual error.

This series starts with the proxmox-backup-client and includes a patch
from Fabian that introduces pretty-printing of errors (including 
the context) and some other high-level stuff from me. The scope 
is not to remove every format call and add a context everywhere, 
but to enable this change in the future.

Changelog v2:
 - moved explanation to readme.rst file
 - remove unwrap, use take() in `if let`
 - cargo fmt 



proxmox:

Fabian Grünbichler (1):
  CLI: print fatal errors including causes

 proxmox-router/README.rst         | 95 +++++++++++++++++++++++++++++++
 proxmox-router/src/cli/command.rs |  4 +-
 2 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 proxmox-router/README.rst


proxmox-backup:

Gabriel Goller (3):
  pxar: remove ArchiveError
  pxar: add UniqueContext helper
  pxar: use anyhow::Error in PxarBackupStream

 pbs-client/src/pxar/create.rs        | 48 +++++++++++++---------------
 pbs-client/src/pxar_backup_stream.rs | 18 +++++------
 2 files changed, 32 insertions(+), 34 deletions(-)


Summary over all repositories:
  4 files changed, 129 insertions(+), 36 deletions(-)

-- 
murpp v0.4.0

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

[Gabriel Goller]

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

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

The downside to adding context is that the default Display implementation
will *just* print the context, which hides the root cause. This is why
we print the errors using the pretty-print formatter in this change.

More info in `proxmox-router/README.rst`.

Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-router/README.rst         | 95 +++++++++++++++++++++++++++++++
 proxmox-router/src/cli/command.rs |  4 +-
 2 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 proxmox-router/README.rst

diff --git a/proxmox-router/README.rst b/proxmox-router/README.rst
new file mode 100644
index 0000000..9c7c213
--- /dev/null
+++ b/proxmox-router/README.rst
@@ -0,0 +1,95 @@
+================
+ proxmox-router
+================
+
+cli
+==================
+To improve our error handling story, we use anyhow and `.context()` on
+all errors. This means no more `format_err()` and `format()` of errors.
+
+For example, with two calls to `.with_context` when bubbling up errors in
+proxmox-offline-mirror:
+
+.. code-block::
+    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(),
+
+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.
+
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

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

[Max Carrara]

On 2/20/24 11:28, Gabriel Goller wrote:
> From: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> 
> as a first step of improving our error handling story, printing context
> and causes if the error contains them.
> 
> The downside to adding context is that the default Display implementation
> will *just* print the context, which hides the root cause. This is why
> we print the errors using the pretty-print formatter in this change.
> 
> More info in `proxmox-router/README.rst`.
> 
> Signed-off-by: Fabian Grünbichler <f.gruenbichler@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---

There are some lines in `README.rst` that contain only whitespace and some
things regarding ReST formatting. See comments inline.

As these things are non-issues IMO and easily fixed, I'll send in a follow-up fixup
that may be applied and rebased if desired.

>  proxmox-router/README.rst         | 95 +++++++++++++++++++++++++++++++
>  proxmox-router/src/cli/command.rs |  4 +-
>  2 files changed, 97 insertions(+), 2 deletions(-)
>  create mode 100644 proxmox-router/README.rst
> 
> diff --git a/proxmox-router/README.rst b/proxmox-router/README.rst
> new file mode 100644
> index 0000000..9c7c213
> --- /dev/null
> +++ b/proxmox-router/README.rst
> @@ -0,0 +1,95 @@
> +================
> + proxmox-router
> +================

AFAIK, our top-level sections consist of the title just being underlined with '='s, as in:

proxmox-router
==============

> +
> +cli
> +==================

This should then instead be underlined with '-' - and the title could be prettier IMO:

Command Line
------------

> +To improve our error handling story, we use anyhow and `.context()` on
> +all errors. This means no more `format_err()` and `format()` of errors.
> +
> +For example, with two calls to `.with_context` when bubbling up errors in
> +proxmox-offline-mirror:

In this paragraph (and others) you're using backticks to denote code, but those actually
mark something as "interpreted text" [0]. Instead, double backticks (e.g. ``.with_context``)
are used to mark something as an inline literal [1] (oiw. code).

I know this may be somewhat annoying (as it's not the same as in Markdown), but that's
just how the format works.

[0]: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#interpreted-text
[1]: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#inline-literals

> +
> +.. code-block::
> +    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;
> +    

Whitespace in this line here ^

> +    -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},
> +     };
> +    

... and here ^ and also in the remaining inline diff below.

> +    -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(),
> +
> +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

Double backticks should be used here as well ...

> +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

... and here.

> +*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.
> +
> 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);
>          }
>      }

[pbs-devel] [PATCH proxmox] fixup! CLI: print fatal errors including causes

[Max Carrara]

---
 proxmox-router/README.rst | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/proxmox-router/README.rst b/proxmox-router/README.rst
index 9c7c2139..5fef633f 100644
--- a/proxmox-router/README.rst
+++ b/proxmox-router/README.rst
@@ -1,13 +1,13 @@
-================
- proxmox-router
-================
+proxmox-router
+==============
 
-cli
-==================
-To improve our error handling story, we use anyhow and `.context()` on
-all errors. This means no more `format_err()` and `format()` of errors.
+Command Line
+------------
 
-For example, with two calls to `.with_context` when bubbling up errors in
+To improve our error handling story, we use anyhow and ``.context()`` on
+all errors. This means no more ``format_err()`` and ``format()`` of errors.
+
+For example, with two calls to ``.with_context`` when bubbling up errors in
 proxmox-offline-mirror:
 
 .. code-block::
@@ -18,11 +18,11 @@ proxmox-offline-mirror:
     @@ -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 => {
@@ -41,7 +41,7 @@ proxmox-offline-mirror:
     @@ -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};
@@ -49,7 +49,7 @@ proxmox-offline-mirror:
      use proxmox_apt::deb822::CheckSums;
     @@ -45,10 +45,12 @@ impl Pool {
              }
-    
+
              if !pool.exists() {
     -            create_path(pool, None, None)?;
     +            create_path(pool, None, None)
@@ -59,7 +59,7 @@ proxmox-offline-mirror:
     -        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(),
 
@@ -77,14 +77,14 @@ Instead of the original::
 
 Which is not really helpful without knowing the path.
 
-For non-fatal cases or logging inside tasks, `{:#}` could be used which just
+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
+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'
-- 
2.39.2

[pbs-devel] [PATCH v2 proxmox-backup 2/4] pxar: remove ArchiveError

[Gabriel Goller]

The sole purpose of the ArchiveError was to add the file-path to the
error. Using anyhow::Error we can add this information using the context
and don't need this struct anymore.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 26 +-------------------------
 1 file changed, 1 insertion(+), 25 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 75376c0c..06f396e0 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -1,6 +1,5 @@
 use std::collections::{HashMap, HashSet};
 use std::ffi::{CStr, CString, OsStr};
-use std::fmt;
 use std::io::{self, Read};
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd};
@@ -89,25 +88,11 @@ pub fn is_virtual_file_system(magic: i64) -> bool {
         SYSFS_MAGIC)
 }
 
-#[derive(Debug)]
-struct ArchiveError {
-    path: PathBuf,
-    error: Error,
 }
 
-impl ArchiveError {
-    fn new(path: PathBuf, error: Error) -> Self {
-        Self { path, error }
     }
 }
 
-impl std::error::Error for ArchiveError {}
-
-impl fmt::Display for ArchiveError {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "error at {:?}: {}", self.path, self.error)
-    }
-}
 
 #[derive(Eq, PartialEq, Hash)]
 struct HardLinkInfo {
@@ -219,14 +204,6 @@ impl Archiver {
         self.feature_flags & self.fs_feature_flags
     }
 
-    fn wrap_err(&self, err: Error) -> Error {
-        if err.downcast_ref::<ArchiveError>().is_some() {
-            err
-        } else {
-            ArchiveError::new(self.path.clone(), err).into()
-        }
-    }
-
     fn archive_dir_contents<'a, T: SeqWrite + Send>(
         &'a mut self,
         encoder: &'a mut Encoder<'_, T>,
@@ -265,8 +242,7 @@ impl Archiver {
                 (self.callback)(&file_entry.path)?;
                 self.path = file_entry.path;
                 self.add_entry(encoder, dir_fd, &file_entry.name, &file_entry.stat)
-                    .await
-                    .map_err(|err| self.wrap_err(err))?;
+                    .await.context(format!("error at {:?}", self.path))?;
             }
             self.path = old_path;
             self.entry_counter = entry_counter;
-- 
2.43.0

[pbs-devel] [PATCH v2 proxmox-backup 3/4] pxar: add UniqueContext helper

[Gabriel Goller]

To create a pxar archive, we recursively traverse the target folder.
If there is an error further down and we add a context using anyhow,
the context will be duplicated and we get an output like:

> Error: error at "xattr/xattr.txt": error at "xattr/xattr.txt": E2BIG [skip]

This is obviously not optimal, so in recursive contexts we can use the
UniqueContext, which quickly checks the context from the last item in
the error chain and only adds it if it is unique.

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 06f396e0..679b3c62 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -1,5 +1,6 @@
 use std::collections::{HashMap, HashSet};
 use std::ffi::{CStr, CString, OsStr};
+use std::fmt::Display;
 use std::io::{self, Read};
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, OwnedFd, RawFd};
@@ -88,12 +89,32 @@ pub fn is_virtual_file_system(magic: i64) -> bool {
         SYSFS_MAGIC)
 }
 
+trait UniqueContext<T> {
+    fn unique_context<S>(self, context: S) -> Result<T, anyhow::Error>
+    where
+        S: Display + Send + Sync + 'static;
 }
 
+impl<T> UniqueContext<T> for Result<T, anyhow::Error> {
+    fn unique_context<S>(self, context: S) -> Result<T, anyhow::Error>
+    where
+        S: Display + Send + Sync + 'static,
+    {
+        match self {
+            Ok(ok) => Ok(ok),
+            Err(err) => {
+                let last_error = err.chain().next();
+                if let Some(e) = last_error {
+                    if e.to_string() == context.to_string() {
+                        return Err(err);
+                    }
+                }
+                Err(err.context(context))
+            }
+        }
     }
 }
 
-
 #[derive(Eq, PartialEq, Hash)]
 struct HardLinkInfo {
     st_dev: u64,
@@ -242,7 +263,8 @@ impl Archiver {
                 (self.callback)(&file_entry.path)?;
                 self.path = file_entry.path;
                 self.add_entry(encoder, dir_fd, &file_entry.name, &file_entry.stat)
-                    .await.context(format!("error at {:?}", self.path))?;
+                    .await
+                    .unique_context(format!("error at {:?}", self.path))?;
             }
             self.path = old_path;
             self.entry_counter = entry_counter;
-- 
2.43.0

[pbs-devel] [PATCH v2 proxmox-backup 4/4] pxar: use anyhow::Error in PxarBackupStream

[Gabriel Goller]

Instead of storing the error as a string in the PxarBackupStream, we
store it as an anyhow::Error. As we can't clone an anyhow::Error, we take
it out from the mutex and return it. This won't change anything as
the consumation of the stream will stop if it gets a Some(Err(..)).

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 pbs-client/src/pxar_backup_stream.rs | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/pbs-client/src/pxar_backup_stream.rs b/pbs-client/src/pxar_backup_stream.rs
index 22a6ffdc..0fe83342 100644
--- a/pbs-client/src/pxar_backup_stream.rs
+++ b/pbs-client/src/pxar_backup_stream.rs
@@ -5,7 +5,7 @@ use std::pin::Pin;
 use std::sync::{Arc, Mutex};
 use std::task::{Context, Poll};
 
-use anyhow::{format_err, Error};
+use anyhow::Error;
 use futures::future::{AbortHandle, Abortable};
 use futures::stream::Stream;
 use nix::dir::Dir;
@@ -25,7 +25,7 @@ use pbs_datastore::catalog::CatalogWriter;
 pub struct PxarBackupStream {
     rx: Option<std::sync::mpsc::Receiver<Result<Vec<u8>, Error>>>,
     handle: Option<AbortHandle>,
-    error: Arc<Mutex<Option<String>>>,
+    error: Arc<Mutex<Option<Error>>>,
 }
 
 impl Drop for PxarBackupStream {
@@ -68,7 +68,7 @@ impl PxarBackupStream {
             .await
             {
                 let mut error = error2.lock().unwrap();
-                *error = Some(err.to_string());
+                *error = Some(err);
             }
         };
 
@@ -100,18 +100,18 @@ impl Stream for PxarBackupStream {
     fn poll_next(self: Pin<&mut Self>, _cx: &mut Context) -> Poll<Option<Self::Item>> {
         {
             // limit lock scope
-            let error = self.error.lock().unwrap();
-            if let Some(ref msg) = *error {
-                return Poll::Ready(Some(Err(format_err!("{}", msg))));
+            let mut error = self.error.lock().unwrap();
+            if let Some(err) = error.take() {
+                return Poll::Ready(Some(Err(err)));
             }
         }
 
         match proxmox_async::runtime::block_in_place(|| self.rx.as_ref().unwrap().recv()) {
             Ok(data) => Poll::Ready(Some(data)),
             Err(_) => {
-                let error = self.error.lock().unwrap();
-                if let Some(ref msg) = *error {
-                    return Poll::Ready(Some(Err(format_err!("{}", msg))));
+                let mut error = self.error.lock().unwrap();
+                if let Some(err) = error.take() {
+                    return Poll::Ready(Some(Err(err)));
                 }
                 Poll::Ready(None) // channel closed, no error
             }
-- 
2.43.0

Re: [pbs-devel] [PATCH v2 proxmox{, -backup} 0/4] output full anyhow context in client

[Gabriel Goller]

bump.


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH v2 proxmox{, -backup} 0/4] output full anyhow context in client

[Gabriel Goller]

Submitted a v3!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel