Merge pull request #80 from graphql-rust/field-error-refactor

Field error refactor

juniper/src/executor.rs

@@ -1,3 +1,5 @@
+use std::cmp::Ordering;
+use std::fmt::Display;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::sync::RwLock;
@@ -52,18 +54,116 @@ where
 ///
 /// All execution errors contain the source position in the query of the field
 /// that failed to resolve. It also contains the field stack.
-#[derive(Debug, PartialOrd, Ord, PartialEq, Eq)]
+#[derive(Debug, PartialEq)]
 pub struct ExecutionError {
     location: SourcePosition,
     path: Vec<String>,
+    error: FieldError,
+}
+
+impl Eq for ExecutionError {}
+
+impl PartialOrd for ExecutionError {
+    fn partial_cmp(&self, other: &ExecutionError) -> Option<Ordering> {
+        (&self.location, &self.path, &self.error.message)
+            .partial_cmp(&(&other.location, &other.path, &other.error.message))
+    }
+}
+
+impl Ord for ExecutionError {
+    fn cmp(&self, other: &ExecutionError) -> Ordering {
+        (&self.location, &self.path, &self.error.message)
+            .cmp(&(&other.location, &other.path, &other.error.message))
+    }
+}
+
+/// Error type for errors that occur during field resolution
+///
+/// Field errors are represented by a human-readable error message and an
+/// optional `Value` structure containing additional information.
+///
+/// They can be converted to from any type that implements `std::fmt::Display`,
+/// which makes error chaining with the `?` operator a breeze:
+///
+/// ```rust
+/// # use juniper::FieldError;
+/// fn get_string(data: Vec<u8>) -> Result<String, FieldError> {
+///     let s = String::from_utf8(data)?;
+///     Ok(s)
+/// }
+/// ```
+#[derive(Debug, PartialEq)]
+pub struct FieldError {
     message: String,
+    data: Value,
+}
+
+impl<T: Display> From<T> for FieldError {
+    fn from(e: T) -> FieldError {
+        FieldError {
+            message: format!("{}", e),
+            data: Value::null(),
+        }
+    }
+}
+
+impl FieldError {
+    /// Construct a new error with additional data
+    ///
+    /// You can use the `graphql_value!` macro to construct an error:
+    ///
+    /// ```rust
+    /// # #[macro_use] extern crate juniper;
+    /// use juniper::FieldError;
+    ///
+    /// # fn sample() {
+    /// FieldError::new(
+    ///     "Could not open connection to the database",
+    ///     graphql_value!({ "internal_error": "Connection refused" })
+    /// );
+    /// # }
+    /// # fn main() { }
+    /// ```
+    ///
+    /// The `data` parameter will be added to the `"data"` field of the error
+    /// object in the JSON response:
+    ///
+    /// ```json
+    /// {
+    ///   "errors": [
+    ///     "message": "Could not open connection to the database",
+    ///     "locations": [{"line": 2, "column": 4}],
+    ///     "data": {
+    ///       "internal_error": "Connection refused"
+    ///     }
+    ///   ]
+    /// }
+    /// ```
+    ///
+    /// If the argument is `Value::null()`, no extra data will be included.
+    pub fn new<T: Display>(e: T, data: Value) -> FieldError {
+        FieldError {
+            message: format!("{}", e),
+            data: data,
+        }
+    }
+
+    #[doc(hidden)]
+    pub fn message(&self) -> &str {
+        &self.message
+    }
+
+    #[doc(hidden)]
+    pub fn data(&self) -> &Value {
+        &self.data
+    }
 }
 
 /// The result of resolving the value of a field of type `T`
-pub type FieldResult<T> = Result<T, String>;
+pub type FieldResult<T> = Result<T, FieldError>;
 
 /// The result of resolving an unspecified field
-pub type ExecutionResult = Result<Value, String>;
+pub type ExecutionResult = Result<Value, FieldError>;
 
 /// The map of variables used for substitution during query execution
 pub type Variables = HashMap<String, InputValue>;
@@ -256,7 +356,7 @@ impl<'a, CtxT> Executor<'a, CtxT> {
     }
 
     /// Add an error to the execution engine
-    pub fn push_error(&self, error: String, location: SourcePosition) {
+    pub fn push_error(&self, error: FieldError, location: SourcePosition) {
         let mut path = Vec::new();
         self.field_path.construct_path(&mut path);
 
@@ -265,7 +365,7 @@ impl<'a, CtxT> Executor<'a, CtxT> {
         errors.push(ExecutionError {
             location: location,
             path: path,
-            message: error,
+            error: error,
         });
     }
 }
@@ -290,17 +390,17 @@ impl<'a> FieldPath<'a> {
 
 impl ExecutionError {
     #[doc(hidden)]
-    pub fn new(location: SourcePosition, path: &[&str], message: &str) -> ExecutionError {
+    pub fn new(location: SourcePosition, path: &[&str], error: FieldError) -> ExecutionError {
         ExecutionError {
             location: location,
             path: path.iter().map(|s| (*s).to_owned()).collect(),
-            message: message.to_owned(),
+            error: error,
         }
     }
 
     /// The error message
-    pub fn message(&self) -> &str {
+    pub fn error(&self) -> &FieldError {
-        &self.message
+        &self.error
     }
 
     /// The source location _in the query_ of the field that failed to resolve

juniper/src/executor_tests/executor.rs

@@ -215,7 +215,8 @@ mod dynamic_context_switching {
     use types::scalars::EmptyMutation;
     use schema::model::RootNode;
     use parser::SourcePosition;
-    use executor::{Context, ExecutionError, FieldResult};
+    use executor::{Context, ExecutionError, FieldError, FieldResult};
+    use result_ext::ResultExt;
 
     struct Schema;
 
@@ -241,11 +242,12 @@ mod dynamic_context_switching {
             executor.context().items.get(&key)
                 .ok_or(format!("Could not find key {}", key))
                 .map(|c| (c, ItemRef))
+                .to_field_result()
         }
 
         field item_res_opt(&executor, key: i32) -> FieldResult<Option<(&InnerContext, ItemRef)>> {
             if key > 100 {
-                Err(format!("Key too large: {}", key))
+                Err(format!("Key too large: {}", key)).to_field_result()
             } else {
                 Ok(executor.context().items.get(&key)
                    .map(|c| (c, ItemRef)))
@@ -320,7 +322,7 @@ mod dynamic_context_switching {
             ExecutionError::new(
                 SourcePosition::new(70, 3, 12),
                 &["missing"],
-                "Could not find key 2",
+                FieldError::new("Could not find key 2", Value::null()),
             ),
         ]);
 
@@ -363,7 +365,7 @@ mod dynamic_context_switching {
             ExecutionError::new(
                 SourcePosition::new(123, 4, 12),
                 &["tooLarge"],
-                "Key too large: 200",
+                FieldError::new("Key too large: 200", Value::null()),
             ),
         ]);
 
@@ -414,7 +416,7 @@ mod dynamic_context_switching {
 mod nulls_out_errors {
     use value::Value;
     use schema::model::RootNode;
-    use executor::{ExecutionError, FieldResult};
+    use executor::{ExecutionError, FieldError, FieldResult};
     use parser::SourcePosition;
     use types::scalars::EmptyMutation;
 
@@ -422,7 +424,7 @@ mod nulls_out_errors {
 
     graphql_object!(Schema: () |&self| {
         field sync() -> FieldResult<&str> { Ok("sync") }
-        field sync_error() -> FieldResult<&str> { Err("Error for syncError".to_owned()) }
+        field sync_error() -> FieldResult<&str> { Err("Error for syncError")? }
     });
 
     #[test]
@@ -449,7 +451,7 @@ mod nulls_out_errors {
                 ExecutionError::new(
                     SourcePosition::new(8, 0, 8),
                     &["syncError"],
-                    "Error for syncError",
+                    FieldError::new("Error for syncError", Value::null()),
                 ),
             ]);
     }

juniper/src/integrations/serde.rs

@@ -15,10 +15,10 @@ impl ser::Serialize for ExecutionError {
     where
         S: ser::Serializer,
     {
-        let mut map = try!(serializer.serialize_map(Some(3)));
+        let mut map = try!(serializer.serialize_map(Some(4)));
 
         try!(map.serialize_key("message"));
-        try!(map.serialize_value(self.message()));
+        try!(map.serialize_value(self.error().message()));
 
         let locations = vec![self.location()];
         try!(map.serialize_key("locations"));
@@ -27,6 +27,11 @@ impl ser::Serialize for ExecutionError {
         try!(map.serialize_key("path"));
         try!(map.serialize_value(self.path()));
 
+        if !self.error().data().is_null() {
+            try!(map.serialize_key("data"));
+            try!(map.serialize_value(self.error().data()));
+        }
+
         map.end()
     }
 }

juniper/src/lib.rs

@@ -57,11 +57,12 @@ graphql_object!(User: Database |&self| {
         &self.name
     }
 
-    // FieldResult<T> is an alias for Result<T, String> - simply return
+    // FieldResult<T> is an alias for Result<T, FieldError>, which can be
-    // a string from this method and it will be correctly inserted into
+    // converted to from anything that implements std::fmt::Display - simply
-    // the execution response.
+    // return an error with a string using the ? operator from this method and
+    // it will be correctly inserted into the execution response.
     field secret() -> FieldResult<&String> {
-        Err("Can't touch this".to_owned())
+        Err("Can't touch this".to_owned())?
     }
 
     // Field accessors can optionally take an "executor" as their first
@@ -158,8 +159,8 @@ use executor::execute_validated_query;
 pub use ast::{FromInputValue, InputValue, Selection, ToInputValue, Type};
 pub use value::Value;
 pub use types::base::{Arguments, GraphQLType, TypeKind};
-pub use executor::{Context, ExecutionError, ExecutionResult, Executor, FieldResult, FromContext,
+pub use executor::{Context, ExecutionError, ExecutionResult, Executor, FieldError, FieldResult,
-                   IntoResolvable, Registry, Variables};
+                   FromContext, IntoResolvable, Registry, Variables};
 pub use validation::RuleError;
 pub use types::scalars::{EmptyMutation, ID};
 pub use schema::model::RootNode;

juniper/src/macros/object.rs

@@ -120,10 +120,16 @@ even have to be backed by a trait!
 
 ## Emitting errors
 
-`FieldResult<T>` is a simple type alias for `Result<T, String>`. In the end,
+`FieldResult<T>` is a type alias for `Result<T, FieldError>`, where
-errors that fields emit are serialized into strings in the response. However,
+`FieldResult` is a tuple that contains an error message and optionally a
-the execution system will keep track of the source of all errors, and will
+JSON-like data structure. In the end, errors that fields emit are serialized
-continue executing despite some fields failing.
+into strings in the response. However, the execution system will keep track of
+the source of all errors, and will continue executing despite some fields
+failing.
+
+Anything that implements `std::fmt::Display` can be converted to a `FieldError`
+automatically via the `?` operator, or you can construct them yourself using
+`FieldError::new`.
 
 ```
 # #[macro_use] extern crate juniper;
@@ -136,7 +142,7 @@ graphql_object!(User: () |&self| {
     }
 
     field name() -> FieldResult<&String> {
-        Err("Does not have a name".to_owned())
+        Err("Does not have a name".to_owned())?
     }
 });
 

juniper/src/result_ext.rs

@@ -1,20 +1,22 @@
 use std::fmt;
 use std::result::Result;
 
+use FieldError;
+
 /**
 Helper trait to produce `FieldResult`s
 
 `FieldResult` only have strings as errors as that's what's going out
 in the GraphQL response. As such, all errors must be manually
 converted to strings. Importing the `ResultExt` macro and using its
-only method `to_field_err` can help with that:
+only method `to_field_result` can help with that:
 
 ```rust
 use std::str::FromStr;
 use juniper::{FieldResult, ResultExt};
 
 fn sample_fn(s: &str) -> FieldResult<i32> {
-    i32::from_str(s).to_field_err()
+    i32::from_str(s).to_field_result()
 }
 
 # fn main() { assert_eq!(sample_fn("12"), Ok(12)); }
@@ -42,12 +44,12 @@ fn sample_fn(s: &str) -> FieldResult<i32> {
  */
 pub trait ResultExt<T, E: fmt::Display> {
     /// Convert the error to a string by using it's `Display` implementation
-    fn to_field_err(self) -> Result<T, String>;
+    fn to_field_result(self) -> Result<T, FieldError>;
 }
 
 impl<T, E: fmt::Display> ResultExt<T, E> for Result<T, E> {
-    fn to_field_err(self) -> Result<T, String> {
+    fn to_field_result(self) -> Result<T, FieldError> {
-        self.map_err(|e| format!("{}", e))
+        self.map_err(|e| FieldError::from(e))
     }
 }
 
@@ -59,5 +61,5 @@ trait.
  */
 #[macro_export]
 macro_rules! jtry {
-    ( $e:expr ) => { try!($crate::ResultExt::to_field_err($e)) }
+    ( $e:expr ) => { try!($crate::ResultExt::to_field_result($e)) }
 }

juniper/src/value.rs

@@ -130,3 +130,152 @@ impl ToInputValue for Value {
         }
     }
 }
+
+impl<'a> From<&'a str> for Value {
+    fn from(s: &'a str) -> Value {
+        Value::string(s)
+    }
+}
+
+impl From<String> for Value {
+    fn from(s: String) -> Value {
+        Value::string(s)
+    }
+}
+
+impl From<bool> for Value {
+    fn from(b: bool) -> Value {
+        Value::boolean(b)
+    }
+}
+
+impl From<i32> for Value {
+    fn from(i: i32) -> Value {
+        Value::int(i)
+    }
+}
+
+impl From<f64> for Value {
+    fn from(f: f64) -> Value {
+        Value::float(f)
+    }
+}
+
+impl<T> From<Option<T>> for Value where Value: From<T> {
+    fn from(v: Option<T>) -> Value {
+        match v {
+            Some(v) => Value::from(v),
+            None => Value::null()
+        }
+    }
+}
+
+/// Construct JSON-like values by using JSON syntax
+///
+/// This macro can be used to create `Value` instances using a JSON syntax.
+/// Value objects are used mostly when creating custom errors from fields.
+///
+/// Here are some examples; the resulting JSON will look just like what you
+/// passed in.
+/// ```rust
+/// #[macro_use] extern crate juniper;
+/// 
+/// graphql_value!(1234);
+/// graphql_value!("test");
+/// graphql_value!([ 1234, "test", true ]);
+/// graphql_value!({ "key": "value", "foo": 1234 });
+/// ```
+#[macro_export]
+macro_rules! graphql_value {
+    ([ $($arg:tt),* $(,)* ]) => {
+        $crate::Value::list(vec![
+            $( graphql_value!($arg), )*
+        ])
+    };
+    ({ $($key:tt : $val:tt ),* $(,)* }) => {
+        $crate::Value::object(vec![
+            $( ($key, graphql_value!($val)), )*
+        ].into_iter().collect())
+    };
+    ($e:expr) => ($crate::Value::from($e))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::Value;
+
+    #[test]
+    fn value_macro_string() {
+        assert_eq!(
+            graphql_value!("test"),
+            Value::string("test")
+        );
+    }
+
+    #[test]
+    fn value_macro_int() {
+        assert_eq!(
+            graphql_value!(123),
+            Value::int(123)
+        );
+    }
+
+    #[test]
+    fn value_macro_float() {
+        assert_eq!(
+            graphql_value!(123.5),
+            Value::float(123.5)
+        );
+    }
+
+    #[test]
+    fn value_macro_boolean() {
+        assert_eq!(
+            graphql_value!(false),
+            Value::boolean(false)
+        );
+    }
+
+    #[test]
+    fn value_macro_option() {
+        assert_eq!(
+            graphql_value!(Some("test")),
+            Value::string("test")
+        );
+        assert_eq!(
+            graphql_value!((None as Option<String>)),
+            Value::null()
+        );
+    }
+
+    #[test]
+    fn value_macro_list() {
+        assert_eq!(
+            graphql_value!([ 123, "Test", false ]),
+            Value::list(vec![
+                Value::int(123),
+                Value::string("Test"),
+                Value::boolean(false),
+            ])
+        );
+        assert_eq!(
+            graphql_value!([ 123, [ 456 ], 789 ]),
+            Value::list(vec![
+                Value::int(123),
+                Value::list(vec![ Value::int(456) ]),
+                Value::int(789),
+            ])
+        );
+    }
+
+    #[test]
+    fn value_macro_object() {
+        assert_eq!(
+            graphql_value!({ "key": 123, "next": true }),
+            Value::object(vec![
+                ("key", Value::int(123)),
+                ("next", Value::boolean(true)),
+            ].into_iter().collect())
+        );
+    }
+}