Verifier: Refactor errors in sgx module

[kartikjoshi21]

Fixes: #231

[mkulke]

with thiserror we do not have to recreate every error explicitly (it will also lose information if the original error is reduced to a string). The following code illustrates how it works.

use anyhow;
use thiserror;
use thiserror::Error;
use std::fmt;

#[derive(Debug, Clone)]
struct OtherError;

impl std::error::Error for OtherError {}

impl fmt::Display for OtherError {
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        write!(formatter, "some other thing failed")
    }
}

fn with_other_error() -> Result<(), OtherError> {
    Ok(())
}

#[derive(Error, Debug)]
pub enum MyError {
    #[error("some thing")]
    Something,
    #[error("some other thing")]
    Other(#[from] OtherError)
}

fn with_thiserror() -> Result<(), MyError> {
    with_other_error()?;
    Err(MyError::Something)
}

fn top_level_with_anyhow() -> anyhow::Result<()> {
    with_thiserror()?;
    Ok(())
}

top_level_with_anyhow() returns an anyhow Result, it calls with_thiserror() which returns MyError, a thiserror enum as a Result. MyError has both its own type as well as a wrapper for OtherError which is a custom defined type, which is returned by with_other_error().

We don't need to recreate any errors. MyError converts into anyhow implicitly and for OtherError we create a simple #from wrapper in the MyError struct.

[kartikjoshi21]

with thiserror we do not have to recreate every error explicitly (it will also lose information if the original error is reduced to a string). The following code illustrates how it works.

use anyhow;
use thiserror;
use thiserror::Error;
use std::fmt;

#[derive(Debug, Clone)]
struct OtherError;

impl std::error::Error for OtherError {}

impl fmt::Display for OtherError {
    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
        write!(formatter, "some other thing failed")
    }
}

fn with_other_error() -> Result<(), OtherError> {
    Ok(())
}

#[derive(Error, Debug)]
pub enum MyError {
    #[error("some thing")]
    Something,
    #[error("some other thing")]
    Other(#[from] OtherError)
}

fn with_thiserror() -> Result<(), MyError> {
    with_other_error()?;
    Err(MyError::Something)
}

fn top_level_with_anyhow() -> anyhow::Result<()> {
    with_thiserror()?;
    Ok(())
}

top_level_with_anyhow() returns an anyhow Result, it calls with_thiserror() which returns MyError, a thiserror enum as a Result. MyError has both its own type as well as a wrapper for OtherError which is a custom defined type, which is returned by with_other_error().

We don't need to recreate any errors. MyError converts into anyhow implicitly and for OtherError we create a simple ´#fromwrapper in theMyError` struct.

@mkulke Yess i will make this change. we will then dont have to map things and we can preserve the original error string. Thanks

[mkulke]

@kartikjoshi21

note, in some cases we want to provide context to the source error because it isn't specific enough. that's how .context("...") is being used here with anyhow.

Usually the source error should provide the context we need but sometimes it's not enough and we need to provide context from the caller site:

fn with_other_error() -> Result<(), OtherError> {
    Err(OtherError)
}

#[derive(Error, Debug)]
pub enum MyError {
    #[error("some thing")]
    Something,
    #[error("some other thing")]
    Other(#[from] OtherError),
    #[error("some other thing with context")]
    FailedDoingSomethingSpecific(#[source] OtherError),
}

fn with_thiserror() -> Result<(), MyError> {
    // we're doing something specific, that we want provide context about which
    // the source error doesn't have.
    with_other_error().map_err(|e| MyError::FailedDoingSomethingSpecific(e))?;
    // or shorter even, since the constructor to `MyError::FailedDoingSomethingSpecific`
    // can act as a closure
    with_other_error().map_err(MyError::FailedDoingSomethingSpecific)?;
    // we can just bubble up the source error, we don't need context
    with_other_error()?;
    Err(MyError::Something)
}

[mythi]

@kartikjoshi21 are you still working on this? One higher-level comment is that many of the errors introduced here are generic to DCAP (covers both TDX and SGX). We have also discussed getting those two verifier modules to reuse code (see #404 (comment)) so perhaps we need a set to DCAP errors instead?