diff --git a/graphql-dgs-example-java/build.gradle.kts b/graphql-dgs-example-java/build.gradle.kts index b9b655aa5..1253db924 100644 --- a/graphql-dgs-example-java/build.gradle.kts +++ b/graphql-dgs-example-java/build.gradle.kts @@ -25,9 +25,11 @@ dependencies { // please review the Spring Boot Docs in the link below for additional information. // https://docs.spring.io/spring-boot/docs/current/reference/html/production-ready-features.html#production-ready-metrics-export-simple implementation("org.springframework.boot:spring-boot-starter-actuator") + // Adding the DGS Micrometer integration that provides timers for gql.* // For additional information go to the link below: // https://netflix.github.io/dgs/advanced/instrumentation/ + implementation(project(":graphql-dgs-spring-boot-micrometer")) implementation("com.github.ben-manes.caffeine:caffeine") testImplementation(project(":graphql-dgs-client")) diff --git a/graphql-dgs-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/MyInstrumentation.java b/graphql-dgs-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/MyInstrumentation.java index 072984520..4b4c7db30 100644 --- a/graphql-dgs-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/MyInstrumentation.java +++ b/graphql-dgs-example-java/src/main/java/com/netflix/graphql/dgs/example/datafetcher/MyInstrumentation.java @@ -16,30 +16,25 @@ package com.netflix.graphql.dgs.example.datafetcher; -import com.netflix.graphql.dgs.mvc.DgsRestController; +import com.netflix.graphql.dgs.internal.DgsExecutionResult; import graphql.ExecutionResult; -import graphql.ExecutionResultImpl; import graphql.execution.instrumentation.SimpleInstrumentation; import graphql.execution.instrumentation.parameters.InstrumentationExecutionParameters; +import org.springframework.http.HttpHeaders; import org.springframework.stereotype.Component; -import java.util.HashMap; -import java.util.Map; import java.util.concurrent.CompletableFuture; @Component public class MyInstrumentation extends SimpleInstrumentation { @Override public CompletableFuture instrumentExecutionResult(ExecutionResult executionResult, InstrumentationExecutionParameters parameters) { - HashMap extensions = new HashMap<>(); - if(executionResult.getExtensions() != null) { - extensions.putAll(executionResult.getExtensions()); - } + HttpHeaders responseHeaders = new HttpHeaders(); + responseHeaders.add("myHeader", "hello"); - Map responseHeaders = new HashMap<>(); - responseHeaders.put("myHeader", "hello"); - extensions.put(DgsRestController.DGS_RESPONSE_HEADERS_KEY, responseHeaders); - - return super.instrumentExecutionResult(new ExecutionResultImpl(executionResult.getData(), executionResult.getErrors(), extensions), parameters); + return super.instrumentExecutionResult(new DgsExecutionResult( + executionResult, + responseHeaders + ), parameters); } } diff --git a/graphql-dgs-example-java/src/main/resources/application.yml b/graphql-dgs-example-java/src/main/resources/application.yml index 5558589aa..db8118336 100644 --- a/graphql-dgs-example-java/src/main/resources/application.yml +++ b/graphql-dgs-example-java/src/main/resources/application.yml @@ -2,6 +2,13 @@ spring: application: name: dgs-example +management: + endpoints: + web: + exposure: + include: + - metrics + logging: level: com.netflix.graphql.dgs.example: DEBUG \ No newline at end of file diff --git a/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt b/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt index 66e8f31a0..21fef6d82 100644 --- a/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt +++ b/graphql-dgs-spring-boot-micrometer/src/main/kotlin/com/netflix/graphql/dgs/metrics/micrometer/DgsGraphQLMetricsInstrumentation.kt @@ -6,6 +6,7 @@ import com.netflix.graphql.dgs.metrics.DgsMetrics.GqlMetric import com.netflix.graphql.dgs.metrics.DgsMetrics.GqlTag import com.netflix.graphql.dgs.metrics.micrometer.tagging.DgsGraphQLMetricsTagsProvider import com.netflix.graphql.dgs.metrics.micrometer.utils.QuerySignatureRepository +import com.netflix.graphql.types.errors.ErrorType import graphql.ExecutionInput import graphql.ExecutionResult import graphql.GraphQLError @@ -344,11 +345,11 @@ class DgsGraphQLMetricsInstrumentation( when (error) { is ValidationError -> { errorPath = error.queryPath ?: emptyList() - errorType = errorType(error) + errorType = ErrorType.BAD_REQUEST.name } is InvalidSyntaxError -> { errorPath = emptyList() - errorType = errorType(error) + errorType = ErrorType.BAD_REQUEST.name } else -> { errorPath = error.path ?: emptyList() @@ -368,10 +369,6 @@ class DgsGraphQLMetricsInstrumentation( return dedupeErrorPaths.values } - private fun errorType(error: T): String { - return error.errorType?.toString() ?: TagUtils.TAG_VALUE_UNKNOWN - } - private fun errorTypeExtension(error: T): String { return extension(error, "errorType", TagUtils.TAG_VALUE_UNKNOWN) } diff --git a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt index 6193a810b..e16343c47 100644 --- a/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt +++ b/graphql-dgs-spring-boot-micrometer/src/test/kotlin/com/netflix/graphql/dgs/metrics/micrometer/MicrometerServletSmokeTest.kt @@ -27,11 +27,13 @@ import com.netflix.graphql.dgs.DgsTypeDefinitionRegistry import com.netflix.graphql.dgs.InputArgument import com.netflix.graphql.dgs.exceptions.DefaultDataFetcherExceptionHandler import com.netflix.graphql.dgs.exceptions.DgsBadRequestException +import com.netflix.graphql.dgs.exceptions.DgsException import com.netflix.graphql.dgs.metrics.micrometer.tagging.DgsContextualTagCustomizer import com.netflix.graphql.dgs.metrics.micrometer.tagging.DgsExecutionTagCustomizer import com.netflix.graphql.dgs.metrics.micrometer.tagging.DgsFieldFetchTagCustomizer import com.netflix.graphql.dgs.metrics.micrometer.utils.QuerySignatureRepository import com.netflix.graphql.types.errors.ErrorDetail +import com.netflix.graphql.types.errors.ErrorType import com.netflix.graphql.types.errors.TypedGraphQLError import graphql.GraphQLError import graphql.execution.DataFetcherExceptionHandler @@ -388,7 +390,7 @@ class MicrometerServletSmokeTest { .and("gql.query.complexity", "none") .and("gql.query.sig.hash", "none") .and("gql.errorDetail", "none") - .and("gql.errorCode", "InvalidSyntax") + .and("gql.errorCode", "BAD_REQUEST") .and("gql.path", "[]") .and("outcome", "failure") ) @@ -448,7 +450,7 @@ class MicrometerServletSmokeTest { .and("gql.query.complexity", "none") .and("gql.query.sig.hash", "none") .and("gql.errorDetail", "none") - .and("gql.errorCode", "ValidationError") + .and("gql.errorCode", "BAD_REQUEST") .and("gql.path", "[hello]") .and("outcome", "failure") ) @@ -479,8 +481,12 @@ class MicrometerServletSmokeTest { """ |{ | "errors":[ - | {"message":"java.lang.IllegalStateException: Exception triggered.","locations":[], - | "path":["triggerInternalFailure"],"extensions":{"errorType":"INTERNAL"}} + | { + | "message":"java.lang.IllegalStateException: Exception triggered.", + | "locations": [], + | "path": ["triggerInternalFailure"], + | "extensions": {"errorType":"INTERNAL"} + | } | ], | "data":{"triggerInternalFailure":null} |} @@ -550,7 +556,7 @@ class MicrometerServletSmokeTest { """ |{ | "errors":[ - | {"message":"com.netflix.graphql.dgs.exceptions.DgsBadRequestException: Exception triggered.", + | {"message":"Exception triggered.", | "locations":[],"path":["triggerBadRequestFailure"], | "extensions":{"errorType":"BAD_REQUEST"}} | ], @@ -688,18 +694,12 @@ class MicrometerServletSmokeTest { .andExpect( content().json( """ - |{ - | "errors":[ - | {"message":"java.lang.IllegalStateException: Exception triggered.","locations":[], - | "path":["triggerInternalFailure"],"extensions":{"errorType":"INTERNAL"}}, - | {"message":"com.netflix.graphql.dgs.exceptions.DgsBadRequestException: Exception triggered.","locations":[], - | "path":["triggerBadRequestFailure"],"extensions":{"errorType":"BAD_REQUEST"}}, - | {"message":"Exception triggered.","locations":[], - | "path":["triggerCustomFailure"], - | "extensions":{"errorType":"UNAVAILABLE","errorDetail":"ENHANCE_YOUR_CALM"}} - | ], - | "data":{"triggerInternalFailure":null,"triggerBadRequestFailure":null,"triggerCustomFailure":null} - |} + | {"errors":[ + | {"message":"java.lang.IllegalStateException: Exception triggered.","locations":[],"path":["triggerInternalFailure"],"extensions":{"errorType":"INTERNAL"}}, + | {"message":"Exception triggered.","locations":[],"path":["triggerBadRequestFailure"],"extensions":{"class":"com.netflix.graphql.dgs.exceptions.DgsBadRequestException","errorType":"BAD_REQUEST"}}, + | {"message":"Exception triggered.","locations":[],"path":["triggerCustomFailure"],"extensions":{"errorType":"UNAVAILABLE","errorDetail":"ENHANCE_YOUR_CALM"}} + | ], + | "data":{"triggerInternalFailure":null,"triggerBadRequestFailure":null,"triggerCustomFailure":null}} """.trimMargin(), false ) @@ -969,5 +969,5 @@ class MicrometerServletSmokeTest { } } - class CustomException(message: String?) : java.lang.IllegalStateException(message) + class CustomException(message: String) : DgsException(message = message, errorType = ErrorType.INTERNAL) } diff --git a/graphql-dgs-spring-webflux-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/webflux/handlers/DefaultDgsWebfluxHttpHandler.kt b/graphql-dgs-spring-webflux-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/webflux/handlers/DefaultDgsWebfluxHttpHandler.kt index 2af9cae3a..e43181e97 100644 --- a/graphql-dgs-spring-webflux-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/webflux/handlers/DefaultDgsWebfluxHttpHandler.kt +++ b/graphql-dgs-spring-webflux-autoconfigure/src/main/kotlin/com/netflix/graphql/dgs/webflux/handlers/DefaultDgsWebfluxHttpHandler.kt @@ -20,6 +20,7 @@ import com.fasterxml.jackson.core.JsonParseException import com.fasterxml.jackson.databind.ObjectMapper import com.fasterxml.jackson.databind.exc.MismatchedInputException import com.fasterxml.jackson.module.kotlin.readValue +import com.netflix.graphql.dgs.internal.DgsExecutionResult import com.netflix.graphql.dgs.reactive.DgsReactiveQueryExecutor import graphql.ExecutionResult import org.slf4j.Logger @@ -74,9 +75,15 @@ class DefaultDgsWebfluxHttpHandler( } return executionResult.flatMap { result -> + val dgsExecutionResult = when (result) { + is DgsExecutionResult -> result + else -> DgsExecutionResult(result) + } + ServerResponse - .ok() - .bodyValue(result.toSpecification()) + .status(dgsExecutionResult.status) + .headers { it.addAll(dgsExecutionResult.headers) } + .bodyValue(dgsExecutionResult.toSpecification()) }.onErrorResume { ex -> when (ex) { is JsonParseException -> diff --git a/graphql-dgs-spring-webflux-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webflux/MalformedQueryContentTest.kt b/graphql-dgs-spring-webflux-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webflux/MalformedQueryContentTest.kt index 978d7f3b0..e948f1569 100644 --- a/graphql-dgs-spring-webflux-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webflux/MalformedQueryContentTest.kt +++ b/graphql-dgs-spring-webflux-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webflux/MalformedQueryContentTest.kt @@ -77,16 +77,19 @@ class MalformedQueryContentTest { .bodyValue("{ }") .exchange() .expectStatus() - .isOk + .isBadRequest .expectBody() .json( """ { "errors":[ { - "message": "The query is null or empty.", + "message": "GraphQL operations must contain a non-empty `query`.", "locations":[], - "extensions":{"errorType":"BAD_REQUEST"} + "extensions": { + "errorType":"BAD_REQUEST", + "class":"com.netflix.graphql.dgs.exceptions.DgsBadRequestException" + } } ] } diff --git a/graphql-dgs-spring-webmvc-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webmvc/MalformedQueryContentTest.kt b/graphql-dgs-spring-webmvc-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webmvc/MalformedQueryContentTest.kt index e9acdc6f7..a41026533 100644 --- a/graphql-dgs-spring-webmvc-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webmvc/MalformedQueryContentTest.kt +++ b/graphql-dgs-spring-webmvc-autoconfigure/src/test/kotlin/com/netflix/graphql/dgs/webmvc/MalformedQueryContentTest.kt @@ -89,18 +89,21 @@ class MalformedQueryContentTest { .content("{ }") mvc.perform(uriBuilder) - .andExpect(status().isOk) + .andExpect(status().isBadRequest) .andExpect( content().json( """ { "errors":[ - { - "message": "The query is null or empty.", - "locations": [], - "extensions": {"errorType":"BAD_REQUEST"} - } - ] + { + "message":"GraphQL operations must contain a non-empty `query`.", + "locations":[], + "extensions":{ + "class":"com.netflix.graphql.dgs.exceptions.DgsBadRequestException", + "errorType":"BAD_REQUEST" + } + } + ] } """.trimIndent() ) diff --git a/graphql-dgs-spring-webmvc/src/main/kotlin/com/netflix/graphql/dgs/mvc/DgsRestController.kt b/graphql-dgs-spring-webmvc/src/main/kotlin/com/netflix/graphql/dgs/mvc/DgsRestController.kt index 562251e6b..13e149009 100644 --- a/graphql-dgs-spring-webmvc/src/main/kotlin/com/netflix/graphql/dgs/mvc/DgsRestController.kt +++ b/graphql-dgs-spring-webmvc/src/main/kotlin/com/netflix/graphql/dgs/mvc/DgsRestController.kt @@ -18,15 +18,13 @@ package com.netflix.graphql.dgs.mvc import com.fasterxml.jackson.core.JsonParseException import com.fasterxml.jackson.databind.ObjectMapper -import com.fasterxml.jackson.databind.exc.InvalidDefinitionException import com.fasterxml.jackson.databind.exc.MismatchedInputException import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper import com.fasterxml.jackson.module.kotlin.readValue import com.netflix.graphql.dgs.DgsQueryExecutor +import com.netflix.graphql.dgs.internal.DgsExecutionResult import com.netflix.graphql.dgs.internal.utils.MultipartVariableMapper import com.netflix.graphql.dgs.internal.utils.TimeTracer -import graphql.ExecutionResultImpl -import graphql.GraphqlErrorBuilder import graphql.execution.reactive.SubscriptionPublisher import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -75,7 +73,9 @@ open class DgsRestController( ) { companion object { - const val DGS_RESPONSE_HEADERS_KEY = "dgs-response-headers" + // defined in here and DgsExecutionResult, for backwards compatibility. + // keep these two variables synced. + const val DGS_RESPONSE_HEADERS_KEY = DgsExecutionResult.DGS_RESPONSE_HEADERS_KEY private val logger: Logger = LoggerFactory.getLogger(DgsRestController::class.java) } @@ -226,40 +226,9 @@ open class DgsRestController( .body("Trying to execute subscription on /graphql. Use /subscriptions instead!") } - val responseHeaders = if (executionResult.extensions?.containsKey(DGS_RESPONSE_HEADERS_KEY) == true) { - val dgsResponseHeaders = executionResult.extensions[DGS_RESPONSE_HEADERS_KEY] - val responseHeaders = HttpHeaders() - if (dgsResponseHeaders is Map<*, *>) { - dgsResponseHeaders.forEach { - if (it.key != null) { - responseHeaders.add(it.key.toString(), it.value?.toString()) - } - } - } else { - logger.warn( - "{} must be of type java.util.Map, but was {}", - DGS_RESPONSE_HEADERS_KEY, - dgsResponseHeaders?.javaClass?.name - ) - } - - executionResult.extensions.remove(DGS_RESPONSE_HEADERS_KEY) - responseHeaders - } else HttpHeaders() - - val result = try { - TimeTracer.logTime( - { mapper.writeValueAsBytes(executionResult.toSpecification()) }, - logger, - "Serialized JSON result in {}ms" - ) - } catch (ex: InvalidDefinitionException) { - val errorMessage = "Error serializing response: ${ex.message}" - val errorResponse = ExecutionResultImpl(GraphqlErrorBuilder.newError().message(errorMessage).build()) - logger.error(errorMessage, ex) - mapper.writeValueAsBytes(errorResponse.toSpecification()) + return when (executionResult) { + is DgsExecutionResult -> executionResult.toSpringResponse() + else -> DgsExecutionResult(executionResult).toSpringResponse() } - - return ResponseEntity(result, responseHeaders, HttpStatus.OK) } } diff --git a/graphql-dgs-spring-webmvc/src/test/kotlin/com/netflix/graphql/dgs/mvc/DgsRestControllerTest.kt b/graphql-dgs-spring-webmvc/src/test/kotlin/com/netflix/graphql/dgs/mvc/DgsRestControllerTest.kt index 69f884b72..859e19dca 100644 --- a/graphql-dgs-spring-webmvc/src/test/kotlin/com/netflix/graphql/dgs/mvc/DgsRestControllerTest.kt +++ b/graphql-dgs-spring-webmvc/src/test/kotlin/com/netflix/graphql/dgs/mvc/DgsRestControllerTest.kt @@ -203,9 +203,22 @@ class DgsRestControllerTest { val result = DgsRestController(dgsQueryExecutor).graphql(requestBody.toByteArray(), null, null, null, httpHeaders, webRequest) assertThat(result.headers["myHeader"]).contains("hello") + + assertThat(result.body).isInstanceOfSatisfying(ByteArray::class.java) { body -> + val mapper = jacksonObjectMapper() + val (_, _, extensions) = mapper.readValue(body, GraphQLResponse::class.java) + assertThat(extensions).isNull() + } } - data class GraphQLResponse(val data: Map = emptyMap(), val errors: List = emptyList()) + data class GraphQLResponse( + val data: Map = emptyMap(), + val errors: List = emptyList(), + + // should not be on the response; only here so we can + // test that it does not exist. + val extensions: Map? + ) @JsonIgnoreProperties(ignoreUnknown = true) data class GraphQLError(val message: String) diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt index eb1dcf91b..fea1431ec 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandler.kt @@ -46,23 +46,17 @@ class DefaultDataFetcherExceptionHandler : DataFetcherExceptionHandler { "Exception while executing data fetcher for ${handlerParameters.path}: ${exception.message}", exception ) - val graphqlError = if (springSecurityAvailable && isSpringSecurityAccessException(exception)) { - TypedGraphQLError.newPermissionDeniedBuilder() - .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) - .path(handlerParameters.path).build() - } else if (exception is DgsBadRequestException) { - 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) - .path(handlerParameters.path).build() + + val graphqlError = when (exception) { + is DgsException -> exception.toGraphQlError(handlerParameters.path) + else -> when { + springSecurityAvailable && isSpringSecurityAccessException(exception) -> TypedGraphQLError.newPermissionDeniedBuilder() + else -> TypedGraphQLError.newInternalErrorBuilder() + }.message("%s: %s", exception::class.java.name, exception.message) + .path(handlerParameters.path) + .build() } + return DataFetcherExceptionHandlerResult.newResult() .error(graphqlError) .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 index 55dd4a9f8..beb20c3c9 100644 --- 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 @@ -1,5 +1,5 @@ /* - * Copyright 2021 Netflix, Inc. + * Copyright 2022 Netflix, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,5 +15,11 @@ */ package com.netflix.graphql.dgs.exceptions +import com.netflix.graphql.types.errors.ErrorType -open class DgsBadRequestException(override val message: String = "Malformed or incorrect request") : RuntimeException(message) +open class DgsBadRequestException(override val message: String = "Malformed or incorrect request") : + DgsException(message = message, errorType = ErrorType.BAD_REQUEST) { + companion object { + val NULL_OR_EMPTY_QUERY_EXCEPTION = DgsBadRequestException("GraphQL operations must contain a non-empty `query`.") + } +} diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsEntityNotFoundException.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsEntityNotFoundException.kt index ec3e82467..d4fa36425 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsEntityNotFoundException.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsEntityNotFoundException.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 Netflix, Inc. + * Copyright 2022 Netflix, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,4 +16,7 @@ package com.netflix.graphql.dgs.exceptions -class DgsEntityNotFoundException(override val message: String = "Requested entity not found") : RuntimeException(message) +import com.netflix.graphql.types.errors.ErrorType + +class DgsEntityNotFoundException(override val message: String = "Requested entity not found") : + DgsException(message = message, errorType = ErrorType.NOT_FOUND) diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsException.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsException.kt new file mode 100644 index 000000000..ed9a33dbf --- /dev/null +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsException.kt @@ -0,0 +1,45 @@ +/* + * Copyright 2022 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 + +import com.netflix.graphql.types.errors.ErrorType +import com.netflix.graphql.types.errors.TypedGraphQLError +import graphql.execution.ResultPath + +abstract class DgsException( + override val message: String, + override val cause: Exception? = null, + val errorType: ErrorType = ErrorType.UNKNOWN +) : RuntimeException(message, cause) { + companion object { + const val EXTENSION_CLASS_KEY = "class" + } + + fun toGraphQlError(path: ResultPath? = null): TypedGraphQLError { + return TypedGraphQLError + .newBuilder() + .apply { + if (path != null) { + path(path) + } + } + .errorType(errorType) + .message(message) + .extensions(mapOf(EXTENSION_CLASS_KEY to this::class.java.name)) + .build() + } +} diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsInvalidInputArgumentException.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsInvalidInputArgumentException.kt index ac1155d64..5261841cb 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsInvalidInputArgumentException.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/exceptions/DgsInvalidInputArgumentException.kt @@ -1,5 +1,5 @@ /* - * Copyright 2021 Netflix, Inc. + * Copyright 2022 Netflix, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,4 +16,7 @@ package com.netflix.graphql.dgs.exceptions -class DgsInvalidInputArgumentException(override val message: String, override val cause: Exception? = null) : RuntimeException(message, cause) +import com.netflix.graphql.types.errors.ErrorType + +class DgsInvalidInputArgumentException(override val message: String, override val cause: Exception? = null) : + DgsException(message = message, cause = cause, errorType = ErrorType.BAD_REQUEST) diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/BaseDgsQueryExecutor.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/BaseDgsQueryExecutor.kt index fb134529c..79e6e3811 100644 --- a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/BaseDgsQueryExecutor.kt +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/BaseDgsQueryExecutor.kt @@ -27,9 +27,12 @@ import com.jayway.jsonpath.ParseContext import com.jayway.jsonpath.spi.json.JacksonJsonProvider import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider import com.netflix.graphql.dgs.context.DgsContext -import com.netflix.graphql.types.errors.ErrorType -import com.netflix.graphql.types.errors.TypedGraphQLError -import graphql.* +import com.netflix.graphql.dgs.exceptions.DgsBadRequestException +import graphql.ExecutionInput +import graphql.ExecutionResult +import graphql.ExecutionResultImpl +import graphql.GraphQL +import graphql.GraphQLError import graphql.execution.ExecutionIdProvider import graphql.execution.ExecutionStrategy import graphql.execution.instrumentation.Instrumentation @@ -37,6 +40,7 @@ import graphql.execution.preparsed.PreparsedDocumentProvider import graphql.schema.GraphQLSchema import org.slf4j.Logger import org.slf4j.LoggerFactory +import org.springframework.http.HttpStatus import org.springframework.util.StringUtils import java.util.* import java.util.concurrent.CompletableFuture @@ -72,19 +76,18 @@ object BaseDgsQueryExecutor { idProvider: Optional, preparsedDocumentProvider: PreparsedDocumentProvider? ): CompletableFuture { - var inputVariables = variables ?: Collections.emptyMap() + val inputVariables = variables ?: Collections.emptyMap() if (!StringUtils.hasText(query)) { return CompletableFuture.completedFuture( - ExecutionResultImpl - .newExecutionResult() - .addError( - TypedGraphQLError - .newBadRequestBuilder() - .message("The query is null or empty.") - .errorType(ErrorType.BAD_REQUEST) - .build() - ).build() + DgsExecutionResult( + status = HttpStatus.BAD_REQUEST, + errors = listOf( + DgsBadRequestException + .NULL_OR_EMPTY_QUERY_EXCEPTION + .toGraphQlError() + ) + ) ) } @@ -101,19 +104,19 @@ object BaseDgsQueryExecutor { val dataLoaderRegistry = dataLoaderProvider.buildRegistryWithContextSupplier { dgsContext } - @Suppress("DEPRECATION") - val executionInputBuilder: ExecutionInput.Builder = - ExecutionInput - .newExecutionInput() - .query(query) - .operationName(operationName) - .variables(inputVariables) - .dataLoaderRegistry(dataLoaderRegistry) - .context(dgsContext) - .graphQLContext(dgsContext) - .extensions(extensions.orEmpty()) - return try { + @Suppress("DEPRECATION") + val executionInputBuilder: ExecutionInput.Builder = + ExecutionInput + .newExecutionInput() + .query(query) + .operationName(operationName) + .variables(inputVariables) + .dataLoaderRegistry(dataLoaderRegistry) + .context(dgsContext) + .graphQLContext(dgsContext) + .extensions(extensions.orEmpty()) + graphQL.executeAsync(executionInputBuilder.build()) } catch (e: Exception) { logger.error("Encountered an exception while handling query $query", e) diff --git a/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResult.kt b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResult.kt new file mode 100644 index 000000000..e2177c107 --- /dev/null +++ b/graphql-dgs/src/main/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResult.kt @@ -0,0 +1,136 @@ +/* + * Copyright 2022 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.internal + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.databind.exc.InvalidDefinitionException +import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper +import com.netflix.graphql.dgs.internal.utils.TimeTracer +import graphql.ExecutionResult +import graphql.ExecutionResultImpl +import graphql.GraphQLError +import graphql.GraphqlErrorBuilder +import org.slf4j.Logger +import org.slf4j.LoggerFactory +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpStatus +import org.springframework.http.ResponseEntity + +class DgsExecutionResult @JvmOverloads constructor( + val executionResult: ExecutionResult, + val headers: HttpHeaders = HttpHeaders(), + val status: HttpStatus = HttpStatus.OK +) : ExecutionResult by executionResult { + companion object { + // defined in here and DgsRestController, for backwards compatibility. + // keep these two variables synced. + const val DGS_RESPONSE_HEADERS_KEY = "dgs-response-headers" + private val sentinelObject = Any() + private val logger: Logger = LoggerFactory.getLogger(DgsExecutionResult::class.java) + } + + init { + addExtensionsHeaderKeyToHeader() + } + + constructor( + status: HttpStatus = HttpStatus.OK, + headers: HttpHeaders = HttpHeaders.EMPTY, + errors: List = listOf(), + extensions: Map? = null, + + // By default, assign data as a sentinel object. + // If we were to default to null here, this constructor + // would be unable to discriminate between an intentionally null + // response and one that the user left default. + data: Any? = sentinelObject + ) : this( + headers = headers, + status = status, + executionResult = ExecutionResultImpl + .newExecutionResult() + .errors(errors) + .extensions(extensions) + .apply { + if (data != sentinelObject) { + data(data) + } + } + .build() + ) + + // for backwards compat with https://github.com/Netflix/dgs-framework/pull/1261. + private fun addExtensionsHeaderKeyToHeader() { + if (executionResult.extensions?.containsKey(DGS_RESPONSE_HEADERS_KEY) == true) { + val dgsResponseHeaders = executionResult.extensions[DGS_RESPONSE_HEADERS_KEY] + + if (dgsResponseHeaders is Map<*, *>) { + dgsResponseHeaders.forEach { + if (it.key != null) { + headers.add(it.key.toString(), it.value?.toString()) + } + } + } else { + logger.warn( + "{} must be of type java.util.Map, but was {}", + DGS_RESPONSE_HEADERS_KEY, + dgsResponseHeaders?.javaClass?.name + ) + } + } + } + + fun toSpringResponse( + mapper: ObjectMapper = jacksonObjectMapper() + ): ResponseEntity { + val result = try { + TimeTracer.logTime( + { mapper.writeValueAsBytes(this.toSpecification()) }, + logger, + "Serialized JSON result in {}ms" + ) + } catch (ex: InvalidDefinitionException) { + val errorMessage = "Error serializing response: ${ex.message}" + val errorResponse = ExecutionResultImpl(GraphqlErrorBuilder.newError().message(errorMessage).build()) + logger.error(errorMessage, ex) + mapper.writeValueAsBytes(errorResponse.toSpecification()) + } + + return ResponseEntity( + result, + headers, + status + ) + } + + // overridden for compatibility with https://github.com/Netflix/dgs-framework/pull/1261. + override fun toSpecification(): MutableMap { + val spec = executionResult.toSpecification() + + if (spec["extensions"] != null && extensions.containsKey(DGS_RESPONSE_HEADERS_KEY)) { + val extensions = spec["extensions"] as Map<*, *> + + if (extensions.size != 1) { + spec["extensions"] = extensions.minus(DGS_RESPONSE_HEADERS_KEY) + } else { + spec.remove("extensions") + } + } + + return spec + } +} diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt index dc76c2736..45683260b 100644 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/DefaultDgsFederationResolverTest.kt @@ -198,7 +198,7 @@ class DefaultDgsFederationResolverTest { .satisfies( Consumer { assertThat(it) - .endsWith("MissingFederatedQueryArgument: The federated query is missing field(s) __typename") + .endsWith("The federated query is missing field(s) __typename") } ) } @@ -414,7 +414,7 @@ class DefaultDgsFederationResolverTest { .satisfies( Consumer { assertThat(it) - .contains("com.netflix.graphql.dgs.exceptions.DgsInvalidInputArgumentException: Invalid input argument exception") + .contains("Invalid input argument exception") } ) assertThat(customExceptionHandler.invocationCounter).isEqualTo(1) diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandlerTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandlerTest.kt index 4e302c8c3..970b55f9b 100644 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandlerTest.kt +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DefaultDataFetcherExceptionHandlerTest.kt @@ -16,6 +16,7 @@ package com.netflix.graphql.dgs.exceptions +import com.netflix.graphql.types.errors.ErrorType import graphql.execution.DataFetcherExceptionHandlerParameters import graphql.execution.ResultPath import io.mockk.MockKAnnotations @@ -106,4 +107,23 @@ internal class DefaultDataFetcherExceptionHandlerTest { // We return null here because we don't want graphql-java to write classification field assertThat(result.errors[0].errorType).isNull() } + + @Test + fun `custom DGS exception should return custom error`() { + val customDgsExceptionMessage = "Studio Search Who" + val customDgsExceptionType = ErrorType.FAILED_PRECONDITION + class CustomDgsException() : + DgsException(message = customDgsExceptionMessage, errorType = customDgsExceptionType) + + every { dataFetcherExceptionHandlerParameters.exception }.returns(CustomDgsException()) + + val result = DefaultDataFetcherExceptionHandler().handleException(dataFetcherExceptionHandlerParameters).get() + assertThat(result.errors.size).isEqualTo(1) + + val extensions = result.errors[0].extensions + assertThat(extensions["errorType"]).isEqualTo(customDgsExceptionType.name) + + // We return null here because we don't want graphql-java to write classification field + assertThat(result.errors[0].errorType).isNull() + } } diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DgsExceptionTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DgsExceptionTest.kt new file mode 100644 index 000000000..57c9ecbd3 --- /dev/null +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/exceptions/DgsExceptionTest.kt @@ -0,0 +1,70 @@ +/* + * Copyright 2022 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 + +import graphql.execution.ResultPath +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.entry +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test + +class DgsExceptionTest { + companion object { + const val FAKE_ERROR_MESSAGE = "dgs message" + } + + @Nested + inner class ToGraphQlError { + @Test + fun `should default to not include path`() { + assertThat( + CustomDgsException() + .toGraphQlError() // no path + .path + ).isNull() + } + + @Test + fun `should include path if explicitly asked`() { + val pathSegments = listOf("path", "pathObj") + + assertThat( + CustomDgsException() + .toGraphQlError(path = ResultPath.fromList(pathSegments)) + .path + ).containsAll(pathSegments) + } + + @Test + fun `should contain error class name in extensions`() { + assertThat( + CustomDgsException() + .toGraphQlError() + .extensions + ).contains( + entry( + DgsException.EXTENSION_CLASS_KEY, + CustomDgsException::class.java.name + ) + ) + } + } + + private inner class CustomDgsException : DgsException( + message = FAKE_ERROR_MESSAGE + ) +} diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DefaultDgsQueryExecutorTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DefaultDgsQueryExecutorTest.kt index fbfdc6eee..ce070e1b8 100644 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DefaultDgsQueryExecutorTest.kt +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DefaultDgsQueryExecutorTest.kt @@ -19,11 +19,12 @@ package com.netflix.graphql.dgs.internal import com.jayway.jsonpath.TypeRef import com.jayway.jsonpath.spi.mapper.MappingException import com.netflix.graphql.dgs.* +import com.netflix.graphql.dgs.exceptions.DgsBadRequestException import com.netflix.graphql.dgs.exceptions.DgsQueryExecutionDataExtractionException import com.netflix.graphql.dgs.exceptions.QueryException import com.netflix.graphql.dgs.internal.method.InputArgumentResolver import com.netflix.graphql.dgs.internal.method.MethodDataFetcherFactory -import com.netflix.graphql.types.errors.ErrorType +import graphql.InvalidSyntaxError import graphql.execution.AsyncExecutionStrategy import graphql.execution.AsyncSerialExecutionStrategy import graphql.execution.instrumentation.SimpleInstrumentation @@ -42,6 +43,7 @@ import java.time.LocalDateTime import java.util.* import java.util.function.Supplier +@Suppress("GraphQLUnresolvedReference") @ExtendWith(MockKExtension::class) internal class DefaultDgsQueryExecutorTest { @@ -152,12 +154,48 @@ internal class DefaultDgsQueryExecutorTest { } @Test - fun `Returns a GraphQL Error wth BAD_REQUEST described in the extensions`() { + fun `Empty query returns DgsExecutionResult with NULL_OR_EMPTY_QUERY_EXCEPTION`() { val result = dgsQueryExecutor.execute(" ") + + assertThat(result) + .isNotNull + .isInstanceOf(DgsExecutionResult::class.java) + + assertThat( + result + .errors + .first() + .extensions["errorType"] + // default bad request error type + ).isEqualTo( + DgsBadRequestException() + .errorType + .name + ) + + assertThat( + result + .errors + .first() + .message + ).isEqualTo(DgsBadRequestException.NULL_OR_EMPTY_QUERY_EXCEPTION.message) + + val springResponse = (result as DgsExecutionResult).toSpringResponse() + + assertThat( + springResponse + .statusCode + .value() + ).isEqualTo(400) + } + + @Test + fun `Invalid Syntax query returns a GraphQL Error wth type SyntaxError`() { + val result = dgsQueryExecutor.execute("a") assertThat(result) .isNotNull - .extracting { it.errors.first().extensions["errorType"] } - .isEqualTo(ErrorType.BAD_REQUEST.name) + .extracting { it.errors.first() } + .isInstanceOf(InvalidSyntaxError::class.java) } @Test diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt new file mode 100644 index 000000000..5a00741ac --- /dev/null +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/DgsExecutionResultTest.kt @@ -0,0 +1,83 @@ +/* + * Copyright 2022 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.internal + +import org.assertj.core.api.Assertions.assertThat +import org.assertj.core.api.Assertions.entry +import org.junit.jupiter.api.Nested +import org.junit.jupiter.api.Test +import org.springframework.http.HttpHeaders +import org.springframework.http.HttpStatus + +class DgsExecutionResultTest { + @Test + fun `should be able to pass null for data`() { + assertThat( + DgsExecutionResult( + data = null + ).toSpecification() + ).contains(entry("data", null)) + } + + @Test + fun `should default to not having data`() { + assertThat( + DgsExecutionResult() + .toSpecification() + ).doesNotContainKey("data") + } + + @Test + fun `should be able to pass in custom data`() { + val data = "Check under your chair" + + assertThat( + DgsExecutionResult(data = data) + .toSpecification() + ).contains(entry("data", data)) + } + + @Nested + inner class ToSpringResponse { + @Test + fun `should create a spring response with specified headers`() { + val headers = HttpHeaders() + headers.add("We can add headers now??", "Yes we can") + + assertThat( + DgsExecutionResult( + headers = headers + ).toSpringResponse() + .headers + .toMap() + ).containsAllEntriesOf(headers.toMap()) + } + + @Test + fun `should create a spring response with specified http status code`() { + val httpStatusCode = HttpStatus.ALREADY_REPORTED + + assertThat( + DgsExecutionResult( + status = httpStatusCode + ).toSpringResponse() + .statusCode + .value() + ).isEqualTo(httpStatusCode.value()) + } + } +} diff --git a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/InputArgumentTest.kt b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/InputArgumentTest.kt index e0c987a20..98dd85b66 100644 --- a/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/InputArgumentTest.kt +++ b/graphql-dgs/src/test/kotlin/com/netflix/graphql/dgs/internal/InputArgumentTest.kt @@ -73,7 +73,7 @@ import java.time.LocalDateTime import java.time.format.DateTimeFormatter import java.util.* -@Suppress("unused") +@Suppress("unused", "GraphQLUnresolvedReference") @ExtendWith(MockKExtension::class) internal class InputArgumentTest { @MockK