BurntSushi / jiff

A date-time library for Rust that encourages you to jump into the pit of success.
The Unlicense
1.73k stars 30 forks source link

about error types #8

Open BurntSushi opened 4 months ago

BurntSushi commented 4 months ago

When I initially started building this crate, my intent was to try and avoid the "one true god error type" pattern that I've seen a lot of folks complain about. The "one true god error type" pattern occurs when a crate defines approximately one main Error type, and then uses that error type for approximately every fallible operation in the crate. This is done even when, internally, only a subset of error cases are possible for any given operation.

One need not look far to find canonical examples of the "one true god error type" pattern. For example, std::io::Error is one such manifestation.

As hinted at, the main drawback of this pattern is that you get less information in the type system about what kinds of errors can occur for any given operation. I basically agree that this is a drawback, but I don't think it exists in a vacuum. Namely, there are benefits to omitting specificity:

I also think it's worth questioning just how useful it is to know the universe of possible error cases for any given fallible operation. Usually all you need is the ability to check whether a specific error case occurred, as opposed to needing knowledge about what's possible. For example, std::io::Error enables this via std::io::ErrorKind. I think I can do the same in Jiff, but might start without it to see how far we can get. With that said, I have tried very precise errors in the past, and my sense of things is that nobody really cares. Most of the time, for errors, you just want to print them and/or combine them with other errors, and maybe in some rare cases, inspect what "class" of error it is.

So when I initially started building this crate, I tried hard to assign specific error types to fallible operations. A lot of the lower level date arithmetic routines, for example, are either infallible or can only fail as a result of a RangeError. But as I started building higher level abstractions, the error types got more annoying, as it often felt entirely arbitrary whether a routine returned an Error (that is, any possible error type) or a more specific error type. It felt like I had implementation details leaking out into the API. And in particular, I had started converting RangeErrors into String values in order to combine them with ParseErrors because my parsing functions all returned ParseError and not a god error type.

On top of that, I often want to combine multiple error together.

So I think because of that, where I'm leaning is creating my own simplified anyhow::Error, but limited to the error types in Jiff. Internally it will create a chain, but will only expose a single error in order to make formatting of Box<dyn std::error::Error> work like one expects. And then I'll basically move all routines over to returning just a jiff::Error. And this simplifies a lot.

(Another reason why I didn't initially start with a chainable god error type was because I wanted Jiff to be usable in a no-{alloc,std} context, but no-alloc is an incredibly annoying constraint. I'm not sure that it's worth it. I think that if folks need no-alloc time handling, we should handle that on a case-by-case basis and possible build some new jiff-core crate.)

BurntSushi commented 3 months ago

I ended up going with the one god error type pattern. Once I did, I found that an enormous mental burden had been lifted, and composing high level APIs had a lot less friction. I was constant chaffing every time I composed APIs before. I find this to be an immense benefit, and it will probably take a lot of convincing for me to roll this back.

BurntSushi commented 3 months ago

Another benefit I found to have a god error type is that my error messages have substantially improved. A Jiff Error is basically like an anyhow::Error internally, with the ability to very easily attach a chainable "context" to any particular error. This in turn has led to me contextualizing errors instead of just bubbling up, say, a RangeError. This is especially important in very high level routines like rounding spans where a range error can occur in the depths of datetime arithmetic that doesn't have obvious meaning to the caller. By attaching context, we can at least add some extra details to the message very easily that was very annoying to do before. (Error messages still aren't great, but at least now we have a reasonable path to making them better.)

tedmielczarek-fastly commented 2 months ago

my sense of things is that nobody really cares. Most of the time, for errors, you just want to print them and/or combine them with other errors, and maybe in some rare cases, inspect what "class" of error it is.

I'm pretty solidly in agreement with you here. As long as there's a straightforward way to check for specific errors for special handling in certain cases, the overwhelming majority of the time people are just going to use ? to propagate errors upward, and at some point print an error message.

What I haven't seen a good solution for is ease of documenting specific errors that can be returned in these situations. For things like kernel and libc APIs it is often useful to know the full set of errors, and which specific errors map to specific failure modes. I haven't looked through the jiff APIs yet so I don't know if that is actually a problem that is relevant here.

BurntSushi commented 2 months ago

Yeah, I don't have a good sense of whether folks will even care about the class of error. If I had to guess, it might be useful to distinguish between "overflow" and "something in the input is not supported." In any case, I'm happy to add more introspection capabilities to jiff::Error, but I would definitely like to have some use cases first.

allan2 commented 2 months ago

A possible use case:

In rust-postgres #1164, I handled overflow here. With the current variants, I only encounter ErrorKind::Range so there's no need for further introspection.

If ErrorKind was exposed though, I could use it to distinguish the overflow case from future variants of ErrorKind, if more were to be introduced.

BurntSushi commented 2 months ago

@allan2 Can you say more? Why not return the jiff Error itself there? Or convert it to a string of you're worried about leaking the type?

musjj commented 4 days ago

Is there a reason why you don't just use anyhow::Error? It also comes with the ability to attach contexts.

BurntSushi commented 4 days ago

Because I didn't think it was worth the dependency in this context.