Skip to content

Commit

Permalink
Merge pull request #1197 from raptros/fix-entity-fetcher-exception-ha…
Browse files Browse the repository at this point in the history 
…ndling

Make fed resolver always apply configured exception handler
  • Loading branch information
@srinivasankavitha
srinivasankavitha authored Aug 23, 2022
2 parents e74038f + b9009d2 commit b7ca0d3
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 49 deletions.
40 changes: 18 additions & 22 deletions ...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import reactor.core.publisher.Mono
import java.lang.reflect.InvocationTargetException
import java.util.*
import java.util.concurrent.CompletableFuture
import java.util.concurrent.CompletionException
import java.util.concurrent.CompletionStage

@DgsComponent
Expand Down Expand Up @@ -124,31 +125,26 @@ open class DefaultDgsFederationResolver() :
.filter { tryResult -> tryResult.isFailure }
.map { tryResult -> tryResult.throwable }
.flatMap { e ->
if (e is InvocationTargetException && e.targetException != null) {
if (dgsExceptionHandler.isPresent) {
val res = dgsExceptionHandler.get().handleException(
DataFetcherExceptionHandlerParameters
.newExceptionParameters()
.dataFetchingEnvironment(env)
.exception(e.targetException)
.build()
)
res.join().errors.asSequence()
} else {
sequenceOf(
TypedGraphQLError.newInternalErrorBuilder()
.message(
"%s: %s",
e.targetException::class.java.name,
e.targetException.message
)
.path(ResultPath.parse("/_entities")).build()
)
}
// extract exception from known wrapper types
val exception = when {
e is InvocationTargetException && e.targetException != null -> e.targetException
e is CompletionException && e.cause != null -> e.cause!!
else -> e
}
// handle the exception (using the custom handler if present)
if (dgsExceptionHandler.isPresent) {
val res = dgsExceptionHandler.get().handleException(
DataFetcherExceptionHandlerParameters
.newExceptionParameters()
.dataFetchingEnvironment(env)
.exception(exception)
.build()
)
res.join().errors.asSequence()
} else {
sequenceOf(
TypedGraphQLError.newInternalErrorBuilder()
.message("%s: %s", e::class.java.name, e.message)
.message("%s: %s", exception::class.java.name, exception.message)
.path(ResultPath.parse("/_entities"))
.build()
)
Expand Down
70 changes: 43 additions & 27 deletions graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import com.netflix.graphql.dgs.internal.DgsSchemaProvider
import com.netflix.graphql.dgs.internal.EntityFetcherRegistry
import com.netflix.graphql.dgs.internal.method.MethodDataFetcherFactory
import graphql.execution.DataFetcherExceptionHandler
import graphql.execution.DataFetcherExceptionHandlerParameters
import graphql.execution.DataFetcherExceptionHandlerResult
import graphql.execution.DataFetcherResult
import graphql.execution.ExecutionStepInfo
import graphql.execution.ResultPath
Expand Down Expand Up @@ -164,8 +166,7 @@ class DefaultDgsFederationResolverTest {
@Test
fun `Will throw a MissingDgsEntityFetcherException, if unable to find the DGSEntityFetcher for the given __typename`() {
val arguments = mapOf<String, Any>(_Entity.argumentName to listOf(mapOf("__typename" to "Movie")))
val dataFetchingEnvironment =
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
val dataFetchingEnvironment = constructDFE(arguments)
val result =
(
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
Expand All @@ -182,8 +183,7 @@ class DefaultDgsFederationResolverTest {
@Test
fun `Will throw a MissingFederatedQueryArgument, if they query is missing the __typename`() {
val arguments = mapOf<String, Any>(_Entity.argumentName to listOf(mapOf("something" to "Else")))
val dataFetchingEnvironment =
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
val dataFetchingEnvironment = constructDFE(arguments)
val result =
(
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
Expand Down Expand Up @@ -286,10 +286,7 @@ class DefaultDgsFederationResolverTest {
val arguments = mapOf<String, Any>(
_Entity.argumentName to listOf(mapOf("__typename" to "Movie", "movieId" to "1"))
)
val dataFetchingEnvironment =
DgsDataFetchingEnvironment(
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
)
val dataFetchingEnvironment = constructDFE(arguments)

val result =
(
Expand Down Expand Up @@ -326,10 +323,7 @@ class DefaultDgsFederationResolverTest {
_Entity.argumentName to
listOf(mapOf("__typename" to "Movie", "movieId" to "1"))
)
val dataFetchingEnvironment =
DgsDataFetchingEnvironment(
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
)
val dataFetchingEnvironment = constructDFE(arguments)

val result =
(
Expand Down Expand Up @@ -397,19 +391,19 @@ class DefaultDgsFederationResolverTest {
_Entity.argumentName to listOf(mapOf("__typename" to "Movie", "movieId" to "invalid"))
)

val executionStepInfo = ExecutionStepInfo.newExecutionStepInfo().path(ResultPath.parse("/_entities")).type(
GraphQLList.list(
GraphQLUnionType.newUnionType().name("Entity")
.possibleTypes(GraphQLObjectType.newObject().name("Movie").build()).build()
)
).build()
val dataFetchingEnvironment = DgsDataFetchingEnvironment(
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments)
.executionStepInfo(executionStepInfo).build()
)
val dataFetchingEnvironment = constructDFE(arguments)

val customExceptionHandler = object : DataFetcherExceptionHandler {
var invocationCounter = 0
override fun handleException(handlerParameters: DataFetcherExceptionHandlerParameters?): CompletableFuture<DataFetcherExceptionHandlerResult> {
invocationCounter++
return dgsExceptionHandler.handleException(handlerParameters)
}
}

val result =
(
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler)).entitiesFetcher()
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(customExceptionHandler)).entitiesFetcher()
.get(dataFetchingEnvironment) as CompletableFuture<DataFetcherResult<List<*>>>
)

Expand All @@ -423,6 +417,7 @@ class DefaultDgsFederationResolverTest {
.contains("com.netflix.graphql.dgs.exceptions.DgsInvalidInputArgumentException: Invalid input argument exception")
}
)
assertThat(customExceptionHandler.invocationCounter).isEqualTo(1)
}

@Test
Expand Down Expand Up @@ -452,8 +447,7 @@ class DefaultDgsFederationResolverTest {
)

)
val dataFetchingEnvironment =
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
val dataFetchingEnvironment = constructDFE(arguments)

val result =
DefaultDgsFederationResolver(entityFetcherRegistry, Optional.of(dgsExceptionHandler))
Expand All @@ -468,8 +462,7 @@ class DefaultDgsFederationResolverTest {
@Test
fun `Invoking an Entity Fetcher missing an argument`() {
val arguments = mapOf<String, Any>(_Entity.argumentName to listOf(mapOf("__typename" to "Movie")))
val dataFetchingEnvironment =
DataFetchingEnvironmentImpl.newDataFetchingEnvironment().arguments(arguments).build()
val dataFetchingEnvironment = constructDFE(arguments)

val result =
(
Expand All @@ -492,5 +485,28 @@ class DefaultDgsFederationResolverTest {
return schemaGenerator.makeExecutableSchema(registry, RuntimeWiring.newRuntimeWiring().build())
}

private fun constructDFE(arguments: Map<String, Any>): DgsDataFetchingEnvironment {
val executionStepInfo = ExecutionStepInfo
.newExecutionStepInfo()
.path(ResultPath.parse("/_entities"))
.type(
GraphQLList.list(
GraphQLUnionType
.newUnionType()
.name("Entity")
.possibleTypes(GraphQLObjectType.newObject().name("Movie").build())
.build()
)
)
.build()
return DgsDataFetchingEnvironment(
DataFetchingEnvironmentImpl
.newDataFetchingEnvironment()
.arguments(arguments)
.executionStepInfo(executionStepInfo)
.build()
)
}

data class Movie(val movieId: String = "", val title: String = "")
}

0 comments on commit b7ca0d3

Please sign in to comment.