Replacing .unwrap() with Results

I'm trying to learn how to use Result to make a program more robust in the face of errors.

I have the following to get a file from a .zip file and return it in a buffer:

fn get_file_from_zip(zip_fn : &PathBuf, file_fn: &PathBuf) -> Vec<u8> {
    let zipfile = File::open(zip_fn).expect("Failed to open .zip");
    let reader = BufReader::new(zipfile);
    let filename = file_fn.clone().into_os_string().into_string().unwrap();
    let filename = filename.as_str().clone();
    let mut archive = ZipArchive::new(reader).expect("Failed to create ZipArchive from reader");
    let mut file = archive.by_name(filename).expect("Failed to get file from archive");
    let mut buffer = Vec::new();
    std::io::copy(&mut file,&mut buffer).unwrap();
    buffer
}

This works fine when the .zip file exists, is valid, can be opened, and the named file exists within the archive. I now want to know how to turn this into something more robust that returns a Result. I would like information on the error to be available to the caller.

  1. What type should I return?
  2. If Result<T,E> then what type should E be? (T clearly must be Vec)
  3. If there are multiple places where a std::io:Error can occur, how to I indicate to the caller what has failed?

Any other comments welcome.

For making choosing an error type simpler, or ways of adding additional information to errors, especially for building applications (as opposed to libraries) many people in Rust, as far as I’m aware, like to use crates like anyhow or eyre.

1 Like

Part of the exercise for me is creating a working example of using the Result type. So the main thing I don't understand at present is if I have multiple sources of Err's with a different Error type.

Something that does what I am aiming to do, but is a) ugly; and b) probably very unidiomatic; is

enum MyError {
    FailedToOpenZipFile(std::io::Error),
    FailedToConvertFilenameToString(OsString),
    FailedToCreateZipArchiveFromReader(ZipError),
    FileNotFoundInZip(ZipError),
    FailedToReadCompressedFileIntoBuffer(std::io::Error),
}

fn get_file_from_zip(zip_fn : &PathBuf, file_fn: &PathBuf) -> Result<Vec<u8>,MyError> {
    let zipfile = File::open(zip_fn);
    if let Err(err) = zipfile { return Err(MyError::FailedToOpenZipFile(err)); }
    let zipfile = zipfile.unwrap();

    let reader = BufReader::new(zipfile);

    let filename = file_fn.clone().into_os_string().into_string();
    if let Err(bad_string) = filename { return Err(MyError::FailedToConvertFilenameToString(bad_string)); }
    let filename = filename.unwrap();
    let filename = filename.as_str().clone();

    let mut archive = ZipArchive::new(reader);
    if let Err(ziperr) = archive { return Err(MyError::FailedToCreateZipArchiveFromReader(ziperr)); }
    let mut archive = archive.unwrap();

    let mut file = archive.by_name(filename);
    if let Err(ziperr) = file { return Err(MyError::FileNotFoundInZip(ziperr)); }
    let mut file = file.unwrap();

    let mut buffer = Vec::new();

    if let Err(err) = std::io::copy(&mut file,&mut buffer) { return Err(MyError::FailedToReadCompressedFileIntoBuffer(err)); }

    Ok(buffer)
}

// then
#[get("/zips/<path..>")]
fn get_img(...) {
    ...
    let buffer = get_file_from_zip(&zippath, &filepath_in_zip);
    match buffer {
        Ok(data) => (ContentType::JPEG,data),
        Err(err) => panic!("No data"),
    }
}

Then a Rocket specific thing is how to deal with the situation where if the file does exist, we get binary data of of type Vec<u8> and content type image/jpeg; but if not, we return an error as JSON.

The idea here is that instead of Err(err) => panic!("No data") I can generate and serve an appropriate error message depending on how get_file_from_zip failed. The Rocket docs mention Responders, so that's a nice separate exercise in learning Rocket.

Again, any comments suggestions welcome. But this is in part an exercise in getting my head around things like Result and Option, so workarounds that avoid Result and Option won't help me learn.

We have the question mark!

Then you can turn

let mut archive = ZipArchive::new(reader);
if let Err(ziperr) = archive { return Err(MyError::FailedToCreateZipArchiveFromReader(ziperr)); }
let mut archive = archive.unwrap();

into

let mut archive = ZipArchive::new(reader)?;

if you use thiserror to generate From<SomeError> for YourError like

enum YourError {
    #[error("failed to open Zip file")]
    FailedToOpen(#[from] ZipError)
}
6 Likes

Thanks. I'll have a look at thiserror later.

The problem I had with the question mark is that some functions returned a Result<T,std::io::Error>, whereas others returned a Result<T,ZipErrpr> and others returned Options. So the issue that had me stumped is that of finding a single return type that is compatible with all of these. (So using the question mark wherever I was using an unwrap led to compile errors. I get that the function needs to return a Result<T,E>, and I'm clear on what type the T is (Vec<u8>), but had no clue what the E should be. I'll also have a play with cargo expand so as to see what thiserror does.)

Ok. So writing the above led me to try and come up with the simplest example of what I was struggling with (albeit a completely artificial example, whereas the one I originally gave had a practical result I was trying to achieve). The result was:

fn one() -> Result<i32,i32> {
    Err(1)
}
fn two() -> Result<f32,f32> {
    Err(2.0)
}
fn combine() -> Result<f32,i32> {
    let x = one()?;
    let y = two()?;
    if x != 0 {
        Ok(0.0)
    } else {
        Ok(1.0)
    }
}
fn main() {
    let a = combine();
    match a {
        Err(x) => println!("Error {}",x),
        Ok(x) => println!("Ok {}",x),
    }
}

This gave the following error

7 | fn combine() -> Result<f32,i32> {
  |                 --------------- expected `i32` because of this
8 |     let x = one()?;
9 |     let y = two()?;
  |                  ^ the trait `From<f32>` is not implemented for `i32`

And, after having read about #[from] in thiserror (thanks for the above), I came up with the following

use std::convert::From;
use std::fmt; 

// See https://doc.rust-lang.org/std/convert/trait.From.html
// this is the Err type that combine() will return
#[derive(Debug)]
enum MyError {
    FError(f32),
    IError(i32),
}
// this is so that we can convert from a Err(i32) or Err(f32) to an Err(MyError)
impl From<i32> for MyError {
    fn from(value: i32) -> Self {
        MyError::IError(value)
    }
}
impl From<f32> for MyError {
    fn from(value: f32) -> Self {
        MyError::FError(value)
    }
}
// This is so we can println!() our error
impl fmt::Display for MyError {
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
        match self {
            MyError::FError(value) => write!(f, "FError: {}", value),
            MyError::IError(value) => write!(f, "IError: {}", value)
        }
    }
}

// totally artificial example where two functions return Results but with different Err types
fn one() -> Result<i32,i32> {
    Err(1)
}
fn two() -> Result<f32,f32> {
    Err(2.0)
}
fn combine() -> Result<f32,MyError> {
    let x = one()?;
    let y = two()?;
    if x != 0 {
        Ok(0.0)
    } else if y != 0.0 {
        Ok(2.0)
    } else {
        Ok(1.0)
    }
}

fn main() {
    let a = combine();
    match a {
        Err(x) => println!("Error {}",x),
        Ok(x) => println!("Ok {}",x),
    }
}

I go with the "enum that wraps different error types" approach that you've shown. The principle I go with is, give the caller as much and as little information as is useful, so that they can make good decisions / provide good feedback.

i.e. how would you know if one std::io::Error is different to the next? Having a String paired as contextual information helps you decide how to recover from a failure at runtime, but having an enum variant per failure lets you decide what to do in code at compile time.

e.g.

  • For an automation tool, an error type may contain the source errors all the way down, so that the automation tool developer can figure out what caused the automation to fail.

  • For a web application, an inner library may contain all the source errors all the way down, but when it reaches the business layer, the code there may:

    1. Split the error and record detailed information (e.g. could not reach third party service at ) somewhere for someone to fix later.
    2. Return a subset of information to the user who sent the request, e.g. "sorry we couldn't process your order, here's an error code: ABC-1234" for you to mention when you contact support.

See: error.rs -- this allows one to provide rust compiler style errors specific to each variant, because it's not just "a Box<dyn std::error::Error>" (which I do use in tests, since I don't need much more feedback than "this test broke, here is the error" to figure out what to do).

1 Like

Here's how I might write the same code:

use {
    std::{
        fs::File,
        io::{BufReader, Read},
        path::Path,
    },
    zip::ZipArchive,
};

pub fn get_file_from_zip<P: AsRef<Path>>(zip_name: P, file_name: &str) -> anyhow::Result<Vec<u8>> {
    let zip_file = BufReader::new(File::open(zip_name)?);
    let mut buffer = vec![];
    ZipArchive::new(zip_file)?
        .by_name(file_name)?
        .read_to_end(&mut buffer)?;
    Ok(buffer)
}

Or, with more error information:

use {
    anyhow::Context,
    std::{
        fs::File,
        io::{BufReader, Read},
        path::Path,
    },
    zip::ZipArchive,
};

pub fn get_file_from_zip<P: AsRef<Path>>(zip_name: P, file_name: &str) -> anyhow::Result<Vec<u8>> {
    let zip_file = BufReader::new(File::open(zip_name).context("cannot open ZIP file")?);
    let mut buffer = vec![];
    ZipArchive::new(zip_file)
        .context("cannot parse ZIP file")?
        .by_name(file_name)
        .context("cannot find file in ZIP file")?
        .read_to_end(&mut buffer)
        .context("cannot read file")?;
    Ok(buffer)
}
3 Likes

FWIW, I would use derive_more - Rust to reduce some boilerplate in your sample code:

use derive_more::{From, Display};

#[derive(Debug, Display, From)]
enum MyError {
    FError(f32),
    IError(i32),
}

fn one() -> Result<i32,i32> {
    Err(1)
}
fn two() -> Result<f32,f32> {
    Err(2.0)
}

fn combine() -> Result<f32,MyError> {
    let x = one()?;
    let y = two()?;
    if x != 0 {
        Ok(0.0)
    } else if y != 0.0 {
        Ok(2.0)
    } else {
        Ok(1.0)
    }
}

fn main() {
    let a = combine();
    match a {
        Err(x) => println!("Error {}",x),
        Ok(x) => println!("Ok {}",x),
    }
}

If f32 and i32 implemented Error you could also use derive_more::Error, but as is this will work.

Anyway, I concur with @azriel91 that your "enum that wraps error types" is a good approach. In your case, derive_more::From may give you trouble since you have two enum variants that wrap std::io::Error and two that wrap ZipError, so you may have to tell the derive macro to ignore some variants and then create them manually.