router icon indicating copy to clipboard operation
router copied to clipboard

GraphQL error paths are missing with a subgraph is down

Open smyrick opened this issue 1 year ago • 2 comments

Describe the bug If my subgraph is in a nested entity fetch query plan and the subgraph has a service error or is offline, the GraphQL errors reported from Router do not have a path assigned.

To Reproduce Router version 1.35.0

  • Compose a supergraph of two schemas but only start up the server of the first subgraph that has the root resolver
  • Run a query that does a Flatten across a list
  • Notice the query plan for this is [email protected]
    • In older versions of Router this query plan path made into the GraphQL error path which is invalid
  • However in the error block there is not reference to fieldB and it's path even though it is the causing the error from the Router perspective

Expected behavior All GraphQL errors have an associated path if we can process the request and send requests to subgraphs

Output Router 1.35.0 Screenshot 2024-01-24 at 1 42 36 PM

Desktop (please complete the following information):

  • OS: MacOS
  • Version 14.3

Additional context In previous versions of Router, 1.19.0, we actually has an invalid path that included the at @ symbol from query plans (which brought up this spec clarification https://github.com/graphql/graphql-spec/pull/1073)

Router 1.19.0 Screenshot 2024-01-24 at 1 40 58 PM

For Router 1.35.0

Running this graph: https://github.com/apollosolutions/retail-supergraph

Here is the query:

query {
  searchProducts {
    id
    recommendedProducts {
      id
      reviews {
        id
        body
      }
    }
  }
}

Query plan

QueryPlan {
  Sequence {
    Fetch(service: "products") {
      {
        searchProducts {
          __typename
          id
        }
      }
    },
    Flatten(path: "searchProducts.@") {
      Fetch(service: "discovery") {
        {
          ... on Product {
            __typename
            id
          }
        } =>
        {
          ... on Product {
            recommendedProducts {
              __typename
              id
            }
          }
        }
      },
    },
    Flatten(path: "[email protected].@") {
      Fetch(service: "products") {
        {
          ... on Product {
            __typename
            id
          }
        } =>
        {
          ... on Product {
            upc
          }
        }
      },
    },
    Flatten(path: "[email protected].@") {
      Fetch(service: "reviews") {
        {
          ... on Product {
            __typename
            upc
          }
        } =>
        {
          ... on Product {
            reviews {
              id
              body
            }
          }
        }
      },
    },
  },
}

smyrick avatar Jan 25 '24 20:01 smyrick

If we follow the spec strictly then we should have an error for every single searchProducts.1-n.reccomendProducts.1-n.reviews field but that seems excesive. So maybe we default to just returning one error for searchProducts.0.reccomendProducts.0.reviews?

smyrick avatar Jan 25 '24 20:01 smyrick

looking at the code, there are multiple issues. in subgraph_service.rs, in multiple places we convert a FetchError to a graphql error: https://github.com/apollographql/router/blob/dev/apollo-router/src/services/subgraph_service.rs#L711

But in other places we return the FetchError. Which might explain why you get the error path for some errors but not others. And as you said, we should post process the error path to replace the @ and apply to every index in array

Geal avatar Feb 07 '24 15:02 Geal