gqlgen
gqlgen copied to clipboard
non-nullable @external field that are not part of @key fieldset blows up federation
What happened?
If schema specifies non-nullable @external
fields that are not part of the @key
field set, entity resolution will blow up trying to coerce nil
to a target type even if they are not part of the query. Those fields will only be correctly coerced if end user select a field that references those external fields using @requires
field set. It appears that the current logic assumes that those fields will always be populated in entity representation.
Example use case from https://github.com/apollographql/apollo-federation-subgraph-compatibility. Given a type definition
extend type User @key(fields: "email") {
averageProductsCreatedPerYear: Int @requires(fields: "totalProductsCreated yearsOfEmployment")
email: ID! @external
name: String @override(from: "users")
totalProductsCreated: Int @external
yearsOfEmployment: Int! @external
}
@requires
directive allows to reference external fields that are necessary for calculating its value. Those @external
fields will be provided in the entity representation ONLY if field referencing them is requested (i.e. router will only include totalProductsCreated
and yearsOfEmployment
in the user entity representation if averageProductsCreatedPerYear
is in query selection set). Attempting to run a query without @requires
field blows up the execution, e.g.
# blows up
query userEntity ($representations: [_Any!]!) {
_entities(representations: $representations) {
...on User { email name }
}
}
# works
query userEntity ($representations: [_Any!]!) {
_entities(representations: $representations) {
...on User { email name averageProductsCreatedPerYear }
}
}
What did you expect?
Query is executed successfully even if those fields are referenced by the selection set (i.e. they are not populated).
Minimal graphql.schema and models to reproduce
Start example reviews
server from https://github.com/99designs/gqlgen/tree/v0.17.16/_examples/federation
Execute following query
query userEntity ($representations: [_Any!]!) {
_entities(representations: $representations) {
...on User { id }
}
}
Variables
{
"representations": [
{
"__typename": "User",
"id": "123"
}
]
}
Execution blows up at https://github.com/99designs/gqlgen/blob/v0.17.16/_examples/federation/reviews/graph/generated/federation.go#L124 with interface conversion: interface {} is nil, not map[string]interface {}
versions
-
go run github.com/99designs/gqlgen version
v0.17.16 -
go version
go1.19 darwin/arm64
Can you please see if #2371 addressed your issue? If not, can you submit a PR?
👋 referenced PR fixed directive definitions which is great but does not solve the underlying issue.
Issue here is that codegen assumes that ALL @external
fields will always be populated in the _entities
query. Existing code works fine if external fields are nullable but blows up on external non-nullable @required
fields (which are not part of the @key
fieldset), i.e. code from the example repro
case "findUserByID":
// correct, resolves ID from the @key(fields: "id")
id0, err := ec.unmarshalNID2string(ctx, rep["id"])
if err != nil {
return fmt.Errorf(`unmarshalling param 0 for findUserByID(): %w`, err)
}
entity, err := ec.resolvers.Entity().FindUserByID(ctx, id0)
if err != nil {
return fmt.Errorf(`resolving Entity "User": %w`, err)
}
// BELOW WILL BLOW UP
// -> while host is not nullable, _entities query may not specify it in the representation as query may not be referencing this field
entity.Host.ID, err = ec.unmarshalNString2string(ctx, rep["host"].(map[string]interface{})["id"])
if err != nil {
return err
}
// THIS WILL ALSO BLOW UP due to the same reason as above
entity.Email, err = ec.unmarshalNString2string(ctx, rep["email"])
if err != nil {
return err
}
list[idx[i]] = entity
return nil
}
While this use case may not be clear from the example app schema as gateway would never attempt to resolve just the User.id
from this subgraph. If we slightly modify the schema
extend type User @key(fields: "id") {
id: ID! @external
host: EmailHost! @external
email: String! @external
reviews: [Review] @requires(fields: "host {id} email")
# new local field resolved by this subgraph
foo: String
}
If we now get a query asking for just User.foo
, the query will blow up due to the reason stated above. You will only be able to resolve User.foo
if query also request User.reviews
data.
IMHO the simplest solution would be to check whether the representation map contains entries for those non-nullable fields and only then attempt to populate corresponding fields.
As for the PR -> sorry but won't be able to work on this anytime soon.
Ok, I burned through my OSS hours for this week, but would fast-track reviewing any PR for this. Thanks for providing more information!
Hi people, other than the issue discussed in this issue, there's another issue with requires
as well, that I have mentioned in this issue: https://github.com/99designs/gqlgen/issues/2559
@shawnhugginsjr @jclyons52 @lleadbet Hey, if one of you has a chance to work on fixing our outstanding issue with Apollo Federation support, I would love a PR!
Facing similar issue where getting "interface conversion: interface {} is nil, not map[string]interface {}"
error in subgraph's federation go when that subgraph has dependency on other subgraph entity's field.
According to https://www.apollographql.com/docs/federation/v1/federation-spec/#scalar-_fieldset the selection can only be a fieldset
Is this different in v2 of the federation spec?
fixed in https://github.com/99designs/gqlgen/pull/2884