bug: GraphQLJavaErrorInstrumentation changes error type of DataFetchingException

[hpuac]

Hey folks, I'd like to report a change in behavior that I currently consider a bug. It was introduced in 8.6.0 with this PR: https://github.com/Netflix/dgs-framework/pull/1905.

Please let me know your opinion on this topic 🙂

Expected behavior

When returning a DataFetcherResult containing a GraphQLError I'm expecting my error to be propagated to the client, including my given code and errorType.

Actual behavior

The GraphQLJavaErrorInstrumentation is creating a new TypedGraphQLError and overriding the errorType. The code is kept but the errorType is overridden. See https://github.com/Netflix/dgs-framework/blob/f9382bff5c8a5f9af70087c160c7f82d632c1b1f/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/GraphQLJavaErrorInstrumentation.kt#L53-L57

Steps to reproduce

Return a DataFetcherResult that contains a custom GraphQLError. Let me know in case you would like to see a demo project.

[kailyak]

Hi @hpuac, thank you for the feedback and report.

To make sure I'm understanding: the use case is, in the case of a customized GraphQLError arising from data fetching, the customized fields (in this case the errorType) should be returned to the client without any modifications? And that this behavior is no longer occurring with the linked change? If you have a demo project available that would be great to see, otherwise we will investigate further and try and reproduce.

[kailyak]

Hello, I replicated the behavior via this datafetcher, let me know if it's a different case:

@DgsQuery
fun dataFetcherResult(): DataFetcherResult<String> {
  return DataFetcherResult.newResult<String>().error(
    GraphQLError.newError()
    .message("A DataFetchingException error occurred while datafetching").errorType(graphql.ErrorType.DataFetchingException).build()
  ).build()
}

In the linked PR, the goal was to make error reporting more consistent in the framework and it closed a gap that was there previously, which is where your bug report comes in. We do want to keep the more consistent error handling. In this case, to keep the error info communicated back to the client, two options you could try is adding the ErrorType in the extensions or in the error message.