Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

about error types #8

Open
BurntSushi opened this issue Jun 4, 2024 · 8 comments
Open

about error types #8

BurntSushi opened this issue Jun 4, 2024 · 8 comments
Labels
probably not An idea that probably isn't going to happen. question Further information is requested

Comments

@BurntSushi
Copy link
Owner

BurntSushi commented Jun 4, 2024

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:

  • Less specificity in the error types means you have more semver flexibility in your API evolution. It can be difficult to predict forever-and-always the specific scope of errors allowed for a fallible operation, and then stick to that for all semver compatible releases. In a god error type, you have a lot more flexibility to add new error cases to existing fallible operations. One might argue that the new error cases could be construed as a breaking change in and of itself, but I don't think this is always the case. For example, maybe it is discovered in some rounding operation that datetime arithmetic is needed where it wasn't previously used. In addition to, say, a RoundInputError, the routine might now also want to return a RangeError. If you're using a god error type that subsumes all error types, it is not definitively a breaking change. But if your rounding function was previously returning RoundInputError because you tried to be really careful about your error types, it is definitively a breaking change to change the error type to something else. You're only way out is with a semver incompatible release, or somehow hack RoundInputError to be capable of containing a RangeError. (Which is probably possible because error types tend to be opaque.)
  • Even when not using one god error type, it is likely you'll need to sacrifice some specificity some of the time. For example, maybe there are three error types, A, B and C. And now you want to write a fallible operation that can only fail via A and B. And maybe another fallible op can only fail via A and C. And yet another fallible op can only fail via B and C. In other words, in the worst and more precise case, you need a distinct error type for every possible combination of error types. Since Rust lacks anonymous union types, this is a pretty difficult pill to swallow.
  • Having just a single error type simplifies a lot. It hides information from the type system, but by doing so, also hides information from consumers of the library API.

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 BurntSushi added the question Further information is requested label Jun 4, 2024
@BurntSushi
Copy link
Owner Author

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
Copy link
Owner Author

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.)

@BurntSushi BurntSushi added the probably not An idea that probably isn't going to happen. label Jul 7, 2024
@tedmielczarek-fastly
Copy link

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
Copy link
Owner Author

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
Copy link

allan2 commented Aug 15, 2024

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
Copy link
Owner Author

@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
Copy link

musjj commented Oct 13, 2024

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

@BurntSushi
Copy link
Owner Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
probably not An idea that probably isn't going to happen. question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants