Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display path where error "Could not create archive" occurred #131

Open
GabrielSimonetto opened this issue Oct 30, 2021 · 2 comments
Open

Comments

@GabrielSimonetto
Copy link
Collaborator

GabrielSimonetto commented Oct 30, 2021

While working on #94 we wound up wanting to build better errors for some cases, one of them is this FinalError:

builder.append_file(path, file.file_mut()).map_err(|err| {
    FinalError::with_title("Could not create archive")
        .detail("Unexpected error while trying to read file")
        .detail(format!("Error: {}.", err))
        .into_owned()
})?;

It isn't a great error message since it doesn't tell you which file broke, in a scenario where you are compressing/decompressing multiple files this could be an issue.

Now, it is troublesome to insert the output_file, since it was inserted on the tar writer from it's generic call. This is a problem because:

  • It seems that the Writer object does not have a getter to the output_path where it intends to write stuff, so we can't get this information from it.
  • So we would end up trying to pass output_path on the function? Maybe, but since it's a generic call we would need to reproduce it to the zip call aswell I think.

So... I can't see a clear path of action, and this is already a hard error to occur aswell, therefore I will put the discussion tag and the low-priority one

@GabrielSimonetto GabrielSimonetto added low priority pending decision We haven't concluded yet what to do labels Oct 30, 2021
@marcospb19
Copy link
Member

Just stopped to think about it, and we don't give this amount of context info when compressing with zip too.

And there are multiple points in that source code where errors can appear, so I guess we need to put custom error messages outside of both calls.

Maybe one message will do it for both, Failed to create the archive {}, fits for zip and tar.

What do you think?

@GabrielSimonetto
Copy link
Collaborator Author

That sounds ideal, you're saying that if the error breaks inside tar or zip modules, it can propagate the error upwards and then the calling method can finish annotating context? I like that

@marcospb19 marcospb19 added medium priority and removed pending decision We haven't concluded yet what to do low priority labels Nov 3, 2021
@marcospb19 marcospb19 changed the title Better error for could not create archive - append the failing output path Display path where error "Could not create archive" occurred Nov 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants