Safer and more idiomatic error handling

[kestred]

Juniper should not implement impl<T: Display> From<T> for FieldError.

This makes it more difficult to enforce good error handling semantics for custom error information that might be provided using the FieldError::new constructor, and also makes it more difficult for crates to be built on top of, or with feature-support for juniper; for example, by preventing implementation of impl From<LibraryError> for juniper::FieldError.

Here are a few alternatives to keep error handling clean and easy for basic users, while allowing gradual upgrade to more sophisticated error handling for advance users.

Some alternatives impact backwards compatibility, so it is recommended that this be implemented prior to a v1.0 release of juniper.

Recommend Design

Juniper should define and expose a ResultExt trait, such as:

pub trait FieldResultExt<T, E> {
    fn fmt_err(self) -> Result<T, FieldError>;

    // Allows easy application of the FieldError's 2nd `data` argument
    fn with_data(self, data: Value) -> Result<T, FieldError>;
}

impl<T, E> FieldResultExt<T, E> for Result<T, E>
    where E: Display
{
    // TODO: Implement
}

At that point the 1st example in the error handling section of the book would behave like:

use juniper::{FieldResult, FieldResultExt};

struct Example {
    filename: PathBuf,
}

graphql_object!(Example: () |&self| {
    field contents() -> FieldResult<String> {
        let mut file = File::open(&self.filename).fmt_err()?;
        let mut contents = String::new();
        file.read_to_string(&mut contents).fmt_err()?;
        Ok(contents)
    }
});

This solution may help with (although may not entirely solve) issue #40.

Alternative A

Internally, Juniper should not rely on the From<T: Display> implementation for FieldError.

For users that would like the trait implemented, they can opt-in to the display functionality with a new crate-level feature flag display_errors.

Alternative B

Internally, Juniper should not rely on the From<T: Display> implementation for FieldError.

For users that would like advanced error handling, they can opt-out the display functionality with a new crate-level feature flag manual_errors.

Alternative B is the least preferred solution, because it is less idiomatic rust and more importantly makes transitioning to more advanced error handling a fairly painful endeavor.

[thedodd]

I agree with this 100%. The blanket implementation can cause subtle problems in how errors are handled, and it also blocks you from being able to implement From/Into based coercion manually for full control.

[kestred]

The PR #205 implemented another alternative for this which solves my original goals for this issue; so I'll close it.