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

ZipStorage panics when failing to open/read zip file #3430

Closed
ctb opened this issue Dec 14, 2024 · 1 comment · Fixed by #3431
Closed

ZipStorage panics when failing to open/read zip file #3430

ctb opened this issue Dec 14, 2024 · 1 comment · Fixed by #3431

Comments

@ctb
Copy link
Contributor

ctb commented Dec 14, 2024

If you run ZipStorage::fromfile,

pub fn from_file<P: AsRef<Path>>(location: P) -> Result<Self> {

this code

            archive_builder: |mapping: &Option<memmap2::Mmap>| {
                piz::ZipArchive::new(mapping.as_ref().unwrap()).unwrap()
            },

will panic, presumably because of the second unwrap() which fails to handle ZipArchive errors.

When I try to switch the second unwrap to a ?, I get:

error[E0277]: the `?` operator can only be used in a closure that returns `Result` or `Option` (or another type that implements `FromResidual`)
   --> src/core/src/storage/mod.rs:409:64
    |
408 |             archive_builder: |mapping: &Option<memmap2::Mmap>| {
    |                              --------------------------------- this function should return `Result` or `Option` to accept `?`
409 |                 piz::ZipArchive::new(mapping.as_ref().unwrap())?
    |                                                                ^ cannot use the `?` operator in a closure that returns `ZipArchive<'_>`
    |
    = help: the trait `FromResidual<std::result::Result<Infallible, ZipError>>` is not implemented for `ZipArchive<'_>`

For more information about this error, try `rustc --explain E0277`.
error: could not compile `sourmash` (lib) due to 1 previous error

which I vaguely understand ;).

I feel like the right thing to do is to move this outside the builder, but not sure.

@ctb
Copy link
Contributor Author

ctb commented Dec 14, 2024

Figured it out! You can use ZipStorageTryBuilder!! see #3431

@ctb ctb closed this as completed in #3431 Dec 15, 2024
ctb added a commit that referenced this issue Dec 15, 2024
This PR switches from `ZipStorageBuilder` to `ZipStorageTryBuilder` in
order to propagate errors from bad zip files.

Fixes #3430
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant