Merge pull request #732 from Netflix/feature/federatred-missingfedera…

…tedquery-exception Missing the typename in a federated query should return a BadRequest
Netflix · Nov 12, 2021 · 2abfb08 · 2abfb08
2 parents 02ee569 + 04a130b
commit 2abfb08
Show file tree

Hide file tree

Showing 5 changed files with 49 additions and 17 deletions.
diff --git a/.../src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt b/.../src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt
@@ -40,13 +40,16 @@ class DefaultDataFetcherExceptionHandler : DataFetcherExceptionHandler {
                 .message("%s: %s", exception::class.java.name, exception.message)
                 .path(handlerParameters.path).build()
         } else if (exception is DgsEntityNotFoundException) {
-            TypedGraphQLError.newNotFoundBuilder().message("%s: %s", exception::class.java.name, exception.message)
+            TypedGraphQLError.newNotFoundBuilder()
+                .message("%s: %s", exception::class.java.name, exception.message)
                 .path(handlerParameters.path).build()
         } else if (exception is DgsBadRequestException) {
-            TypedGraphQLError.newBadRequestBuilder().message("%s: %s", exception::class.java.name, exception.message)
+            TypedGraphQLError.newBadRequestBuilder()
+                .message("%s: %s", exception::class.java.name, exception.message)
                 .path(handlerParameters.path).build()
         } else {
-            TypedGraphQLError.newInternalErrorBuilder().message("%s: %s", exception::class.java.name, exception.message)
+            TypedGraphQLError.newInternalErrorBuilder()
+                .message("%s: %s", exception::class.java.name, exception.message)
                 .path(handlerParameters.path).build()
         }
 

diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsBadRequestException.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsBadRequestException.kt
@@ -16,4 +16,4 @@
 
 package com.netflix.graphql.dgs.exceptions
 
-class DgsBadRequestException(override val message: String = "Malformed or incorrect request") : RuntimeException(message)
+open class DgsBadRequestException(override val message: String = "Malformed or incorrect request") : RuntimeException(message)
diff --git a/...l-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/MissingFederatedQueryArgument.kt b/...l-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/MissingFederatedQueryArgument.kt
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2021 Netflix, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.netflix.graphql.dgs.exceptions
+
+class MissingFederatedQueryArgument(vararg fields: String) : DgsBadRequestException("The federated query is missing field(s) ${fields.joinToString()}")
diff --git a/...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt b/...ql-dgs/src/main/kotlin/com/netflix/graphql/dgs/federation/DefaultDgsFederationResolver.kt
@@ -22,6 +22,7 @@ import com.netflix.graphql.dgs.DgsDataFetchingEnvironment
 import com.netflix.graphql.dgs.DgsFederationResolver
 import com.netflix.graphql.dgs.exceptions.InvalidDgsEntityFetcher
 import com.netflix.graphql.dgs.exceptions.MissingDgsEntityFetcherException
+import com.netflix.graphql.dgs.exceptions.MissingFederatedQueryArgument
 import com.netflix.graphql.dgs.internal.DgsSchemaProvider
 import com.netflix.graphql.types.errors.TypedGraphQLError
 import graphql.execution.DataFetcherExceptionHandler
@@ -78,7 +79,7 @@ open class DefaultDgsFederationResolver() :
             .map { values ->
                 Try.tryCall {
                     val typename = values["__typename"]
-                        ?: throw RuntimeException("__typename missing from arguments in federated query")
+                        ?: throw MissingFederatedQueryArgument("__typename")
                     val fetcher = dgsSchemaProvider.entityFetchers[typename]
                         ?: throw MissingDgsEntityFetcherException(typename.toString())
 
@@ -108,38 +109,46 @@ open class DefaultDgsFederationResolver() :
             }
 
         return CompletableFuture.allOf(*resultList.toTypedArray()).thenApply {
+            val trySequence = resultList.asSequence().map { it.join() }
             DataFetcherResult.newResult<List<Any?>>()
                 .data(
-                    resultList.asSequence()
-                        .map { cf -> cf.join() }
+                    trySequence
                         .map { tryResult -> tryResult.orElse(null) }
                         .flatMap { r -> if (r is Collection<*>) r.asSequence() else sequenceOf(r) }
                         .toList()
                 )
                 .errors(
-                    resultList.asSequence()
-                        .map { cf -> cf.join() }
+                    trySequence
                         .filter { tryResult -> tryResult.isFailure }
                         .map { tryResult -> tryResult.throwable }
                         .flatMap { e ->
                             if (e is InvocationTargetException && e.targetException != null) {
                                 if (dgsExceptionHandler.isPresent) {
-                                    val res = dgsExceptionHandler.get().onException(
-                                        DataFetcherExceptionHandlerParameters.newExceptionParameters()
-                                            .dataFetchingEnvironment(env).exception(e.targetException).build()
+                                    val res = dgsExceptionHandler.get().handleException(
+                                        DataFetcherExceptionHandlerParameters
+                                            .newExceptionParameters()
+                                            .dataFetchingEnvironment(env)
+                                            .exception(e.targetException)
+                                            .build()
                                     )
-                                    res.errors.asSequence()
+                                    res.join().errors.asSequence()
                                 } else {
                                     sequenceOf(
                                         TypedGraphQLError.newInternalErrorBuilder()
-                                            .message("%s: %s", e.targetException::class.java.name, e.targetException.message)
+                                            .message(
+                                                "%s: %s",
+                                                e.targetException::class.java.name,
+                                                e.targetException.message
+                                            )
                                             .path(ResultPath.parse("/_entities")).build()
                                     )
                                 }
                             } else {
                                 sequenceOf(
-                                    TypedGraphQLError.newInternalErrorBuilder().message("%s: %s", e::class.java.name, e.message)
-                                        .path(ResultPath.parse("/_entities")).build()
+                                    TypedGraphQLError.newInternalErrorBuilder()
+                                        .message("%s: %s", e::class.java.name, e.message)
+                                        .path(ResultPath.parse("/_entities"))
+                                        .build()
                                 )
                             }
                         }

diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsFederationResolverTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DgsFederationResolverTest.kt
@@ -142,7 +142,8 @@ class DgsFederationResolverTest {
         assertThat(result).isNotNull
         assertThat(result.get().data.size).isEqualTo(1)
         assertThat(result.get().errors.size).isEqualTo(1)
-        assertThat(result.get().errors[0].message).contains("RuntimeException")
+        assertThat(result.get().errors.first().message)
+            .endsWith("MissingFederatedQueryArgument: The federated query is missing field(s) __typename")
     }
 
     @Test