Prevent nested error messages

code review
1

I have multiple error logs that looks like this message Failed to fetch dataset for data collection game: DbErr(RecordNotFound("Cannot find collected additional data")), continue...

This happens because I do multiple error handlings in different nested functions. How can I make these error messages more cleaner?

This is one example that I have:

pub async fn get_additional_collection(
    con: &DatabaseConnection,
    user_id: i32
) -> Result<additional_collection::Model, sea_orm::DbErr> {

    let additional_collection = AdditionalCollection::find()
        .filter(additional_collection::Column::UserId.eq(user_id))
        .one(con)
        .await?;

    if let Some(col) = additional_collection {
        Ok(col)
    } else {
        Err(sea_orm::DbErr::RecordNotFound("Cannot find collected additional data".to_string()))
    }
}

pub async fn get_class_datasets(
    con: &DatabaseConnection,
    class_id: i32
) -> Result<Vec<String>, error_handler::IsumisError> {

    let class_name = class_db::get_class_by_id(con, class_id).await?;
    let users = user_db::get_users(con, class_name.class_name).await?;

    let datasets= {

        let mut datasets = Vec::new();

        for user in users.into_iter() {
            if let Err(err) = create_dataset(con, user.id).await {
                warn!("Failed to fetch dataset for data collection game: {}, continue...", err);
            } else {
                let dataset = create_dataset(con, user.id).await?;

                datasets.push(dataset.to_string());
            }
        }

        datasets

    };

    Ok(datasets)
}
2

That error message looks pretty good to me. It tells you the high level operation that failed as well as the reason for the failure. Why do you think it is not clean?

3

It is expected that the fmt::Debug formatting of nested errors looks like this. If you don't want that, you should instead use fmt::Display to get a human-friendly message, and follow the Error::source() chain to print those too — this allows you to print a backtrace-like list of error causes instead of parenthetical nesting. (Error-handling libraries like anyhow and eyre often have tools to do this printing for you.)

That said, something strange is going on here, because your line

warn!("Failed to fetch dataset for data collection game: {}, continue...", err);

is asking for Display formatting. It seems like whatever the type of err is (I don't know because you haven't shown us fn create_dataset), it is mixing up Debug and Display in its own Display implementation.

1 Like
4

@jumpnbrownweasel because DbErr(RecordNotFound("...")) does not look very clean for me

@kpreid this mix appears because I am using my own error handler to use the ? for multiple error types.

#[derive(Debug)]
pub enum IsumisError {
    ErrorResponse,
    DbErr(sea_orm::DbErr),
    IOError(std::io::Error),
    ReqwestError(reqwest::Error),
    SerdeJsonError(serde_json::Error),
    WhoIsError(whois_rust::WhoIsError),
    HttpError(http_error::HttpError),
    RedisError(redis_error::RedisErr),
    JsonWebTokenError(jsonwebtoken::errors::Error),
    VarError(VarError)
}

impl From<sea_orm::DbErr> for IsumisError {
    fn from(e: sea_orm::DbErr) -> Self {

        let error: IsumisError = Self::DbErr(e);

        match error {
            IsumisError::DbErr(err) => Self::DbErr(err),
            _ => unreachable!("Unexpected error in SeaORM")
        }
    }
}

impl Display for IsumisError {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        write!(f, "{:?}", self)
    }
}

This allows me to handle the DbErr and other errors in one function.

5

If you don't want the Debug output, then you must specify a different Display format somehow. To make this as easy as possible, use anyhow or one of the other error crates.

6

To be precise, this line right here is what is producing the output you don't want:

:? is Debug format.

1 Like
7

Is there a list that shows the different formats?

8

The general list is here: std::fmt / Formatting Traits. But for errors, you have only two choices, Debug and Display, and you're already trying to implement Display so calling it on self would just be infinite recursion.

In the end, somewhere, you're going to need a match on the contents of the enum. But libraries such as thiserror can create one for you and properly format the error.

Note that in general, when error types wrap other error types, they should make a choice between being transparent or not. Transparent errors are just wrapping other error types and convey no information. Non-transparent errors convey their own information about what error happened, in addition to the wrapped error. In your case you should probably create a non-transparent error so that the user can receive more informative error messages — a std::io::Error by itself is very unhelpful without any information about what operation was attempted.

Transparent errors should:

  • Implement Display to format exactly the same as the wrapped error.
  • Implement Error::source() by calling Error::source() on the wrapped error.

Non-transparent errors should:

  • Implement Display to format only their additional information — for example, in the case of a wrapped IO error, the wrapper could print "failed to open file /some/path".
  • Implement Error::source() to return the wrapped error.

thiserror can help you implement either behavior.

In either case, when logging or showing a user an error, Error::source() should be used to obtain and print the full chain of errors. This is not the responsibility of the individual error type.

4 Likes
9

Great, thanks!