Why I use `anyhow::Error` even in libraries

Or, "Why we should have std::fs::File::open_optional".

This blog roughly covers the same problem domain as a blog from a sled database developer but has a somewhat different conclusion.

Error handling in Rust is a big topic, with a variety of opinions. This is even more true of the programming language design space in general.

The general 2021 Rust status quo

The Rust Book example on error handling has an example of a correct way to handle a common error path.

To save you the click, here's the code:

    let f = File::open("hello.txt");

    let f = match f {
        Ok(file) => file,
        Err(error) => match error.kind() {
            ErrorKind::NotFound => match File::create("hello.txt") {
                Ok(fc) => fc,
                Err(e) => panic!("Problem creating the file: {:?}", e),
            },
            other_error => {
                panic!("Problem opening the file: {:?}", other_error)
            }
        },
    }

There are popular crates like thiserror and anyhow.

A number of crates and APIs don't strongly differentiate likely-fatal versus likely-not-fatal errors

The above "file not found" case is in my experience by far the best example. It's true in most programming languages; e.g. in Python open() will throw an exception you need to catch, Go has os.IsNotExist() etc.

I would say that in my experience, 99% of the time I am matching on a filesystem error it is to handle "file not found".

And particularly in Rust, we have a nice Option<T> type that matches the domain here perfectly. So I'm arguing that the standard library should have this:

diff --git a/library/std/src/fs.rs b/library/std/src/fs.rs
index 014d5119b05..f9b3e69265e 100644
--- a/library/std/src/fs.rs
+++ b/library/std/src/fs.rs
@@ -328,6 +328,33 @@ pub fn open<P: AsRef<Path>>(path: P) -> io::Result<File> {
         OpenOptions::new().read(true).open(path.as_ref())
     }
 
+    /// Attempts to open a file that may not exist in read-only mode.
+    ///
+    /// See the [`OpenOptions::open`] method for more details.
+    ///
+    /// # Errors
+    ///
+    /// This function will return `Ok(None)` if the file does not exist.
+    /// Other errors may also be returned according to [`OpenOptions::open`].
+    ///
+    /// # Examples
+    ///
+    /// ```no_run
+    /// use std::fs::File;
+    ///
+    /// fn main() -> std::io::Result<()> {
+    ///     let mut f = File::open_optional("foo.txt")?;
+    ///     Ok(())
+    /// }
+    /// ```
+    pub fn open_optional<P: AsRef<Path>>(path: P) -> io::Result<Option<File>> {
+        match OpenOptions::new().read(true).open(path.as_ref()) {
+            Ok(f) => Ok(Some(f)),
+            Err(e) if error.kinds() == std::io::ErrorKind::NotFound => Ok(None),
+            Err(e) => Err(e),
+        }
+    }
+
     /// Opens a file in write-only mode.
     ///
     /// This function will create a file if it does not exist,

While this is obviously just a minor convenience, the standard library is also the "baseline" around which the larger Rust ecosystem evolves. It's also about reflecting best practices there.

In other words, this blog post isn't just about files. The core argument here is:

If any regular user of your API is likely to match on an error type, consider
whether instead the pattern of using e.g. anyhow::Result<SomeOtherType> would
work better.

The sled developer above argues for nesting Result<T>, which to me is ugly
and also it's still easy to "double unwrap" accidentally. Specifically, I would
argue that a better API for that case in sled is:

fn compare_and_swap(
  &mut self,
  key: Key,
  old_value: Value,
  new_value: Value
) -> Result<Option<CompareAndSwapError>, sled::Error>

It is a bit ugly in that the "happy path" is None, but again I think that's better than also unintentially supporting tree.compare_and_swap(...)??. The point here is, the callers of this API should generally be inspecting the inner value and not using ? on it.

But I'm writing a foundational crate!

Right, for something like e.g. hyper, the custom hyper::Error type makes complete sense.

I'm not arguing against crates like thiserror. I'm arguing that one shouldn't blindy think "this is a library, I should use thiserror for everything".

Even in "foundational" library crates, there are still often APIs for which callers aren't ever going to match on.

Conclusion

Let's normalize using anyhow::Error in library crates for "should be fatal" errors.

5 Likes

I don't understand why a library writer would presume to know anything at all about what a user is intending to do with their API. As a library writer you can't tell me what errors are (likely) fatal or not, because you don't know what my application is doing, and to what extents I am willing to go to handle an error. Maybe I panic on all errors. Maybe I have have 26 different fallback routes for every possible configuration. Maybe I blindly ignore every error you give me. You don't know, so what business do you have trying to distinguish between non-fatal or fatal errors? You don't have enough context to know what 'fatal' means as a library developer. You don't know what I'm likely to match on or not.

This is absurd to me. What do you care how far I have to propagate an error to handle it? It's none of your business. Don't try to dictate your users' architecture.

12 Likes

Hmm, you have a fairly confrontational tone here. I was trying to start a friendly discussion in which it's very much expected there are a variety of opinions. Let's stay civil!

This however I would disagree with as a basis. The author of a library should have a fair understanding of the problem domain and what types of errors are sensible to "match on" versus which should be fatal.

Now, that said: A library author can still offer a "lower level" API that e.g. tries to propagate the underlying error, if they think there are reasonable cases for this.

Or we can flip this discussion around; a library author could define a custom error type for most of their API, but still use anyhow::Result<T> for a few APIs that they are fairly confident there's no valid reason to match on the error.

Or even more weakly: I'm really justifying my use of anyhow::Result<T> in a library, and I think it applies to other cases. But if you don't agree with your cases, that seems fine. I definitely didn't expect universal consensus :wink:

(But out of curiosity, do you agree or disagree there should be File::open_optional()?)

2 Likes

:100:% this. The proposal/PoV here tries to prescribe downstream code's actions in every possible, unforeseen future. That hinders composability – this kind of design would be inferior to just letting the caller decide what they want to do with the error.

6 Likes

No, you're not getting a "confrontational tone," you're experiencing a slightly different culture. I've said nothing uncivil.

I don't see the point in it when you can just match on the error that File::open gives you. Why try to open a file if you don't want it opened? What does Ok(None) even mean? That the file was successfully not opened?

FWIW, I don't care if you want to use anyhow::Error in libraries. That's your decision and I obviously don't know enough about your problem to tell you not to. But don't think that you're doing your users a favor by somehow trying to force them into matching them instead of propagating them. Your users will probably want to do both, and you don't know when, where, how, or why they're doing it.

3 Likes

No, that's absolutely not what I said.

What I said in a nutshell is more: "I think it's okay to use anyhow::Result<T> (or Box<dyn Error> etc.) in some libraries for some APIs.".

I guess I'm missing some context, but what does the proposed API extension for std::fs have to do with anyhow? These appear to be very disparate, perhaps only incidentally related. If you care to elaborate on that point, it would be helpful.

Things will start to get more subjective from here. IMHO, anyhow is great if you literally do not care what the error is, and just want to throw it over the wall. And by all means, you should do that, if it's your thing.

But if I use your library and I want to actually handle errors instead of blindly throwing them over the wall, I'm going to end up downcasting them to concrete types. Perhaps often. In an ironic twist of fate, downcasting returns a ... wait for it ... Result! So now I have to handle errors for handling errors. The other option is converting the error to a string and trying to match with RegExp or something equally dubious.

It's also going to be a chore to find out exactly which error types can show up in an API that returns anyhow::Result. Do I have to comb through the library's source code to find out if I/O errors are possible so I can attempt to downcast them?

In short, I agree in principle that's ok to use type-erasing error handling in libraries, just as long as real concrete error types are available in the public-facing API. Have the courtesy to tell me how the call failed. Please? "Stringly typed" error handling would be a great regression.


The other thing is that Result<Option<T>> probably doesn't express the intention of open_optional very well. This construct is a tristate, but it requires double-unwrapping the Ok(Some) state. This one is also a tristate, but only needs a single unwrap to get at the data:

enum OptionalOpen {
    Opened(File),
    NotFound,
    Error(io::Error),
}

impl OptionalOpen {
    pub fn unwrap(self) -> File {
        match self {
            Self::Opened(file) => file,
            Self::NotFound => panic!("Not Found"),
            Self::Error(err) => panic!("{}", err),
        }
    }
}

You can imagine a number of other impls on this enum for various ways to unwrap, map the file and error invariants, etc.

That said, I'm not proposing anything other than pointing out how an API can effectively communicate its intentions to the caller.

6 Likes

The tone is decidedly uncivil and unprofessional. Address the issue, don’t attack the individual. There is no reason to use “you” statements when addressing an esoteric concept. This “culture” would not be tolerated in kindergarten. It discourages discussion and stifles healthy disagreements that may lead to new ideas.

6 Likes

Moderator note:

If someone calls you out, try not to be defensive. This is explicitly mentioned in the Code of Conduct:

And if someone takes issue with something you said or did, resist the urge to be defensive. Just stop doing what it was they complained about and apologize. Even if you feel you were misinterpreted or unfairly accused, chances are good there was something you could’ve communicated better — remember that it’s your responsibility to make your fellow Rustaceans comfortable. Everyone wants to get along and we are all here first and foremost because we want to talk about cool technology. You will find that people will be eager to assume good intent and forgive as long as you earn their trust.

Also, publicly calling people out, like I’m doing right now and like has been happening in this thread, should be treated as the last resort, not the first response you have to bad behavior. It’s likely to prompt whoever you call out to defend their behavior, even though the CoC explicitly says not to do that. It’s especially hard on this Users forum, which is often people’s first exposure to the Rust community, so they aren’t always familiar with the rules. Using a private message is a better way to handle a problem like this instead of doing it in public in the thread. It seems to make people less defensive, and in any case, it prevents four different people from piling on (if you click the Flag button, the first option listed will be “Send a Message”; this sends a PM to the user, and doesn’t actually flag them).

If someone just wants to read about “anyhow” and Error handling, they probably don’t want to read a discussion of whether telling someone to “mind their own business” is rude or not. Even if the discussion is otherwise civil, it’s still off-topic.

17 Likes

I think the double nesting was extremely intentional, in line with the nested Ok(Result... in the sled blogpost.The outer error is less-specific to the operation, for example a disk/permissions failure.
The inner error is specific to the particular (_optional) flavor of the operation. Using a unit struct error type here (Result<Result<File, NotFound>, io::Error>) doesn't seem like an improvement here. It both enables blind ?? (if you consider this a fatal error, just use the non-_optional flavor) while not conveying any extra context (it's a unit struct after all).

What does Ok(None) even mean? That the file was successfully not opened?

Yes. The presence of the file was successfully checked for, and it was not there. So the successful result of this operation is the lack of a File handle.

Why try to open a file if you don't want it opened?

In many domains, opening a file if it exists at a particular path, and doing something else if it does not, is a reasonable operation. In that situation, not finding the file is not an error or a failure, semantically. It's a different state of normal operation. The fact that this branching needs to go through the very low-level and opaque/kitchen-sink io::Error type, which requires the use of match guards to do in a single pattern, seems far from optimal. Goodbye .or_else & fam.

It's trivial to write this once in a utility function, and whether it does belong in std is dubious, but that it's a widely used pattern cannot be disputed.

Yet, regarding the "std is the baseline" argument, people might in fact be discouraged (or simply not think to) write their own fn optional_file(path: P) -> io::Result<Option<File>> because

how silly would that be? The io::Error already knows how to represent the "file was not found" condition! If I wrapped that, I would have two different representations of the same state possible! Shame on me for even considering it!

while including a function like this would reenforce that io::Error is an opaque, unexhaustive, potentially-dyn-carrying twin of anyhow::Error, merely with a connotation of being related to kernel io operations and some degree of programmatic categorization possible.

2 Likes
  1. Result<Option<T>> is often bigger than Result<T>, which may not be a worthwhile tradeoff. Especially if you're doing things like collecting errors into a Vec.

  2. When you open a file, you have to handle all of the cases. This would make it slightly more convenient to handle one case (the file doesn't exist), but it's not at all obvious that this is the most common type of error (denial through permissions are very common,) or that the code that handles missing files should be implemented in a different place from code that handles bad permissions. In general, you don't know where these errors should be handled, or even if they should be handled, because that's an application concern, not a library concern.

  3. It's not clear that most users would even want to use this function. Find some code that wants it, and justify it based on how difficult a problem it is to solve. Just because you can imagine some benefit doesn't mean it will be realized in actual code. There are real costs to implementing functions that almost nobody uses.

let file = File::open(path).map_err(|e| match e.kind() {
    ErrorKind::NotFound => { /* handle not found case */ }
    e _ => e
})?;

This code is already concise, easy to expand upon, and allows for any kind of formatting, handling, logging, etc.

Incidentally, if the relevant error kind is hard to inspect or isolate, that's a problem that should be fixed in the Error type to maximize composability -- providing that benefit automatically to all users of the type. It's not something you want to fix in every API that returns it in an ad-hoc and arbitrary fashion. Why should File::open special case 'file not found', and File::create not specialize on 'file exists'? Perhaps File::set_permissions should also return Result<Option<()>>? Which Results should not be Result<Option<_>>, and why?

2 Likes

Vec<File> and Vec<io::Error> are unchanged by this approach.

Collecting just the files and short circuiting on the first "real" error is also trivial and free as in beer:

let a: Vec<&str> = [Ok(Some("file")), Ok(None)]
    .into_iter()
    .filter_map(Result::transpose)
    .collect::<Result<_, _>>()?;
  1. That's why I am not advocating for the removal or deprecation of File::open.
  2. It doesn't matter how common an error is, just how special it is in terms of the expectations of the caller of the function that returns it. The function open_optional is very explicitly concerned with opening files like /etc/docker/dockerd.json, .gitignore, .editorconfig and such, where their absence is a well-specified happy state. Not being able to list the directory where it is expected, or not being able to access the positively existing file are not, they are failures. The pattern is common and recurrent, not universal.

map_err is insufficient here. You'd have to use or_else, map_or_else or an outer match directly. Note that in the first case you'd also have to directly handle the default inline, since you can't change the type of the happy value in order to chain. There's basically nothing you can write inline that is as good as your own utility

fn handle_optional_file(res: io::Result<File>) -> io::Result<Option<File>> {
    match res {
        Ok(f) => Ok(Some(f)),
        Err(e) if error.kinds() == std::io::ErrorKind::NotFound => Ok(None),
        Err(e) => Err(e),
    }
}

At which point it will always be run directly after opening a file, which makes open_optional that actually goes to the fs a superior api.

File::create clobbers the file if it exists, by default.
OpenOptions::create_new sets a flag on a mutable options struct, and isn't a finalizer method that goes to fs, so it cannot change the output type despite the programmer clearly expressing that they want a failure if the file exists.
Additionally, this failure, even when such a desire was expressed, might belong to either category of error, and is not a common pattern.
(I did assume that this paragraph is an ad absurdum mockery, but felt like sharing what I saw looking at create. I of course have nothing to say about Result<Option<()>>, for set_permission or otherwise.)

I ended up looking at OpenOptions in std::fs - Rust, and that was not very inspiring.

The mapping to io::ErrorKind s is not part of the compatibility contract of the function

this line in particular is quite motivating to including an open_optional that guarantees its behavior in that particular case in std.
(Also, the design where OpenOptions of read: false, write: false, append: false is (1) representable at all (2) the default that is created by new, just to return an InvalidInput-kind error at runtime if used unmodified, is dubious)

1 Like

It was not. I'm pointing out that design decisions should be based on generalizable principles, so that their value can be established in a more objective way. If there is some reason that set_permission (or any other function) should not be treated the same way, that is fair enough, but there isn't even a good principle motivating these changes.

You can't win a race if you're running around arbitrary imagined obstacles, and you can't design an above-average programming language if you're responding to imagined benefits for unwritten projects. What is the benefit here? What code is better? Show it.

Don't just ask me to assume it exists, because I've written enough file handling code to know how it looks, and it doesn't look like it needs this. In the vast majority of code, if the file can't be opened for any reason, I maybe try other files, (and if I do, I log which file was used) and if none succeed, ultimately kick an error message to the user to let them figure it out. What special handling needs to occur if the file doesn't exist? I usually don't care why a file is inaccessible. It most often means the user needs to do something to make it so.

This line should be making you ask why it's not part of the guarantee, and how you should design reliable programs in response to that lack of guarantee, not "how do we make a function which guarantees more." That guarantee might not be given because different OSs return different errors depending on why the file cannot be opened, and you will do no better with open_optional, because it would imply that sometimes the function returns Ok(None), while on other OS's it returns Err(_). This is something that cross-OS knowledge is needed to address (which may not exist for forward-compatibility reasons), not something to throw out as some vague benefit based on wishful thinking.

In fact, this makes it pretty clear that attempting to separate file errors into two categories is all the more a hazard, because for one reason or another, OpenOptions is not willing to guarantee which errors go into which categories, making the proposed function unreliable, or the implementation complex and error-prone.

And again, there's been no argument about why anybody should particularly care if a file doesn't exist as opposed to any other reason it might be inaccessible.

2 Likes

I gave examples of some common files where the file not existing at all means "default behavior", while the file existing but in an inaccessible state, or its existence undeterminable means "something went terribly wrong, we must be extremely cautious to not accidentally proceed with the default behavior and instead crash"

I read through that blog post probably two weeks ago, but I didn't get the sense at all that the author was satisfied to be using nested results or that it solved all of their problems. I understood it more as something they settled on, lacking a better alternative. And more importantly, as a warning to others about context sensitivity in API design. That says nothing about the "double nesting", other than it emerged consequentially.

My argument is that neither OP nor the sled author truly wanted "nested results" but would like an API that can communicate more practically that the functions may return three states. I called it a tristate, which is an EE term, but it just means there are three independent states (nothing fancy).

I wouldn't call Ok(None) ("it was successful, and there is no file handle") any more useful than Err(Some) ("There was an error, and it was fatal/non-optional") ... Given that you can define a tristate with various combinations of Result and Option:

  • Result<Option<T>, E> (OP's suggestion)
  • Result<T, Option<E>> (The Err(Some) thing I just mentioned)
  • Option<Result<T, E>> (Now we're getting wacky!)

Logically and pragmatically, one could just represent three independent states as three independent states. And not worry about which of the combinatorics better communicates the intention of the API.

As a matter of convention, I would agree. On the other hand, conventions can be notoriously fragile.

Perfect, we agree wholly on this point. We differ where I extend this same logic to include combinations of Result and Option.

I have to disagree with that. open_optional applies equally to any file on any available file system. Including, obviously, files that will never exist. That property makes it even more general than open.

Thanks for replying; I had been intending to abandon this thread as having just started off wrong, but I'm glad to see someone else arguing in favor! To be clear I didn't expect this to be uncontroversial, but I did hope for a reasoned debate.

To make this less abstract, I recently cargo culted another copy of open_optional over here.

And then, here's a recent nontrivial use of it: tar: Move repo config into `/sysroot/config` by default for backcompat by cgwalters · Pull Request #196 · ostreedev/ostree-rs-ext · GitHub

Basically, we're looking for a file in a new expected path first, but falling back to a legacy path (and also need to handle the case where neither exist as a normal, expected case).

Needing .transpose() for this is slightly awkward, but I otherwise find being able to use the standard Option::or_else here quite elegant. This code would IMO be much uglier with match on error types.

EDIT: And to elaborate further, I think it's good how ? continues to play the role of propagating actually unexpected errors, which is lost with error-matching.

I don't have an opinion on open_optional; it isn't an uncommon pattern, but it is also trivial to implement yourself.

As for distinguishing fatal vs non-fatal errors from libraries - no...just no. Trying to guess how your library will be used and prescribe that behavior to downstream users just leads to inflexibility. I've fought bloody battles with libraries that thought they knew better than me how my program should work, but they had no idea what I was trying to do. This is absolutely a mistake.

1 Like

Can you give specific examples?

I will say again I think I started this thread wrong. I think people keep thinking I'm saying we should replace all error types in all crates with Box<dyn Error> tomorrow. And really this is partly my fault; the initial post definitely came across too strong and not nuanced enough, and this is part of the cause of the equally strong pushback.

I'm talking about some APIs for some libraries. And specifically, I'm not mainly focusing on "foundational" libraries like serde or hyper, but "mid to high level" libraries.

For example, when writing Rust CLI apps, it can often make sense to internally structure most of it as a library. However, for your internal "load the configuration file" error, it's IMO not worth the overhead of using e.g. thiserror to make a custom error type which enumerates the different cases of: I/O error, serde_toml failing to parse, or an invalid configuration key in the file. In the end, you're just going to print the thing to stderr and exit. So even though you're writing what is a library, it still makes sense to use anyhow::Result or similar.

Now the other thing I am arguing which is strongly related to this but is logically distinct is that I'm saying even for "foundational" libraries, in some cases it can be better to use nested types instead of putting everything inside an error type if one or two of those error cases are much more likely to be match'd on than others - it can lead to much better ergonomics.

2 Likes

Most of the examples are not in Rust and I'd rather not directly criticize people's libraries. My experience has been that the more niche or spcialized the library the more likely this is to occur.

I didn't think you were saying that. I'm just voicing my opinion that any library author trying to rank the severity of various errors is almost certainly making a mistake that will limit somone's usage of that library. Far better to provide the infomation to downstream and let them decide. Better ergonomics are always welcome, but should not come at the expense of flexibility.

1 Like

Aren't "should be fatal" expected to be panics?