Idiomatic way to return an optional value that might fail

I'm writing some code to load a record from the data store. Easy enough. This record might or might not exist - so the result needs to indicate that. Other than that though, the only way that it can fail is something outside of my control.

I'm conflicted about the best way to write this. The ways that seem to best fit would be either:

#[async_trait::async_trait]
pub trait UserRepository {
    async fn get_user_by_username(&self, username: &Username) -> anyhow::Result<Option<UserEntity>>;
}

In which I'm returning Ok(Some(user)) on success.

Or alternatively:

#[async_trait::async_trait]
pub trait UserRepository {
    async fn get_user_by_username(&self, username: &Username) -> Result<UserEntity, GetUserError>;
}

#[derive(Debug, PartialEq, thiserror::Error)]
pub enum GetUserError {
    #[error("The requested user was unknown")]
    UnknownUser,

    #[error(transparent)]
    Other(#[from] anyhow::Error),
}

This seems to make the happy case slightly easier, but it feels like the UnknownUser case - which is likely to be the majority of reasons this fails - is just redoing Option<> in a different way.

Ultimately this ends up producing an HTTP 200 for the success case, an HTTP 404 for the UnknownUser case and an HTTP 500 for the Other case, so I need to be able to distinguish between those cases but not in a hugely significant way.

Is there a recommended/idiomatic way to address this? And, if so, what is it? :slight_smile:

Cheers

Majority is not the same as all. Are there other potential failures?

To me, the greatest benefit of using Result<Option<_>> is if you ever need to use Option's functions.
For example, if your logic evolves to fetch a user, and in some case (for a demo version?) fill in a default user, you can use Option::unwrap_or and continue in the success case.

1 Like

I'll normally return a Result<Option<T>, Error> in this case

That way you can write something like this:

if let Some(person) = repository.get_user_by_username(...)? {
  ...
}
3 Likes

It's probably different at different levels.

At the data access layer, probably Result<Option<T>, E>, because in general handling the possibility of it not being found is something that should have type system encouragement.

But at the user login level, say, maybe just Result<T, E> if you need the user to be logged in, whether it failed for the user not existing or the password being wrong or whatever just isn't that important to the conceptual flow, and thus shunting it off to the easier-to-ignore part seems fine.

2 Likes

Majority is not the same as all. Are there other potential failures?

Possible outcomes for this are:

  • Successfully loaded the user
  • The user didn't exist
  • The remote service couldn't be reached - there's a number of things in this, e.g. it was down, my network was down, credentials were wrong, etc
  • The user data was malformed somehow - though if so then the remote system is returning data that doesn't match their schema.

That's about it. I'm planning to lump the last two of those in the "Something outside of my control" category since the only real difference between them would be in the log statements I produce.

Agree with :point_up: and I'm going to add one more reason. Result<Option<T>, E> conveys what to expect from the call. On success I can expect either zero or one Ts. If it's not possible to get data I can expect an E.

I'm also going to add that I've been well served by an over-abundance of custom error types like GetUserError. Being able to carry precise details about failures has made troubleshooting and debugging much easier. At my previous job we generally "flattened out" errors to simple strings very early. A few times I had to perform a substring search to determine what went wrong. In the best case that's just icky and inefficient. In the worst case that's very fragile.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.