Result::inspect_err, could be worth?

Something like this has been already asked some time ago, but almost no discussion came out.

The basic idea is handling a family of error for which you would be happy to panic in development phases, but they are pretty unexpected in production and panicing would make things worse. The kind of situations in which you want to log the error and bubble it up.

To date, if you use chaining, you need to do something like this:

do_this()
    .and_then(/* .. */)
    .map(/* return a Result here */)
    .tranpose()
    .map_err(|err| {
        log!("Did not expect this! Err: {}. Data: {}", err, data);
        err
    })?
   .map(/* and so on */);

My point about using map_err instead of an hypothetical inspect_err is not about having to return err, but the fact that with map_err I am expressing that I am going to change the error type of this Result, which is not true.

However, I have seen people complaining about the complexity of Result (and Option), and even if I don't agree1 I can understand that adding another method will increase complexity even more. This can also be considered a niche case, adding a mental overhead for something not so useful (even if expressive).

Before opening a PR I would like to understand what you people think, if you ever needed this abstraction or not, and if you would consider adding the method something useful.

1 If you remove one of the many methods, you would need a match statement to express the same thing, making chaining impossible.


Practical real-life example: I build a Stream abstraction to get paged data from an Elasticsearch database. All the information is JSON encoded, therefore my approach is to analyze if more requests needs to be performed, eventually prepare the next one and trying to return the deserialized data. If a deserialization error occurs, I can log the original data and the error and bubble up the latter in order to make me understand the problem without causing a downtime. I took this specific example because it's easy to oversight a field that can be null in the DB but you did not encounter the case in months.

5 Likes

Just because you don’t specify its signature here and the original post also only describes it in words. This is about a function like this, right?

impl<T, E> Result<T, E> {
    pub fn inspect_err<F>(self, f: F) -> Self
    where
        F: FnOnce(&E),
    {
        self.map_err(|e| {f(&e); e})
    }
}
2 Likes

Yes, exactly! Sorry, I was focused on the idea and I totally forgot to write about the signature. You obviously got it right :wink:

I’m resurrecting this thread since I tried to use my_option.inspect() and realized that it doesn’t exist. Are inspect for Option/Result and inspect_err for Result things that we would like to add?

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.