Re: [PATCH datacenter-manager 09/15] task cache: include archive file path in error log messages

[Dominik Csapak <d.csapak@proxmox.com>]

one nit inline

On 7/2/26 11:23 AM, Lukas Wagner wrote:
> This should make any kind of troubleshooting much simpler.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   server/src/remote_tasks/task_cache.rs | 26 ++++++++++++++++++++++----
>   1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/server/src/remote_tasks/task_cache.rs b/server/src/remote_tasks/task_cache.rs
> index 02be9ca1..c7057786 100644
> --- a/server/src/remote_tasks/task_cache.rs
> +++ b/server/src/remote_tasks/task_cache.rs
> @@ -1065,8 +1065,10 @@ impl Iterator for TaskArchiveIterator<'_> {
>   struct InnerTaskArchiveIterator {
>       /// Archive files to read.
>       files: Vec<ArchiveFile>,
> -    /// Archive iterator we are currently using, if any
> +    /// Archive iterator we are currently using, if any.
>       current: Option<ArchiveIterator>,
> +    /// Archive file that is currently being iterated over, if any.
> +    current_file: Option<ArchiveFile>,
>   }
>   
>   impl InnerTaskArchiveIterator {
> @@ -1075,6 +1077,7 @@ impl InnerTaskArchiveIterator {
>           Self {
>               files,
>               current: None,
> +            current_file: None,
>           }
>       }
>   }
> @@ -1088,9 +1091,22 @@ impl Iterator for InnerTaskArchiveIterator {
>                   Some(current) => {
>                       let next = current.next();
>                       if next.is_some() {
> -                        return next;
> +                        return next.map(|res| {
> +                            res.with_context(|| {
> +                                format!(
> +                                    "failed to read from {file}",
> +                                    file = self
> +                                        .current_file
> +                                        .as_ref()
> +                                        .expect("self.current and self.current_file are both Some")

i guess we could prevent this expect here if `current` is changed to an
`Option<(ArchiveIterator, ArchiveFile)>`

so that both must be either Some or None?

> +                                        .path
> +                                        .display()
> +                                )
> +                            })
> +                        });
>                       } else {
>                           self.current = None;
> +                        self.current_file = None;
>                       }
>                   }
>                   None => 'inner: loop {
> @@ -1100,6 +1116,7 @@ impl Iterator for InnerTaskArchiveIterator {
>                       match next_file.iter() {
>                           Ok(Some(iter)) => {
>                               self.current = Some(iter);
> +                            self.current_file = Some(next_file);
>                               break 'inner;
>                           }
>                           Ok(None) => {
> @@ -1107,7 +1124,8 @@ impl Iterator for InnerTaskArchiveIterator {
>                           }
>                           Err(err) => {
>                               log::error!(
> -                                "could not create archive iterator while iteration over task archive files, skipping: {err:#}"
> +                                "could not create task archive iterator for {file}, skipping: {err:#}",
> +                                file = next_file.path.display()
>                               )
>                           }
>                       }
> @@ -1292,7 +1310,7 @@ impl JournalIterator {
>               .flat_map(|task| match task {
>                   Ok(task) => Some(task),
>                   Err(err) => {
> -                    log::error!("could not read task while iterating over archive file: {err:#}");
> +                    log::error!("could not read task while iterating over journal file: {err:#}");
>                       None
>                   }
>               })