Why does GraphQLErrorFromExceptionHandler remove information from incoming errors?

[Sam-Kruglov]

I found there this code:

protected List<GraphQLError> filterGraphQLErrors(List<GraphQLError> errors) {
  return errors.stream().map(this::transform).flatMap(Collection::stream).collect(Collectors.toList());
}
private Collection<GraphQLError> transform(GraphQLError error) {
  return extractException(error).map(this::transform)
      .orElse(singletonList(new GenericGraphQLError(error.getMessage())));
}
private Optional<Throwable> extractException(GraphQLError error) {
...
}

This is really confusing to me, as why would I spend effort building up GraphQLError if it's just going to be discarted here? Here is some context:

I created my own DataFetcherExceptionHandler in order to catch some exceptions like IllegalArgumentException, MyServiceException and add an error code inside extensions.errorCode. (btw, I could not find a better way of integrating it other than creating a Spring Bean of new AsyncExecutionStrategy(dataFetcherExceptionHandler) )

Then, I ran the code and I see that I only have the contents of GenericGraphQLError returned to the client. I can see that this class (GraphQLErrorFromExceptionHandler) takes MyCustomGraphQLError, tries to convert it into a Throwable and then back to GraphQLError but without considering any fields I populated (except for the message).

I see, that DefaultGraphQLErrorHandler doesn't do that. I added this property graphql.servlet.exception-handlers-enabled: true because I wanted to use @ExceptionHandler annotations, but that didn't work for me (#309).

Anyway, this is just to understand, why would you want to strip so much information from the errors?

[Sam-Kruglov]

After some digging, I figured out why it may actually be fine.

1. As per make fields protected in ExceptionWhileDataFetching graphql-java/graphql-java#1717, one is not supposed to mess with data fetching error handling.
2. I can provide an @ExceptionHandler that returns GraphQLError and in that case GraphQLErrorFromExceptionHandler will simply pass it as is, so that approach is preferred.

Will be waiting for the fix though #318