Propagate errors to closest nullable parent field

This also fixes a bug where context switching on optional values did
not properly work.

juniper/src/executor.rs

@@ -200,9 +200,9 @@ impl<'a, T: GraphQLType, C> IntoResolvable<'a, T, C> for (&'a T::Context, T) {
     }
 }
 
-impl<'a, T: GraphQLType, C> IntoResolvable<'a, T, C> for Option<(&'a T::Context, T)> {
+impl<'a, T: GraphQLType, C> IntoResolvable<'a, Option<T>, C> for Option<(&'a T::Context, T)> {
-    fn into(self, _: &'a C) -> FieldResult<Option<(&'a T::Context, T)>> {
+    fn into(self, _: &'a C) -> FieldResult<Option<(&'a T::Context, Option<T>)>> {
-        Ok(self)
+        Ok(self.map(|(ctx, v)| (ctx, Some(v))))
     }
 }
 
@@ -212,9 +212,9 @@ impl<'a, T: GraphQLType, C> IntoResolvable<'a, T, C> for FieldResult<(&'a T::Con
     }
 }
 
-impl<'a, T: GraphQLType, C> IntoResolvable<'a, T, C> for FieldResult<Option<(&'a T::Context, T)>> {
+impl<'a, T: GraphQLType, C> IntoResolvable<'a, Option<T>, C> for FieldResult<Option<(&'a T::Context, T)>> {
-    fn into(self, _: &'a C) -> FieldResult<Option<(&'a T::Context, T)>> {
+    fn into(self, _: &'a C) -> FieldResult<Option<(&'a T::Context, Option<T>)>> {
-        self
+        self.map(|o| o.map(|(ctx, v)| (ctx, Some(v))))
     }
 }
 

juniper/src/executor_tests/executor.rs

@@ -297,11 +297,44 @@ mod dynamic_context_switching {
     }
 
     #[test]
-    fn test_res() {
+    fn test_res_success() {
         let schema = RootNode::new(Schema, EmptyMutation::<OuterContext>::new());
         let doc = r"
           {
             first: itemRes(key: 0) { value }
+          }
+          ";
+
+        let vars = vec![].into_iter().collect();
+
+        let ctx = OuterContext {
+            items: vec![
+                (0, InnerContext { value: "First value".to_owned() }),
+                (1, InnerContext { value: "Second value".to_owned() }),
+            ].into_iter()
+                .collect(),
+        };
+
+        let (result, errs) = ::execute(doc, None, &schema, &vars, &ctx).expect("Execution failed");
+
+        assert_eq!(errs, vec![]);
+
+        println!("Result: {:?}", result);
+
+        assert_eq!(
+            result,
+            Value::object(vec![
+                ("first", Value::object(vec![
+                    ("value", Value::string("First value")),
+                ].into_iter().collect())),
+            ].into_iter().collect()));
+    }
+
+    #[test]
+    fn test_res_fail() {
+        let schema = RootNode::new(Schema, EmptyMutation::<OuterContext>::new());
+        let doc = r"
+          {
             missing: itemRes(key: 2) { value }
           }
           ";
@@ -320,7 +353,7 @@ mod dynamic_context_switching {
 
         assert_eq!(errs, vec![
             ExecutionError::new(
-                SourcePosition::new(70, 3, 12),
+                SourcePosition::new(25, 2, 12),
                 &["missing"],
                 FieldError::new("Could not find key 2", Value::null()),
             ),
@@ -328,14 +361,7 @@ mod dynamic_context_switching {
 
         println!("Result: {:?}", result);
 
-        assert_eq!(
+        assert_eq!(result, Value::null());
-            result,
-            Value::object(vec![
-                ("first", Value::object(vec![
-                    ("value", Value::string("First value")),
-                ].into_iter().collect())),
-                ("missing", Value::null()),
-            ].into_iter().collect()));
     }
 
     #[test]
@@ -413,7 +439,7 @@ mod dynamic_context_switching {
     }
 }
 
-mod nulls_out_errors {
+mod propagates_errors_to_nullable_fields {
     use value::Value;
     use schema::model::RootNode;
     use executor::{ExecutionError, FieldError, FieldResult};
@@ -421,16 +447,23 @@ mod nulls_out_errors {
     use types::scalars::EmptyMutation;
 
     struct Schema;
+    struct Inner;
 
     graphql_object!(Schema: () |&self| {
-        field sync() -> FieldResult<&str> { Ok("sync") }
+        field inner() -> Inner { Inner }
-        field sync_error() -> FieldResult<&str> { Err("Error for syncError")? }
+    });
+
+    graphql_object!(Inner: () |&self| {
+        field nullable_field() -> Option<Inner> { Some(Inner) }
+        field non_nullable_field() -> Inner { Inner }
+        field nullable_error_field() -> FieldResult<Option<&str>> { Err("Error for nullableErrorField")? }
+        field non_nullable_error_field() -> FieldResult<&str> { Err("Error for nonNullableErrorField")? }
     });
 
     #[test]
-    fn test() {
+    fn nullable_first_level() {
         let schema = RootNode::new(Schema, EmptyMutation::<()>::new());
-        let doc = r"{ sync, syncError }";
+        let doc = r"{ inner { nullableErrorField } }";
 
         let vars = vec![].into_iter().collect();
 
@@ -440,18 +473,119 @@ mod nulls_out_errors {
 
         assert_eq!(
             result,
-            Value::object(vec![
+            graphql_value!({ "inner": { "nullableErrorField": None } }));
-                ("sync", Value::string("sync")),
-                ("syncError", Value::null()),
-            ].into_iter().collect()));
 
         assert_eq!(
             errs,
             vec![
                 ExecutionError::new(
-                    SourcePosition::new(8, 0, 8),
+                    SourcePosition::new(10, 0, 10),
-                    &["syncError"],
+                    &["inner", "nullableErrorField"],
-                    FieldError::new("Error for syncError", Value::null()),
+                    FieldError::new("Error for nullableErrorField", Value::null()),
+                ),
+            ]);
+    }
+
+    #[test]
+    fn non_nullable_first_level() {
+        let schema = RootNode::new(Schema, EmptyMutation::<()>::new());
+        let doc = r"{ inner { nonNullableErrorField } }";
+
+        let vars = vec![].into_iter().collect();
+
+        let (result, errs) = ::execute(doc, None, &schema, &vars, &()).expect("Execution failed");
+
+        println!("Result: {:?}", result);
+
+        assert_eq!(
+            result,
+            graphql_value!(None));
+
+        assert_eq!(
+            errs,
+            vec![
+                ExecutionError::new(
+                    SourcePosition::new(10, 0, 10),
+                    &["inner", "nonNullableErrorField"],
+                    FieldError::new("Error for nonNullableErrorField", Value::null()),
+                ),
+            ]);
+    }
+
+    #[test]
+    fn nullable_nested_level() {
+        let schema = RootNode::new(Schema, EmptyMutation::<()>::new());
+        let doc = r"{ inner { nullableField { nonNullableErrorField } } }";
+
+        let vars = vec![].into_iter().collect();
+
+        let (result, errs) = ::execute(doc, None, &schema, &vars, &()).expect("Execution failed");
+
+        println!("Result: {:?}", result);
+
+        assert_eq!(
+            result,
+            graphql_value!({ "inner": { "nullableField": None } }));
+
+        assert_eq!(
+            errs,
+            vec![
+                ExecutionError::new(
+                    SourcePosition::new(26, 0, 26),
+                    &["inner", "nullableField", "nonNullableErrorField"],
+                    FieldError::new("Error for nonNullableErrorField", Value::null()),
+                ),
+            ]);
+    }
+
+    #[test]
+    fn non_nullable_nested_level() {
+        let schema = RootNode::new(Schema, EmptyMutation::<()>::new());
+        let doc = r"{ inner { nonNullableField { nonNullableErrorField } } }";
+
+        let vars = vec![].into_iter().collect();
+
+        let (result, errs) = ::execute(doc, None, &schema, &vars, &()).expect("Execution failed");
+
+        println!("Result: {:?}", result);
+
+        assert_eq!(
+            result,
+            graphql_value!(None));
+
+        assert_eq!(
+            errs,
+            vec![
+                ExecutionError::new(
+                    SourcePosition::new(29, 0, 29),
+                    &["inner", "nonNullableField", "nonNullableErrorField"],
+                    FieldError::new("Error for nonNullableErrorField", Value::null()),
+                ),
+            ]);
+    }
+
+    #[test]
+    fn nullable_innermost() {
+        let schema = RootNode::new(Schema, EmptyMutation::<()>::new());
+        let doc = r"{ inner { nonNullableField { nullableErrorField } } }";
+
+        let vars = vec![].into_iter().collect();
+
+        let (result, errs) = ::execute(doc, None, &schema, &vars, &()).expect("Execution failed");
+
+        println!("Result: {:?}", result);
+
+        assert_eq!(
+            result,
+            graphql_value!({ "inner": { "nonNullableField": { "nullableErrorField": None } } }));
+
+        assert_eq!(
+            errs,
+            vec![
+                ExecutionError::new(
+                    SourcePosition::new(29, 0, 29),
+                    &["inner", "nonNullableField", "nullableErrorField"],
+                    FieldError::new("Error for nullableErrorField", Value::null()),
                 ),
             ]);
     }

juniper/src/lib.rs

@@ -138,11 +138,12 @@ extern crate uuid;
 
 use std::borrow::Cow;
 
+#[macro_use]
+mod value;
 #[macro_use]
 mod macros;
 mod ast;
 pub mod parser;
-mod value;
 mod types;
 mod schema;
 mod validation;

juniper/src/macros/tests/object.rs

@@ -5,6 +5,7 @@ use ast::InputValue;
 use value::Value;
 use schema::model::RootNode;
 use types::scalars::EmptyMutation;
+use executor::{FieldResult, Context};
 
 /*
 
@@ -14,6 +15,7 @@ Syntax to validate:
 * Optional Generics/lifetimes
 * Custom name vs. default name
 * Optional commas between items
+* Nullable/fallible context switching
 
  */
 
@@ -102,6 +104,22 @@ graphql_object!(CommasOnMeta: () |&self| {
     field simple() -> i32 { 0 }
 });
 
+struct InnerContext;
+const INNER_CONTEXT: InnerContext = InnerContext;
+impl Context for InnerContext {}
+
+struct InnerType;
+graphql_object!(InnerType: InnerContext |&self| {
+});
+
+struct CtxSwitcher;
+graphql_object!(CtxSwitcher: () |&self| {
+    field ctx_switch_always() -> (&InnerContext, InnerType) { (&INNER_CONTEXT, InnerType) }
+    field ctx_switch_opt() -> Option<(&InnerContext, InnerType)> { Some((&INNER_CONTEXT, InnerType)) }
+    field ctx_switch_res() -> FieldResult<(&InnerContext, InnerType)> { Ok((&INNER_CONTEXT, InnerType)) }
+    field ctx_switch_res_opt() -> FieldResult<Option<(&InnerContext, InnerType)>> { Ok(Some((&INNER_CONTEXT, InnerType))) }
+});
+
 graphql_object!(<'a> Root: () as "Root" |&self| {
     field custom_name() -> CustomName { CustomName {} }
 
@@ -114,6 +132,8 @@ graphql_object!(<'a> Root: () as "Root" |&self| {
 
     field commas_with_trailing() -> CommasWithTrailing { CommasWithTrailing {} }
     field commas_on_meta() -> CommasOnMeta { CommasOnMeta {} }
+
+    field ctx_switcher() -> CtxSwitcher { CtxSwitcher {} }
 });
 
 
@@ -128,6 +148,14 @@ where
             description
             fields(includeDeprecated: true) {
                 name
+                type {
+                    kind
+                    name
+                    ofType {
+                        kind
+                        name
+                    }
+                }
             }
             interfaces {
                 name
@@ -172,9 +200,10 @@ fn introspect_custom_name() {
         assert_eq!(object.get("description"), Some(&Value::null()));
         assert_eq!(object.get("interfaces"), Some(&Value::list(vec![])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -185,9 +214,10 @@ fn introspect_with_lifetime() {
         assert_eq!(object.get("description"), Some(&Value::null()));
         assert_eq!(object.get("interfaces"), Some(&Value::list(vec![])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -198,9 +228,10 @@ fn introspect_with_generics() {
         assert_eq!(object.get("description"), Some(&Value::null()));
         assert_eq!(object.get("interfaces"), Some(&Value::list(vec![])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -216,9 +247,10 @@ fn introspect_description_first() {
             ].into_iter().collect()),
         ])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -234,9 +266,10 @@ fn introspect_fields_first() {
             ].into_iter().collect()),
         ])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -252,9 +285,10 @@ fn introspect_interfaces_first() {
             ].into_iter().collect()),
         ])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -270,9 +304,10 @@ fn introspect_commas_with_trailing() {
             ].into_iter().collect()),
         ])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
     });
 }
 
@@ -288,8 +323,56 @@ fn introspect_commas_on_meta() {
             ].into_iter().collect()),
         ])));
 
-        assert!(fields.contains(&Value::object(vec![
+        assert!(fields.contains(&graphql_value!({
-            ("name", Value::string("simple")),
+            "name": "simple", 
-        ].into_iter().collect())));
+            "type": { "kind": "NON_NULL", "name": None, "ofType": { "kind": "SCALAR", "name": "Int" } }
+        })));
+    });
+}
+
+#[test]
+fn introspect_ctx_switch() {
+    run_type_info_query("CtxSwitcher", |_, fields| {
+        assert!(fields.contains(&graphql_value!({
+            "name": "ctxSwitchAlways",
+            "type": {
+                "kind": "NON_NULL",
+                "name": None,
+                "ofType": {
+                    "kind": "OBJECT",
+                    "name": "InnerType",
+                }
+            }
+        })));
+
+        assert!(fields.contains(&graphql_value!({
+            "name": "ctxSwitchOpt",
+            "type": {
+                "kind": "OBJECT",
+                "name": "InnerType",
+                "ofType": None
+            }
+        })));
+
+        assert!(fields.contains(&graphql_value!({
+            "name": "ctxSwitchRes",
+            "type": {
+                "kind": "NON_NULL",
+                "name": None,
+                "ofType": {
+                    "kind": "OBJECT",
+                    "name": "InnerType",
+                }
+            }
+        })));
+
+        assert!(fields.contains(&graphql_value!({
+            "name": "ctxSwitchResOpt",
+            "type": {
+                "kind": "OBJECT",
+                "name": "InnerType",
+                "ofType": None
+            }
+        })));
     });
 }

juniper/src/types/base.rs

@@ -307,8 +307,11 @@ pub trait GraphQLType: Sized {
     ) -> Value {
         if let Some(selection_set) = selection_set {
             let mut result = OrderMap::new();
-            resolve_selection_set_into(self, info, selection_set, executor, &mut result);
+            if resolve_selection_set_into(self, info, selection_set, executor, &mut result) {
                 Value::object(result)
+            } else {
+                Value::null()
+            }
         } else {
             panic!("resolve() must be implemented by non-object output types");
         }
@@ -321,7 +324,7 @@ fn resolve_selection_set_into<T, CtxT>(
     selection_set: &[Selection],
     executor: &Executor<CtxT>,
     result: &mut OrderMap<String, Value>,
-) where
+) -> bool where
     T: GraphQLType<Context = CtxT>,
 {
     let meta_type = executor
@@ -388,9 +391,15 @@ fn resolve_selection_set_into<T, CtxT>(
                 );
 
                 match field_result {
+                    Ok(Value::Null) if meta_field.field_type.is_non_null() => return false,
                     Ok(v) => merge_key_into(result, response_name, v),
                     Err(e) => {
                         sub_exec.push_error_at(e, start_pos.clone());
+
+                        if meta_field.field_type.is_non_null() {
+                            return false;
+                        }
+
                         result.insert((*response_name).to_owned(), Value::null());
                     }
                 }
@@ -406,13 +415,15 @@ fn resolve_selection_set_into<T, CtxT>(
                     .fragment_by_name(spread.name.item)
                     .expect("Fragment could not be found");
 
-                resolve_selection_set_into(
+                if !resolve_selection_set_into(
                     instance,
                     info,
                     &fragment.selection_set[..],
                     executor,
                     result,
-                );
+                ) {
+                    return false;
+                }
             }
             Selection::InlineFragment(Spanning {
                 item: ref fragment,
@@ -442,19 +453,23 @@ fn resolve_selection_set_into<T, CtxT>(
                         sub_exec.push_error_at(e, start_pos.clone());
                     }
                 } else {
-                    resolve_selection_set_into(
+                    if !resolve_selection_set_into(
                         instance,
                         info,
                         &fragment.selection_set[..],
                         &sub_exec,
                         result,
-                    );
+                    ) {
+                        return false;
                     }
                 }
             }
         }
     }
     
+    true
+}
+
 fn is_excluded(directives: &Option<Vec<Spanning<Directive>>>, vars: &Variables) -> bool {
     if let Some(ref directives) = *directives {
         for &Spanning {

juniper/src/value.rs

@@ -205,6 +205,7 @@ macro_rules! graphql_value {
             $( ($key, graphql_value!($val)), )*
         ].into_iter().collect())
     };
+    (None) => ($crate::Value::null());
     ($e:expr) => ($crate::Value::from($e))
 }
 
@@ -251,7 +252,7 @@ mod tests {
             Value::string("test")
         );
         assert_eq!(
-            graphql_value!((None as Option<String>)),
+            graphql_value!(None),
             Value::null()
         );
     }