Merge pull request #2040 from Netflix/fix/fix-controller-advice

Fix setup of exception handling to make @ControllerAdvice work

graphql-dgs-spring-graphql-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/ControllerExceptionHandler.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright 2024 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.example.datafetcher;
+
+import graphql.GraphQLError;
+import org.springframework.graphql.data.method.annotation.GraphQlExceptionHandler;
+import org.springframework.graphql.execution.ErrorType;
+import org.springframework.web.bind.annotation.ControllerAdvice;
+
+@ControllerAdvice
+public class ControllerExceptionHandler {
+    @GraphQlExceptionHandler
+    public GraphQLError handle(IllegalArgumentException ex) {
+        return GraphQLError.newError().errorType(ErrorType.BAD_REQUEST).message("Successful error handling").build();
+    }
+}

graphql-dgs-spring-graphql-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/WithControllerAdvice.java

@@ -0,0 +1,28 @@
+/*
+ * Copyright 2024 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.example.datafetcher;
+
+import com.netflix.graphql.dgs.DgsComponent;
+import com.netflix.graphql.dgs.DgsQuery;
+
+@DgsComponent
+public class WithControllerAdvice {
+    @DgsQuery
+    public String withControllerAdvice() {
+        throw new IllegalArgumentException("Testing controller advice");
+    }
+}

graphql-dgs-spring-graphql-example-java/src/main/resources/schema/schema.graphqls

@@ -1,6 +1,7 @@
 extend type Query {
     greetings(name: String): String
     greetingFromBatchLoader(person: Person): String
+    withControllerAdvice: String
 }
 
 input Person {

graphql-dgs-spring-graphql/src/main/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/DgsSpringGraphQLAutoConfiguration.kt

@@ -32,8 +32,6 @@ import com.netflix.graphql.dgs.springgraphql.SpringGraphQLDgsQueryExecutor
 import com.netflix.graphql.dgs.springgraphql.SpringGraphQLDgsReactiveQueryExecutor
 import com.netflix.graphql.dgs.springgraphql.webflux.DgsWebFluxGraphQLInterceptor
 import com.netflix.graphql.dgs.springgraphql.webmvc.DgsWebMvcGraphQLInterceptor
-import graphql.execution.AsyncExecutionStrategy
-import graphql.execution.AsyncSerialExecutionStrategy
 import graphql.execution.DataFetcherExceptionHandler
 import graphql.execution.ExecutionStrategy
 import graphql.execution.preparsed.PreparsedDocumentProvider
@@ -119,24 +117,25 @@ open class DgsSpringGraphQLAutoConfiguration {
         @Qualifier("query") providedQueryExecutionStrategy: Optional<ExecutionStrategy>,
         @Qualifier("mutation") providedMutationExecutionStrategy: Optional<ExecutionStrategy>,
         dataFetcherExceptionHandler: DataFetcherExceptionHandler,
-    ): GraphQlSourceBuilderCustomizer {
-        val queryExecutionStrategy =
-            providedQueryExecutionStrategy.orElse(AsyncExecutionStrategy(dataFetcherExceptionHandler))
-        val mutationExecutionStrategy =
-            providedMutationExecutionStrategy.orElse(AsyncSerialExecutionStrategy(dataFetcherExceptionHandler))
-
-        return GraphQlSourceBuilderCustomizer { builder ->
+    ): GraphQlSourceBuilderCustomizer =
+        GraphQlSourceBuilderCustomizer { builder ->
             builder.configureGraphQl { graphQlBuilder ->
                 if (preparsedDocumentProvider.isPresent) {
                     graphQlBuilder
                         .preparsedDocumentProvider(preparsedDocumentProvider.get())
                 }
-                graphQlBuilder
-                    .queryExecutionStrategy(queryExecutionStrategy)
-                    .mutationExecutionStrategy(mutationExecutionStrategy)
+
+                if (providedQueryExecutionStrategy.isPresent) {
+                    graphQlBuilder
+                        .queryExecutionStrategy(providedQueryExecutionStrategy.get())
+                }
+
+                if (providedMutationExecutionStrategy.isPresent) {
+                    graphQlBuilder
+                        .mutationExecutionStrategy(providedMutationExecutionStrategy.get())
+                }
             }
         }
-    }
 
     @Bean
     @ConditionalOnProperty(

graphql-dgs-spring-graphql/src/test/kotlin/com/netflix/graphql/dgs/springgraphql/autoconfig/ControllerAdviceTest.kt

@@ -0,0 +1,88 @@
+/*
+ * Copyright 2024 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.springgraphql.autoconfig
+
+import com.netflix.graphql.dgs.DgsComponent
+import com.netflix.graphql.dgs.DgsQuery
+import com.netflix.graphql.dgs.DgsQueryExecutor
+import com.netflix.graphql.dgs.autoconfig.DgsAutoConfiguration
+import graphql.GraphQLError
+import org.assertj.core.api.Assertions.assertThat
+import org.intellij.lang.annotations.Language
+import org.junit.jupiter.api.Test
+import org.springframework.beans.factory.annotation.Autowired
+import org.springframework.boot.autoconfigure.graphql.GraphQlAutoConfiguration
+import org.springframework.boot.test.context.SpringBootTest
+import org.springframework.boot.test.context.TestConfiguration
+import org.springframework.graphql.data.method.annotation.GraphQlExceptionHandler
+import org.springframework.graphql.execution.ErrorType
+import org.springframework.web.bind.annotation.ControllerAdvice
+
+@SpringBootTest(
+    classes = [
+        DgsSpringGraphQLAutoConfiguration::class,
+        DgsAutoConfiguration::class,
+        DgsSpringGraphQLSourceAutoConfiguration::class,
+        GraphQlAutoConfiguration::class,
+        ControllerAdviceTest.ControllerAdviceTestConfig::class,
+    ],
+    properties = [
+        "dgs.graphql.schema-locations=classpath:/dgs-spring-graphql-smoke-test.graphqls",
+        "spring.graphql.schema.inspection.enabled=true",
+        "dgs.graphql.schema-wiring-validation-enabled=false",
+    ],
+)
+class ControllerAdviceTest {
+    @Autowired
+    lateinit var queryExecutor: DgsQueryExecutor
+
+    @Test
+    fun testControllerAdvice() {
+        @Language("GraphQL")
+        val query =
+            """
+            query {
+                withControllerAdvice
+            }
+            """.trimIndent()
+
+        val result = queryExecutor.execute(query)
+        assertThat(result.errors).isNotEmpty
+        assertThat(result.errors.first().message).isEqualTo("Successful error handling")
+        assertThat(result.errors.first().errorType).isEqualTo(ErrorType.BAD_REQUEST)
+    }
+
+    @TestConfiguration
+    open class ControllerAdviceTestConfig {
+        @DgsComponent
+        class TestDataFetcher {
+            @DgsQuery
+            fun withControllerAdvice(): Unit = throw IllegalArgumentException("Testing Controller Advice")
+        }
+
+        @ControllerAdvice
+        class TestControllerAdvice {
+            @GraphQlExceptionHandler
+            fun handle(ex: java.lang.IllegalArgumentException?): GraphQLError =
+                GraphQLError
+                    .newError()
+                    .errorType(ErrorType.BAD_REQUEST)
+                    .message("Successful error handling")
+                    .build()
+        }
+    }
+}

graphql-dgs-spring-graphql/src/test/resources/dgs-spring-graphql-smoke-test.graphqls

@@ -5,4 +5,5 @@ type Query {
     unmappedArgument: String
     incorrectNamedArgument(somename: String): String
     mappedArguments(firstParam: String, secondParam: Int): String
+    withControllerAdvice: String
 }