bug: Make path field of GraphQLError nullable

[Kalyan457]

Expected behavior

GraphQLError should be successfully deserialized when the path parameter is null.

Actual behavior

com.jayway.jsonpath.spi.mapper.MappingException: java.lang.IllegalArgumentException: Instantiation of [simple type, class com.netflix.graphql.dgs.client.GraphQLError] value failed for JSON property path due to missing (therefore NULL) value for creator parameter path which is a non-nullable type
 at [Source: UNKNOWN; line: -1, column: -1] (through reference chain: java.util.ArrayList[0]->com.netflix.graphql.dgs.client.GraphQLError["path"])
    at com.jayway.jsonpath.spi.mapper.JacksonMappingProvider.map(JacksonMappingProvider.java:58)
    at com.jayway.jsonpath.internal.JsonContext.convert(JsonContext.java:121)
    at com.jayway.jsonpath.internal.JsonContext.read(JsonContext.java:103)
    at com.netflix.graphql.dgs.client.GraphQLResponse.<init>(GraphQLResponse.kt:52)
    at com.netflix.graphql.dgs.client.GraphQLClients.handleResponse(GraphQLClients.kt:47)
    at com.netflix.graphql.dgs.client.CustomGraphQLClient.executeQuery(CustomGraphQLClient.kt:52)
    at com.netflix.graphql.dgs.client.CustomGraphQLClient.executeQuery(CustomGraphQLClient.kt:28)

Steps to reproduce

1. Make a request by omitting any required argument in a GraphQL request to a GraphQL endpoint using GraphQLClient.executeQuery().
2. The GraphQL server first parses the request and finds that the required argument is not provided in the input and it returns without invoking any resolvers.
3. Then, the server sets the path parameter to null. When this is deserialized as GraphQLError, it throws the above mentioned exception.

{
  "data": null,
  "errors": [
    {
      "path": null,
      "locations": [
        {
          "line": 2,
          "column": 26,
          "sourceName": null
        }
      ],
      "message": "Validation error....is missing required fields...."
    }
  ]
}

It would be ideal if path parameter is nullable

[kilink]

Do you have any more details on what the specific error is or how to reproduce it? I can see that the path can be null, but it appears that when the ExecutionResult is serialized by first calling toSpecification, null values such as path are just not included in the response map at all. Is your request going against a DGS service or some other GraphQL service that returns an explicit null for path?

[Kalyan457]

We use AWS AppSync and it explicitly returns null for path if the received request doesn't obey GraphQL schema.

[kilink]

I see, thanks for the context. I think the gap is here due graphql-java's behavior of simply skipping null fields in the response. e.g., your example would be more like this if serialized by graphql-java:

{
  "errors": [
    {
      "locations": [
        {
          "line": 2,
          "column": 26
        }
      ],
      "message": "Validation error....is missing required fields...."
    }
  ]
}

I have a PR open that doesn't change the fields to nullable, but allows your response to be deserialized nonetheless; switching it to be nullable would have some compatibility concerns, so just allowing it to deserialize is a safer approach. For instance, we would deserialize an explicit null path and a missing path property the same, as an empty list. I am struggling to think of a case where the distinction would matter from the client's perspective. Does that work for you?

[Kalyan457]

Yes. This works for me! Thanks! Do you know in which version this would be released?

[kilink]

@Kalyan457 we should be able to cut a bug fix release this week.

[kilink]

This was released in v8.4.0