router

router copied to clipboard

if there's a null field where a non null was expected, an error message should be generated with the path to the field.

cf https://spec.graphql.org/June2018/#sec-Errors

question: if it nullifies the parent field, and that one is also non nullable, should we generate an error message for the parent too?

This section specifically refers to bubbling: https://spec.graphql.org/October2021/#sel-HAPHRPTEBNBS1_I.

One question comes to mind about if there are multiple steps of bubbling? We might want to try and emulate this behavior in the graphql-js to see how it behaves?

To investigate this I created an Apollo Server based testbed with the following definition:

const typeDefs = gql`
  type Post {
    id: ID!
    title: String!
    authorId: ID!
  }

  type Author {
    id: ID!
    name: String!
    departmentId: ID!
    posts: [Post!]!
  }

  type Department {
    id: ID!
    name: String!
    authors: [Author!]!
  }

  type Query {
    authors: [Author!]!
    departments: [Department!]!
  }
`;

I then modified my resolver so that it deliberately returned null for the title of one of the posts and used this query to trigger errors:

query ExampleQuery {
  departments {
    authors {
      posts {
        title
      }
      name
    }
    name
  }
}

With this in place, I captured the error returned by Apollo Server:

{"errors":[{"message":"Cannot return null for non-nullable field Post.title.","locations":[{"line":5,"column":9}],"path":["departments",0,"authors",0,"posts",0,"title"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Cannot return null for non-nullable field Post.title.","    at completeValue (execute.js:594:13)","    at executeField (execute.js:489:19)","    at executeFields (execute.js:413:20)","    at completeObjectValue (execute.js:914:10)","    at completeValue (execute.js:635:12)","    at completeValue (execute.js:584:23)","    at execute.js:696:25","    at Function.from (<anonymous>)","    at completeListValue (execute.js:676:34)","    at completeValue (execute.js:607:12)"]}}}],"data":null}

I then federated my server and performed the same query through the router. First time I forgot to turn off subgraph error redaction and got:

{"data":null,"errors":[{"message":"Subgraph errors redacted","locations":[],"path":null}]}

:) I ran it again and got a response which was very similar to the response I got when querying the subgraph direct:

{"data":null,"errors":[{"message":"Cannot return null for non-nullable field Post.title.","locations":[{"line":1,"column":56}],"path":["departments",0,"authors",0,"posts",0,"title"],"extensions":{"code":"INTERNAL_SERVER_ERROR","exception":{"stacktrace":["Error: Cannot return null for non-nullable field Post.title.","    at completeValue (execute.js:594:13)","    at executeField (execute.js:489:19)","    at executeFields (execute.js:413:20)","    at completeObjectValue (execute.js:914:10)","    at completeValue (execute.js:635:12)","    at completeValue (execute.js:584:23)","    at execute.js:696:25","    at Function.from (<anonymous>)","    at completeListValue (execute.js:676:34)","    at completeValue (execute.js:607:12)"]}}}]}

A difference in ordering, but everything apart from the location details is the same. I checked the location details and I'm not sure why the router is reporting line: 1 column: 56. Apollo Server is correct, the query error is at line: 5, column: 9. That's a problem, but otherwise, the details we get from the router are the same as we would get from Apollo Server as a standalone subgraph.

So: regarding the original questions:

1. @Geal question: if it nullifies the parent field, and that one is also non nullable, should we generate an error message for the parent too?

It seems like the Apollo Server implementation doesn't. If it's desirable to be consistent, then we should continue to be consistent.

2. @abernix I emulated it with ApolloServer rather than graph-js and I'm assuming that's ok.

Could you guys take a look at how I investigated this and tell me if I made a mistake or if I haven't really tested what we wanted to test? I'll have a little look and see if I can figure out what is going wrong with the location details and fix that if I can.

I figured out why the location is different. The router does some transformation on the input query before passing it to the subgraph. The subgraph receives:

            b"{\"query\":\"query ExampleQuery__posts__0{departments{authors{posts{title}name}name}}\",\"operationName\":\"ExampleQuery__posts__0\"}",

You'll note that line: 1, column: 56 is the correct location (from the subgraph's point of view) of where the error occurs. That's a tricky one to unpick since the router would need a reverse mapping of the query transform in order to translate 1:59 back into 5:9. I wonder what the gateway does? More thinking: This is a difficult one to resolve because in a federated world each subgraph is only receiving the part of the query which is relative to it's contribution, so the error is always going to be in a sub-query of the original query.

I poked around at the error structure in js land, and theres some information we're not retrieving, which may sometimes contain things such as "path" and "relative offset" of errors when planning queries for example. It's worth having a look at the js errors thrown, and see if we can retrieve this info

@garypen if you want to explore nullability and the router's behaviour, it will be easier to add unit tests like this one: https://github.com/apollographql/router/blob/main/apollo-router/src/spec/query.rs#L1549-L1562

@Geal question: if it nullifies the parent field, and that one is also non nullable, should we generate an error message for the parent too?

I think we should, because it is easy to traverse a serie of non hull entities and end up with an entirely null response without knowing why.

I would not be too worried about showing the exact line and column, instead the path in the JS object should be the correct one.

I then federated my server and performed the same query through the router.

Can you elaborate how you split entities between subgraphs? I'm getting nothing but { data: null } when subgraph doesn't return an entity for which we need a non-null field.

Here is an example that's using Gateway: https://github.com/apollographql/router/issues/1308#issuecomment-1169661203 Router does the same (returns nothing without any errors).

It's up to you if you want to add/improve error reporting for these cases because normally nobody should ever have not-nullable fields in Federated entities as they can not be enforced. I mean if the issue is here only because I asked for it, you can close it. I'm going to use graphql-inspector to enforce nullability on all fields in Federated entities instead.

I then federated my server and performed the same query through the router.

Can you elaborate how you split entities between subgraphs? I'm getting nothing but { data: null } when subgraph doesn't return an entity for which we need a non-null field.

I can't because my example only "federated" a single subgraph.