Repository: graphql-rust/juniper (language: rust)

Issue:
Interface field resolving causes "Concrete type is not handled by instance resolvers on GraphQL union"
_The bug initially was found while using [juniper-from-schema](https://github.com/davidpdrsn/juniper-from-schema) library. Additional information can be found in [this issue](https://github.com/davidpdrsn/juniper-from-schema/issues/130). Nevertheless, I will repeat the description of the error here:_

I have some field that returns an union type:
```graphql
type Query {
  testField: TestFieldResult!
}

union TestFieldResult =
  | Ok
  | SomeProblem
  | AnotherProblem

type Ok {
  dummy: Boolean!
}
```

`SomeProblem` and `AnotherProblem` are implementing one shared interface:
```graphql
type SomeProblem implements ProblemInterface {
  message: String!
}

type AnotherProblem implements ProblemInterface {
  message: String!
}

interface ProblemInterface {
  message: String!
}
```

Now I'm trying to perform the query and I just need to know that there is a problem, I am not interested in a specific one:
```graphql
query {
  testField {
    __typename
    ... on Ok {
      dummy
    }
    ... on ProblemInterface {
      message
    }
  }
}
```

And I getting an error:
```
thread 'actix-rt:worker:2' panicked at 'Concrete type ProblemInterface is not handled by instance resolvers on GraphQL union TestFieldResult'
```

However, if you specify a specific type of problem, then everything works.

Gist to reproduce an error: https://gist.github.com/acelot/0f54704452b9cff373d280ea3ee94d81



Discussion / hints:
@acelot this is not a bug. What you want is not allowed by [GraphQL spec](https://spec.graphql.org/June2018/#sec-Unions).

Given the syntax `...on Type` on GraphQL union means that you're trying to downcast the union into its variant (member). However, the spec is quite explicit about what types can represent GraphQL union variants:
> 2. The member types of a Union type must all be Object base types; Scalar, Interface and Union types must not be member types of a Union. Similarly, wrapping types must not be member types of a Union.

So you cannot specify GraphQL interface in `...on Type` syntax on the GraphQL union, nor you can make a GraphQL interface a variant of the union. GraphQL is not intended to work like that, sorry 🤷‍♂️

But such situation should definitely not result in a panic. Need to tune up validation rules.
it's a shame it doesn't work that way.  Didn't know it was written in the spec.  Strange, but other libraries support this feature like `gqlgen` and `HotChocolate`.
@acelot examining it more deeply, seems that I was wrong. Spec has [only this about spreading interfaces on unions and vice versa](https://spec.graphql.org/June2018/#sec-Abstract-Spreads-in-Abstract-Scope):
> Union or interfaces fragments can be used within each other. As long as there exists at least one object type that exists in the intersection of the possible types of the scope and the spread, the spread is considered valid.
>
>So for example
>
> ```graphql
> fragment unionWithInterface on Pet {
>   ...dogOrHumanFragment
> }
> 
> fragment dogOrHumanFragment on DogOrHuman {
>   ... on Dog {
>     barkVolume
>   }
> }
> ```
> is consider valid because Dog implements interface Pet and is a member of DogOrHuman.

So, should figure out how to fix this...
Any news on this?
@acelot nope, haven't yet time to dive into this.
Just wanted to add that the "learn" documentation on graphql.org also deals with this when discussing unions
https://graphql.org/learn/schema/#union-types
> Also, in this case, since Human and Droid share a common interface (Character), you can query their common fields in one place rather than having to repeat the same fields across multiple types:
```{
  search(text: "an") {
    __typename
    ... on Character {
      name
    }
    ... on Human {
      height
    }
    ... on Droid {
      primaryFunction
    }
    ... on Starship {
      name
      length
    }
  }
}
```
Need to apply some workarounds in my schema due to this "bug", so count me in for the ones that would like to see this fixed :) 
@BeneSim agree with you. For me, the absence of this feature is a showstopper, because all my API is focused on interfaces.
@acelot @BeneSim fine... just started looking into this. Not yet any estimates can be given, but it's on my focus now.

Fix patch:
diff --git a/juniper/CHANGELOG.md b/juniper/CHANGELOG.md
index 4b08aa5e7..2741f3504 100644
--- a/juniper/CHANGELOG.md
+++ b/juniper/CHANGELOG.md
@@ -1,6 +1,6 @@
 # master
 
-- No changes yet
+- Allow spreading interface fragments on unions and other interfaces ([#965](https://github.com/graphql-rust/juniper/pull/965), [#798](https://github.com/graphql-rust/juniper/issues/798))
 
 # [[0.15.7] 2021-07-08](https://github.com/graphql-rust/juniper/releases/tag/juniper-v0.15.7)
 
diff --git a/juniper/src/types/async_await.rs b/juniper/src/types/async_await.rs
index ca253c1a5..218627711 100644
--- a/juniper/src/types/async_await.rs
+++ b/juniper/src/types/async_await.rs
@@ -307,7 +307,9 @@ where
 
                 let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info);
                 let type_name = instance.type_name(info);
-                if fragment.type_condition.item == concrete_type_name
+                if executor
+                    .schema()
+                    .is_named_subtype(&concrete_type_name, &fragment.type_condition.item)
                     || Some(fragment.type_condition.item) == type_name
                 {
                     let sub_result = instance
@@ -351,11 +353,14 @@ where
                 if let Some(ref type_condition) = fragment.type_condition {
                     // Check whether the type matches the type condition.
                     let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info);
-                    if type_condition.item == concrete_type_name {
+                    if executor
+                        .schema()
+                        .is_named_subtype(&concrete_type_name, &type_condition.item)
+                    {
                         let sub_result = instance
                             .resolve_into_type_async(
                                 info,
-                                type_condition.item,
+                                &concrete_type_name,
                                 Some(&fragment.selection_set[..]),
                                 &sub_exec,
                             )
diff --git a/juniper/src/types/base.rs b/juniper/src/types/base.rs
index 2102925ab..1b351a215 100644
--- a/juniper/src/types/base.rs
+++ b/juniper/src/types/base.rs
@@ -516,7 +516,9 @@ where
 
                 let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info);
                 let type_name = instance.type_name(info);
-                if fragment.type_condition.item == concrete_type_name
+                if executor
+                    .schema()
+                    .is_named_subtype(&concrete_type_name, &fragment.type_condition.item)
                     || Some(fragment.type_condition.item) == type_name
                 {
                     let sub_result = instance.resolve_into_type(
@@ -552,10 +554,13 @@ where
                 if let Some(ref type_condition) = fragment.type_condition {
                     // Check whether the type matches the type condition.
                     let concrete_type_name = instance.concrete_type_name(sub_exec.context(), info);
-                    if type_condition.item == concrete_type_name {
+                    if executor
+                        .schema()
+                        .is_named_subtype(&concrete_type_name, &type_condition.item)
+                    {
                         let sub_result = instance.resolve_into_type(
                             info,
-                            type_condition.item,
+                            &concrete_type_name,
                             Some(&fragment.selection_set[..]),
                             &sub_exec,
                         );


Test patch:
diff --git a/integration_tests/juniper_tests/src/issue_798.rs b/integration_tests/juniper_tests/src/issue_798.rs
new file mode 100644
index 000000000..f3ec4d700
--- /dev/null
+++ b/integration_tests/juniper_tests/src/issue_798.rs
@@ -0,0 +1,195 @@
+use juniper::{
+    graphql_interface, graphql_object, graphql_value, EmptyMutation, EmptySubscription,
+    GraphQLObject, GraphQLUnion, RootNode, Variables,
+};
+
+#[graphql_interface(for = [Human, Droid])]
+trait Character {
+    fn id(&self) -> &str;
+}
+
+#[derive(GraphQLObject)]
+#[graphql(impl = CharacterValue)]
+struct Human {
+    id: String,
+    home_planet: String,
+}
+
+#[graphql_interface]
+impl Character for Human {
+    fn id(&self) -> &str {
+        &self.id
+    }
+}
+
+#[derive(GraphQLObject)]
+#[graphql(impl = CharacterValue)]
+struct Droid {
+    id: String,
+    primary_function: String,
+}
+
+#[graphql_interface]
+impl Character for Droid {
+    fn id(&self) -> &str {
+        &self.id
+    }
+}
+
+#[derive(GraphQLUnion)]
+enum FieldResult {
+    Human(Human),
+    Droid(Droid),
+}
+
+#[derive(Clone, Copy)]
+enum Query {
+    Human,
+    Droid,
+}
+
+#[graphql_object]
+impl Query {
+    fn field(&self) -> FieldResult {
+        match self {
+            Self::Human => FieldResult::Human(Human {
+                id: "human-32".to_owned(),
+                home_planet: "earth".to_owned(),
+            }),
+            Self::Droid => FieldResult::Droid(Droid {
+                id: "droid-99".to_owned(),
+                primary_function: "run".to_owned(),
+            }),
+        }
+    }
+}
+
+type Schema = RootNode<'static, Query, EmptyMutation<()>, EmptySubscription<()>>;
+
+#[tokio::test]
+async fn test_interface_inline_fragment_on_union() {
+    let query = r#"
+        query Query {
+            field {
+                __typename
+                ... on Character {
+                    id
+                }
+                ... on Human {
+                    homePlanet
+                }
+                ... on Droid {
+                    primaryFunction
+                }
+            }
+        }
+    "#;
+
+    let (res, errors) = juniper::execute(
+        query,
+        None,
+        &Schema::new(Query::Human, EmptyMutation::new(), EmptySubscription::new()),
+        &Variables::new(),
+        &(),
+    )
+    .await
+    .unwrap();
+
+    assert_eq!(errors.len(), 0);
+    assert_eq!(
+        res,
+        graphql_value!({
+            "field": {
+                "__typename": "Human",
+                "id": "human-32",
+                "homePlanet": "earth",
+            },
+        }),
+    );
+
+    let (res, errors) = juniper::execute_sync(
+        query,
+        None,
+        &Schema::new(Query::Droid, EmptyMutation::new(), EmptySubscription::new()),
+        &Variables::new(),
+        &(),
+    )
+    .unwrap();
+
+    assert_eq!(errors.len(), 0);
+    assert_eq!(
+        res,
+        graphql_value!({
+            "field": {
+                "__typename": "Droid",
+                "id": "droid-99",
+                "primaryFunction": "run",
+            },
+        }),
+    );
+}
+
+#[tokio::test]
+async fn test_interface_fragment_on_union() {
+    let query = r#"
+        query Query {
+            field {
+                __typename
+                ... CharacterFragment
+                ... on Human {
+                    homePlanet
+                }
+                ... on Droid {
+                    primaryFunction
+                }
+            }
+        }
+
+        fragment CharacterFragment on Character {
+            id
+        }
+    "#;
+
+    let (res, errors) = juniper::execute(
+        query,
+        None,
+        &Schema::new(Query::Human, EmptyMutation::new(), EmptySubscription::new()),
+        &Variables::new(),
+        &(),
+    )
+    .await
+    .unwrap();
+
+    assert_eq!(errors.len(), 0);
+    assert_eq!(
+        res,
+        graphql_value!({
+            "field": {
+                "__typename": "Human",
+                "id": "human-32",
+                "homePlanet": "earth",
+            },
+        }),
+    );
+
+    let (res, errors) = juniper::execute_sync(
+        query,
+        None,
+        &Schema::new(Query::Droid, EmptyMutation::new(), EmptySubscription::new()),
+        &Variables::new(),
+        &(),
+    )
+    .unwrap();
+
+    assert_eq!(errors.len(), 0);
+    assert_eq!(
+        res,
+        graphql_value!({
+            "field": {
+                "__typename": "Droid",
+                "id": "droid-99",
+                "primaryFunction": "run",
+            },
+        }),
+    );
+}
diff --git a/integration_tests/juniper_tests/src/lib.rs b/integration_tests/juniper_tests/src/lib.rs
index 5592c1464..f94c4d728 100644
--- a/integration_tests/juniper_tests/src/lib.rs
+++ b/integration_tests/juniper_tests/src/lib.rs
@@ -19,6 +19,8 @@ mod issue_407;
 #[cfg(test)]
 mod issue_500;
 #[cfg(test)]
+mod issue_798;
+#[cfg(test)]
 mod issue_914;
 #[cfg(test)]
 mod issue_922;
diff --git a/juniper/src/tests/subscriptions.rs b/juniper/src/tests/subscriptions.rs
index ec0d3b669..66d118fcb 100644
--- a/juniper/src/tests/subscriptions.rs
+++ b/juniper/src/tests/subscriptions.rs
@@ -33,7 +33,7 @@ type Schema =
     RootNode<'static, MyQuery, EmptyMutation<MyContext>, MySubscription, DefaultScalarValue>;
 
 fn run<O>(f: impl std::future::Future<Output = O>) -> O {
-    let mut rt = tokio::runtime::Runtime::new().unwrap();
+    let rt = tokio::runtime::Runtime::new().unwrap();
 
     rt.block_on(f)
 }