From 0c3c21936486586dd487ca132f6182543a082e1a Mon Sep 17 00:00:00 2001 From: Magnus Hallin Date: Tue, 8 Aug 2017 21:26:51 +0200 Subject: [PATCH 1/3] Refiy FieldError as a real datatype, add a structured data field --- juniper/src/executor.rs | 103 ++++++++++++++++++++++--- juniper/src/executor_tests/executor.rs | 16 ++-- juniper/src/integrations/serde.rs | 9 ++- juniper/src/lib.rs | 13 ++-- juniper/src/macros/object.rs | 16 ++-- juniper/src/result_ext.rs | 14 ++-- 6 files changed, 136 insertions(+), 35 deletions(-) diff --git a/juniper/src/executor.rs b/juniper/src/executor.rs index 56c57e146..e668d2fea 100644 --- a/juniper/src/executor.rs +++ b/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; @@ -51,18 +53,101 @@ 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, + error: FieldError, +} + +impl Eq for ExecutionError {} + +impl PartialOrd for ExecutionError { + fn partial_cmp(&self, other: &ExecutionError) -> Option { + (&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) -> Result { +/// let s = String::from_utf8(data)?; +/// Ok(s) +/// } +/// ``` +#[derive(Debug, PartialEq)] +pub struct FieldError { message: String, + data: Value, +} + +impl From for FieldError { + fn from(e: T) -> FieldError { + FieldError { + message: format!("{}", e), + data: Value::null(), + } + } +} + +impl FieldError { + /// Construct a new error with additional data + /// + /// 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(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 = Result; +pub type FieldResult = Result; /// The result of resolving an unspecified field -pub type ExecutionResult = Result; +pub type ExecutionResult = Result; /// The map of variables used for substitution during query execution pub type Variables = HashMap; @@ -255,7 +340,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); @@ -264,7 +349,7 @@ impl<'a, CtxT> Executor<'a, CtxT> { errors.push(ExecutionError { location: location, path: path, - message: error, + error: error, }); } } @@ -289,17 +374,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 { - &self.message + pub fn error(&self) -> &FieldError { + &self.error } /// The source location _in the query_ of the field that failed to resolve diff --git a/juniper/src/executor_tests/executor.rs b/juniper/src/executor_tests/executor.rs index 45a4925f0..fb416c79d 100644 --- a/juniper/src/executor_tests/executor.rs +++ b/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> { 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()), ), ]); } diff --git a/juniper/src/integrations/serde.rs b/juniper/src/integrations/serde.rs index aabd68d3d..e92b4aeb3 100644 --- a/juniper/src/integrations/serde.rs +++ b/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() } } diff --git a/juniper/src/lib.rs b/juniper/src/lib.rs index 7b6c8c38b..24458b296 100644 --- a/juniper/src/lib.rs +++ b/juniper/src/lib.rs @@ -57,11 +57,12 @@ graphql_object!(User: Database |&self| { &self.name } - // FieldResult is an alias for Result - simply return - // a string from this method and it will be correctly inserted into - // the execution response. + // FieldResult is an alias for Result, which can be + // converted to from anything that implements std::fmt::Display - simply + // 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 @@ -153,8 +154,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, - IntoResolvable, Registry, Variables}; +pub use executor::{Context, ExecutionError, ExecutionResult, Executor, FieldError, FieldResult, + FromContext, IntoResolvable, Registry, Variables}; pub use validation::RuleError; pub use types::scalars::{EmptyMutation, ID}; pub use schema::model::RootNode; diff --git a/juniper/src/macros/object.rs b/juniper/src/macros/object.rs index 3cd0be332..459663515 100644 --- a/juniper/src/macros/object.rs +++ b/juniper/src/macros/object.rs @@ -120,10 +120,16 @@ even have to be backed by a trait! ## Emitting errors -`FieldResult` is a simple type alias for `Result`. In the end, -errors that fields emit are serialized 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. +`FieldResult` is a type alias for `Result`, where +`FieldResult` is a tuple that contains an error message and optionally a +JSON-like data structure. In the end, errors that fields emit are serialized +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())? } }); diff --git a/juniper/src/result_ext.rs b/juniper/src/result_ext.rs index 2c27a58f4..864ee472c 100644 --- a/juniper/src/result_ext.rs +++ b/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::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 { */ pub trait ResultExt { /// Convert the error to a string by using it's `Display` implementation - fn to_field_err(self) -> Result; + fn to_field_result(self) -> Result; } impl ResultExt for Result { - fn to_field_err(self) -> Result { - self.map_err(|e| format!("{}", e)) + fn to_field_result(self) -> Result { + 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)) } } From a97ac591355badc8cdb9f34983ad46bb745530d6 Mon Sep 17 00:00:00 2001 From: Magnus Hallin Date: Tue, 8 Aug 2017 22:27:45 +0200 Subject: [PATCH 2/3] Add value literal macro --- juniper/src/value.rs | 149 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/juniper/src/value.rs b/juniper/src/value.rs index 1ed0a02dd..732c36dbf 100644 --- a/juniper/src/value.rs +++ b/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 for Value { + fn from(s: String) -> Value { + Value::string(s) + } +} + +impl From for Value { + fn from(b: bool) -> Value { + Value::boolean(b) + } +} + +impl From for Value { + fn from(i: i32) -> Value { + Value::int(i) + } +} + +impl From for Value { + fn from(f: f64) -> Value { + Value::float(f) + } +} + +impl From> for Value where Value: From { + fn from(v: Option) -> 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)), + 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()) + ); + } +} From 6710d780cfc96a742b501a0f91110047fdd7aa58 Mon Sep 17 00:00:00 2001 From: Magnus Hallin Date: Tue, 8 Aug 2017 22:44:20 +0200 Subject: [PATCH 3/3] Update FieldError documentation with new value literal macro --- juniper/src/executor.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/juniper/src/executor.rs b/juniper/src/executor.rs index e668d2fea..d26ca6c57 100644 --- a/juniper/src/executor.rs +++ b/juniper/src/executor.rs @@ -109,6 +109,21 @@ impl From for FieldError { 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: ///