Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Query including fragments return null #1290

Closed
xsoufiane opened this issue Jun 21, 2022 · 13 comments · Fixed by #1296
Closed

Query including fragments return null #1290

xsoufiane opened this issue Jun 21, 2022 · 13 comments · Fixed by #1296
Assignees

Comments

@xsoufiane
Copy link

Describe the bug
We have a query using fragments

{
  ...ShoppingListFragmentTest
}

fragment ShoppingListFragmentTest on ShoppingListQueryInterface {
shoppingLists {
  results {
    id
  }
  total
}

the router returns data:null for it, although the sub-service returns a valid non-null response. We are assuming that there is an issue in assembling the response. The query plan for the query is:

query plan is: QueryPlan {
   usage_reporting: UsageReporting { 
 	stats_report_key: "# -\nfragment ShoppingListFragmentTest on ShoppingListQueryInterface{shoppingLists{results{id}total {...ShoppingListFragmentTest}", 
        referenced_fields_by_type: {
           "ShoppingListQueryInterface": ReferencedFieldsForType { field_names: ["shoppingLists"], is_interface: true }, 
           "ShoppingListQueryResult": ReferencedFieldsForType { field_names: ["results", "total"], is_interface: false },            
           "ShoppingList": ReferencedFieldsForType { field_names: ["id"], is_interface: false }
        } 
    },
     root: Fetch(FetchNode { service_name: "sphere", requires: [], variable_usages: [], operation: "{shoppingLists{results{id}total}}", operation_name: None, operation_kind: Query }) }

Expected behavior
router should be able to assemble the response and return non-null data.

@xsoufiane
Copy link
Author

for info we are using router-v0.9.3

@xsoufiane
Copy link
Author

for info we are using router-v0.9.3

same issue in v0.9.5

@Geal
Copy link
Contributor

Geal commented Jun 22, 2022

thanks! That looks like #1286, no?

@Geal Geal self-assigned this Jun 22, 2022
@Geal Geal removed the triage label Jun 22, 2022
@Geal
Copy link
Contributor

Geal commented Jun 22, 2022

could you give more information on the types and the data returned by the subgraph?

@Geal
Copy link
Contributor

Geal commented Jun 22, 2022

@xsoufiane does this look like the schema you are working with?
https://github.com/apollographql/router/pull/1296/files#diff-ca21c6c2d8f5eacbc8fd08524c17571552b1ec649c2911b908b03b94b2c7c99fR4578-R4603
Maybe with the object field set to non null?

@xsoufiane
Copy link
Author

@xsoufiane does this look like the schema you are working with?
https://github.com/apollographql/router/pull/1296/files#diff-ca21c6c2d8f5eacbc8fd08524c17571552b1ec649c2911b908b03b94b2c7c99fR4578-R4603
Maybe with the object field set to non null?

thank you @Geal yes it's the same 🤩

@Geal
Copy link
Contributor

Geal commented Jun 22, 2022

Great, then I have a good idea of the cause. May I ask why you go through a fragment then an interface on the query object? This feels a bit weird to me so there's probably a use case I don't know about

@xsoufiane
Copy link
Author

xsoufiane commented Jun 22, 2022

Great, then I have a good idea of the cause. May I ask why you go through a fragment then an interface on the query object? This feels a bit weird to me so there's probably a use case I don't know about

also not sure why, those interfaces are implemented by other types, and the fragments maybe to not repeat the same code. Probably @yanns can tell more about this.

thanks! That looks like #1286, no?

seems related 🤔

@yanns
Copy link
Contributor

yanns commented Jun 23, 2022

Yes we use interfaces on query so that one can re-use the same queries on the top field, or on another sub-field:

{
  orders { // all orders
     ....
  }
}
{
  store(key: "store-DE") {
    orders { // orders in the store in Germany
       ....
    }
  }
}

@Mithras
Copy link

Mithras commented Jun 24, 2022

While this is an open issue. Is there a reason router silently returns null instead of propagating an exception or something? I have another case where router returns null (not related to fragments) and the lack of details hurts my soul

@Geal
Copy link
Contributor

Geal commented Jun 24, 2022

@yanns thanks, that's interesting :)
@Mithras yes, that's not great for now, we're planning to add error messages along with the nullability rules. Could you post another issue for your other case? I'd be happy to take a look

@Geal
Copy link
Contributor

Geal commented Jun 24, 2022

@Mithras making a note of the error message for nullified fields in #1304

@xsoufiane
Copy link
Author

thank you @Geal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants