apollographql / federation

🌐  Build and scale a single data graph across multiple services with Apollo's federation gateway.
https://apollographql.com/docs/federation/
Other
667 stars 254 forks source link

Gateway V2 throws "non nullable" error for extended type fields that exist #2187

Open heramb-sawant opened 2 years ago

heramb-sawant commented 2 years ago

Summary:

Hey guys 👋 !

I have been trying to upgrade our Apollo Gateway version to V2 through our gateways here at Neo. Most of the changes were backwards compatible as expected. However, we ran into one bug related to resolving extended type fields under an interface list.

Ive set up a super simplified demo of the problem here https://github.com/heramb-sawant/federation-demo.

Setup:

Service 1 - Product Graph

  extend type Query {
    queryInterfaceType(first: Int = 5): [QueryInterfaceType!]!
  }

  interface QueryInterfaceType {
    id: String!
    typeUnderInterface: [TypeUnderInterface!]!
  }

  type FirstInterface implements QueryInterfaceType {
    id: String!
    typeUnderInterface: [TypeUnderInterface!]!
  }

  type SecondInterface implements QueryInterfaceType {
    id: String!
    typeUnderInterface: [TypeUnderInterface!]!
  }

  type TypeUnderInterface @key(fields: "externalRefKey") {
    externalRefKey: String!
    nonKeyExternalRef: Float!
  }

Service 2 - Inventory Graph

  extend type TypeUnderInterface @key(fields: "externalRefKey") {
    externalRefKey: String! @external
    nonKeyExternalRef: Float! @external
    fieldFromOtherService: String! @requires(fields: "nonKeyExternalRef")
  }

Reproduction:

When querying this graph, if you don't query the interface types directly everything works as expected.

query Query {
  queryInterfaceType {
    id
    typeUnderInterface {
      externalRefKey
      nonKeyExternalRef
      fieldFromOtherService
    }
  }
}

When querying types specific on the interface, we get the "non nullable" error

query Query {
  queryInterfaceType {
    ... on FirstInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
        fieldFromOtherService
      }
    }
    ... on SecondInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
        fieldFromOtherService
      }
    }
  }
}

The expectation is to be able to run both queries with no problems.

The demo is running on "@apollo/gateway": "2.1.3" but if you downgrade to our current version "@apollo/gateway": "0.51.0". This demo works as expected.

Please let me know if there are any other details I could provide!

pcmanus commented 2 years ago

I looked a bit, and I think the issue in the services implementations, where it uses the wrong package/method. Namely, the services are using the buildFederatedSchema from @apollo/federation, but that package/method is defunct and not updated to federation 2. What should be used instead is the buildSubgraphSchema method from the @apollo/subgraph package (which is otherwise used the same way than buildFederatedSchema, so it's just a matter of updating the package imports and method name).

From a quick test, once that change is made, the example work as expected.

To be fair, I definitively see how you can easily miss that dependency change. We do have a deprecation warning for buildFederationSchema, and the npm page of @apollo/federation has some notes about the need to move to @apollo/subgraph (move that, btw, started in federation 0.x in fact), but those are probably fairly easy to miss.

That said, I'm not completely sure how to make that a lot better. At a minimum, we could probably push a new version of @apollo/federation that provides a clearer deprecation warning if buildFederatedSchema or buildSubgraphSchema is used (the current warning only tell you to stop using buildFederatedSchema but really, it should tell you to switch to @apollo/subgraph for those method altogether). I suppose we could even push a new version where the use of those method not just warn but hard error to force everyone to move to @apollo/subgraph, but that's maybe a bit harsh for current users of federation 1.

heramb-sawant commented 2 years ago

Hey sorry it took so long to hear back from me! I added the change you mentioned and still had no luck with the demo I posted :/. I could just be doing something wrong!

I tried to add some commits so its easy to follow!

d0275556953fc030337b4aa91857a184a8368924 - Updated to @apollo/subgraph and the nullable error still exists cf489b915a766cc908f826320512558abb4be87e - Downgraded @apollo/gateway and the nullable error is resolved

pcmanus commented 1 year ago

I just tested your repository, checked out d0275556953fc030337b4aa91857a184a8368924, started services and gateway, and ran the query:

query Query {
  queryInterfaceType {
    ... on FirstInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
        fieldFromOtherService
      }
    }
    ... on SecondInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
      }
    }
  }
}

and I get back:

{
  "data": {
    "queryInterfaceType": [
      {
        "id": "queryInterfaceTypeOne",
        "typeUnderInterface": [
          {
            "externalRefKey": "typeUnderInterfaceOne",
            "nonKeyExternalRef": 1,
            "fieldFromOtherService": "fieldFromOtherService"
          },
          {
            "externalRefKey": "typeUnderInterfaceTwo",
            "nonKeyExternalRef": 2,
            "fieldFromOtherService": "fieldFromOtherService"
          }
        ]
      },
      {
        "id": "queryInterfaceTypeTwo",
        "typeUnderInterface": [
          {
            "externalRefKey": "typeUnderInterfaceOne",
            "nonKeyExternalRef": 1
          },
          {
            "externalRefKey": "typeUnderInterfaceTwo",
            "nonKeyExternalRef": 2
          }
        ]
      }
    ]
  }
}

which I believe is the correct response, no "non-nullable" errors in there.

So I'm not sure what to tell you other than "I can't reproduce". I do not get the error you seem to with @apollo/gateway at 2.1.3 (and I've double checked this was the version in use with npm explain). My only suggestion would be, when testing the version with updated @apollo/subgraph, to make sure all the dependencies are properly updated locally by running npm update (or even removing and node_modules and package-lock.json and re-running npm i).

heramb-sawant commented 1 year ago

Hey, looks like your query is missing a field on it! The query wont throw a non null error without the field. However, I don't believe adding it in should break the query! If you check out the other commit (cf489b915a766cc908f826320512558abb4be87e) it works as intended!

query Query {
  queryInterfaceType {
    ... on FirstInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
        fieldFromOtherService
      }
    }
    ... on SecondInterface {
      id
      typeUnderInterface {
        externalRefKey
        nonKeyExternalRef
        fieldFromOtherService <----- this one
      }
    }
  }
}