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

Completely overhaul error handling #358

Merged
merged 68 commits into from
Apr 25, 2016
Merged

Completely overhaul error handling #358

merged 68 commits into from
Apr 25, 2016

Conversation

brson
Copy link
Contributor

@brson brson commented Apr 25, 2016

This is a huge refactor to try to make error handling easier, in particular to encourage correct chaining of errors, and use of the correct error type. It makes it easy to just use strings as errors for simple cases. Introduces the rustup_error crate, the documentation for which follows.

I'd suggest not reading the individual commits because they are just small refactoring steps.


Based on quick_error! and Cargo's chain_error method.

This crate defines an opinionated strategy for error handling in Rust,
built on the following principles:

  • No error should ever be discarded. This library primarily
    makes it easy to "chain" errors with the chain_err method.
  • Introducing new errors is trivial. Simple errors can be introduced
    at the error site with just a string.
  • Handling errors is possible with pattern matching.
  • Conversions between error types are done in an automatic and
    consistent way - From conversion behavior is never specified
    explicitly.
  • Errors implement Send.
  • Errors carry backtraces.

Similar to other libraries like error-type and quick-error, this
library defines a macro, declare_errors! that declares the types
and implementation boilerplate necessary for fulfilling a
particular error-hadling strategy. Most importantly it defines
a custom error type (called Error by convention) and the From
conversions that let the try! macro and ? operator work.

This library differs in a few ways:

  • Instead of defining the custom Error type as an enum, it is a
    struct containing an ErrorKind (which defines the
    description and display methods for the error), an opaque,
    optional, boxed std::error::Error + Send + 'static object
    (which defines the cause, and establishes the links in the
    error chain), and a Backtrace.
  • The macro additionally defines a trait, by convention called
    ChainErr, that defines a chain_err method. This method
    on all std::error::Error + Send + 'static types extends
    the error chain by boxing the current error into an opaque
    object and putting it inside a new concrete error.
  • It provides automatic From conversions between other error types
    defined by the declare_errors! that preserve type information.
  • It provides automatic From conversions between any other error
    type that hide the type of the other error in the cause box.
  • It collects a single backtrace at the earliest opportunity and
    propagates it down the stack through From and ChainErr
    conversions.

To accomplish its goals it makes some tradeoffs:

  • The split between the Error and ErrorKind types can make it
    slightly more cumbersome to introduce new, unchained,
    errors, requiring an Into or From conversion; as well as
    slightly more cumbersome to match on errors with another layer
    of types to match.
  • Because the error type contains std::error::Error + Send + 'static objects,
    it can't implement PartialEq for easy comparisons.

Declaring error types

Generally, you define one family of error types per crate, though
it's also perfectly fine to define error types on a finer-grained
basis, such as per module.

Assuming you are using crate-level error types, typically you will
define an errors module and inside it call declare_errors!:

declare_errors! {
    // The type defined for this error. These are the conventional
    // and recommended names, but they can be arbitrarily chosen.
    types {
        Error, ErrorKind, ChainErr, Result;
    }

    // Automatic conversions between this error chain and other
    // error chains. In this case, it will generate an
    // `ErrorKind` variant in turn containing `rustup_utils::ErrorKind`,
    // with conversions from `rustup_utils::Error`.
    //
    // This section can be empty.
    links {
        rustup_dist::Error, rustup_dist::ErrorKind, Dist;
        rustup_utils::Error, rustup_utils::ErrorKind, Utils;
    }

    // Automatic conversions between this error chain and other
    // error types not defined by this macro. These will be boxed
    // as the error cause, and their descriptions and display text
    // reused.

    // This section can be empty.
    foreign_links {
        temp::Error, Temp,
        "temporary file error";
    }

    // Define the `ErrorKind` variants. The syntax here is the
    // same as quick_error!, but the `from()` and `cause()`
    // syntax is not supported.
    errors {
        InvalidToolchainName(t: String) {
            description("invalid toolchain name")
            display("invalid toolchain name: '{}'", t)
        }
    }
}

This populates the the module with a number of definitions,
the most of important of which are the Error type
and the ErrorKind type. They look something like the
following:

use std::error::Error as StdError;
use std::sync::Arc;

pub struct Error(pub ErrorKind,
                 pub Option<Box<StdError + Send>>,
                 pub Arc<rustup_error::Backtrace>);

impl Error {
    pub fn kind(&self) -> &ErrorKind { ... }
    pub fn into_kind(self) -> ErrorKind { ... }
    pub fn iter(&self) -> rustup_error::ErrorChainIter { ... }
    pub fn backtrace(&self) -> &rustup_error::Backtrace { ... }
}

impl StdError for Error { ... }
impl Display for Error { ... }

pub enum ErrorKind {
    Msg(String),
    Dist(rustup_dist::ErrorKind),
    Utils(rustup_utils::ErrorKind),
    Temp,
    InvalidToolchainName(String),
}

This is the basic error structure. You can see that ErrorKind
has been populated in a variety of ways. All ErrorKinds get a
Msg variant for basic errors. When strings are converted to
ErrorKinds they become ErrorKind::Msg. The "links" defined in
the macro are expanded to Dist and Utils variants, and the
"foreign links" to the Temp variant.

Both types come with a variety of From conversians as well:
Error can be created from ErrorKind, from &str and String,
and from the "link" and "foreign_link" error types. ErrorKind
can be created from the corresponding ErrorKinds of the link
types, as wall as from &str and String.

into() andFrom::from` are used heavily to massage types into
the right shape. Which one to use in any specific case depends on
the influence of type inference, but there are some patterns that
arise frequently.

Returning new errors

Introducing new error chains, with a string message:

fn foo() -> Result<()> {
    Err("foo error!".into())
}

Introducing new error chains, with an ErrorKind:

fn foo() -> Result<()> {
    Err(ErrorKind::FooError.into())
}

Note that the return type is is the typedef Result, which is
defined by the macro as pub type Result<T> = ::std::result::Result<T, Error>. Note that in both cases
.into() is called to convert a type into the Error type: both
strings and ErrorKind have From conversions to turn them into
Error.

When the error is emitted inside a try! macro or behind the
? operator, then the explicit conversion isn't needed, since
the behavior of try! will automatically convert Err(ErrorKind)
to Err(Error). So the below is equivalent to the previous:

fn foo() -> Result<()> {
    Ok(try!(Err(ErrorKind::FooError)))
}

fn bar() -> Result<()> {
    Ok(try!(Err("bogus!")))

Chaining errors

To extend the error chain:

use errors::ChainErr;
try!(do_something().chain_err(|| "something went wrong"));

chain_err can be called on any Result type where the contained
error type implements std::error::Error + Send + 'static. If
the Result is an Err then chain_err evaluates the closure,
which returns some type that can be converted to ErrorKind,
boxes the original error to store as the cause, then returns a new
error containing the original error.

Foreign links

Errors that do not conform to the same conventions as this library
can still be included in the error chain. They are considered "foreign
errors", and are declared using the foreign_links block of the
declare_errors! macro. Errors are automatically created from
foreign errors by the try! macro.

Foreign links and regular links have one crucial difference:
From conversions for regular links do not introduce a new error
into the error chain
, while conversions for foreign links always
introduce a new error into the error chain
. So for the example
above all errors deriving from the temp::Error type will be
presented to the user as a new ErrorKind::Temp variant, and the
cause will be the original temp::Error error. In contrast, when
rustup_utils::Error is converted to Error the two ErrorKinds
are converted between each other to create a new Error but the
old error is discarded; there is no "cause" created from the
original error.

Backtraces

The earliest non-foreign error to be generated creates a single
backtrace, which is passed through all From conversions and
chain_err invocations of compatible types. To read the backtrace
just call the backtrace() method.

Iteration

The iter method returns an iterator over the chain of error boxes.

brson added 30 commits April 23, 2016 19:39
This combines quick_error! with some of Cargo's ideas.

It defines the ChainError trait and the ChainedError struct.

With it any Result<T, E> where E: std::error::Error + Send + 'static
has a chain_error method that takes yet another Error + Send + 'static.
It returns a Result<T, ChainedError> containing both the boxed
cause and the boxed new error.

BasicError is for simple errors that don't deserve a new variant,
and is used like BasicError::new("msg").

It then exports the easy_error! trait which is exactly like
quick_error! but automatically defines BasicError and ChainError
variants with From conversions from the corresponding types.
@brson brson closed this Apr 25, 2016
@brson brson reopened this Apr 25, 2016
@brson
Copy link
Contributor Author

brson commented Apr 25, 2016

Sorry for the sloppy writing. Been trying to get this done before the week starts.

@brson
Copy link
Contributor Author

brson commented Apr 25, 2016

I'd like to name this library error_chain. And I've got more ideas to improve it but need to move on and knock out other issues.

raw::symlink_dir(src, dest).map_err(|e| {
Error::LinkingDirectory {
Ok(try!(raw::symlink_dir(src, dest).chain_err(|| {
ErrorKind::LinkingDirectory {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this pattern of wrapping in Ok(try! is not necessary here, an artifact of earlier iterations.

@alexcrichton
Copy link
Member

I'm pretty stoked to read this diff

[root]
name = "rustup-error"
version = "0.1.7"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stray file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

@alexcrichton
Copy link
Member

Nice! The support library here seems pretty awesome. Overall this seems like huge win by not throwing away error contexts wherever possible and making it easier to define new errors, I'd be fine merging basically whenever you're comfortable as well.

Some thoughts of mine:

  • The #![recursion_limit] is unfortunate (esp if we want this to become a more widely-used crate). That seems an artifact of the massive macro, though, so maybe there's a way to have a stripped down version with less bells and whistles that fits in the default recursion limit?
  • The judicious use of .into() here is interesting. I'm not sure if that makes this brittle to changes in the standard library maybe? It also kinda encourages "oh did the types not match up? type into!" Seems fine to me, but it's an interesting convention.

@Diggsey
Copy link
Contributor

Diggsey commented Apr 25, 2016

Very nice! Do you think a similar refactoring would work for the Notification system? I see notifications and errors as being symmetric WRT variance, ie. errors are covariant while notifications are contravariant.

@brson
Copy link
Contributor Author

brson commented Apr 25, 2016

Very nice! Do you think a similar refactoring would work for the Notification system? I see notifications and errors as being symmetric WRT variance, ie. errors are covariant while notifications are contravariant.

The basic strategy this uses to compose and convert between error types might work for creating the notification heirarchy without nightly features, but I'm not sure.

One tricky thing with notifications is that they don't want to allocate. The errors make a huge concession that it's ok to do a lot of allocation work on the (rare) error path. Notifications though are on the non-error path, so it's harder to justify moving things onto the heap just to maybe print them to the console.

@brson
Copy link
Contributor Author

brson commented Apr 25, 2016

Seems to be freebsd/netbsd nightly issues.

@brson brson merged commit a378713 into rust-lang:master Apr 25, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants