Skip to content

Commit e823897

Browse files
smyricksmyrick
authored
Do not wrap exceptions twice (ExpediaGroup#1083)
### 📝 Description Do not wrap the exceptions twice if they are already a `GraphQLError` This now also deletes the `KotlinGraphQLError` as it was no longer wrapping anything with required logic. We can instead just map a throwable, if it is not already a `GraphQLError` to a `GraphqlErrorException` graphql-java/graphql-java#1690 ### 🔗 Related Issues Resolves ExpediaGroup#1062
1 parent 4f2295f commit e823897

18 files changed

Lines changed: 76 additions & 269 deletions

File tree

‎examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/exceptions/CustomDataFetcherExceptionHandler.kt‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@
1616

1717
package com.expediagroup.graphql.examples.server.spring.exceptions
1818

19-
import com.expediagroup.graphql.server.exception.KotlinGraphQLError
2019
import com.fasterxml.jackson.annotation.JsonIgnoreProperties
2120
import graphql.ErrorType
2221
import graphql.ErrorType.ValidationError
2322
import graphql.ExceptionWhileDataFetching
2423
import graphql.GraphQLError
24+
import graphql.GraphqlErrorException
2525
import graphql.execution.DataFetcherExceptionHandler
2626
import graphql.execution.DataFetcherExceptionHandlerParameters
2727
import graphql.execution.DataFetcherExceptionHandlerResult
@@ -39,9 +39,17 @@ class CustomDataFetcherExceptionHandler : DataFetcherExceptionHandler {
3939

4040
val error: GraphQLError = when (exception) {
4141
is ValidationException -> ValidationDataFetchingGraphQLError(exception.constraintErrors, path, exception, sourceLocation)
42-
else -> KotlinGraphQLError(exception = exception, locations = listOf(sourceLocation), path = path.toList())
42+
else ->
43+
GraphqlErrorException.newErrorException()
44+
.cause(exception)
45+
.message(exception.message)
46+
.sourceLocation(sourceLocation)
47+
.path(path.toList())
48+
.build()
4349
}
50+
4451
log.warn(error.message, exception)
52+
4553
return DataFetcherExceptionHandlerResult.newResult().error(error).build()
4654
}
4755
}

‎examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQuery.kt‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,31 +16,33 @@
1616

1717
package com.expediagroup.graphql.examples.server.spring.query
1818

19-
import com.expediagroup.graphql.server.exception.KotlinGraphQLError
2019
import com.expediagroup.graphql.server.operations.Query
20+
import graphql.GraphQLError
21+
import graphql.GraphqlErrorException
2122
import graphql.execution.DataFetcherResult
22-
import graphql.execution.ResultPath
23-
import graphql.language.SourceLocation
2423
import org.springframework.stereotype.Component
2524
import java.util.concurrent.CompletableFuture
2625

2726
@Component
2827
class DataAndErrorsQuery : Query {
2928

3029
fun returnDataAndErrors(): DataFetcherResult<String?> {
31-
val error = KotlinGraphQLError(RuntimeException("data and errors"), listOf(SourceLocation(1, 1)), ResultPath.rootPath().toList())
3230
return DataFetcherResult.newResult<String>()
3331
.data("Hello from data fetcher")
34-
.error(error)
32+
.error(getError())
3533
.build()
3634
}
3735

3836
fun completableFutureDataAndErrors(): CompletableFuture<DataFetcherResult<String?>> {
39-
val error = KotlinGraphQLError(RuntimeException("data and errors"), listOf(SourceLocation(1, 1)), ResultPath.rootPath().toList())
4037
val dataFetcherResult = DataFetcherResult.newResult<String>()
4138
.data("Hello from data fetcher")
42-
.error(error)
39+
.error(getError())
4340
.build()
4441
return CompletableFuture.completedFuture(dataFetcherResult)
4542
}
43+
44+
private fun getError(): GraphQLError = GraphqlErrorException.newErrorException()
45+
.cause(RuntimeException("data and errors"))
46+
.message("data and errors")
47+
.build()
4648
}

‎examples/server/spring-server/src/main/kotlin/com/expediagroup/graphql/examples/server/spring/subscriptions/SimpleSubscription.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ package com.expediagroup.graphql.examples.server.spring.subscriptions
1818

1919
import com.expediagroup.graphql.examples.server.spring.context.MySubscriptionGraphQLContext
2020
import com.expediagroup.graphql.generator.annotations.GraphQLDescription
21-
import com.expediagroup.graphql.server.exception.KotlinGraphQLError
2221
import com.expediagroup.graphql.server.operations.Subscription
22+
import graphql.GraphqlErrorException
2323
import graphql.execution.DataFetcherResult
2424
import kotlinx.coroutines.flow.flowOf
2525
import kotlinx.coroutines.reactive.asPublisher
@@ -74,7 +74,7 @@ class SimpleSubscription : Subscription {
7474
fun flowOfErrors(): Publisher<DataFetcherResult<String?>> {
7575
val dfr: DataFetcherResult<String?> = DataFetcherResult.newResult<String?>()
7676
.data(null)
77-
.error(KotlinGraphQLError(Exception("error thrown")))
77+
.error(GraphqlErrorException.newErrorException().cause(Exception("error thrown")).build())
7878
.build()
7979

8080
return flowOf(dfr, dfr).asPublisher()

‎examples/server/spring-server/src/test/kotlin/com/expediagroup/graphql/examples/server/spring/query/CustomDirectiveQueryIT.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,7 @@ class CustomDirectiveQueryIT(@Autowired private val testClient: WebTestClient) {
8484
@MethodSource("specificValueOnlyQueries")
8585
fun `verify specific value only queries with another value`(query: String, expectedValue: String) {
8686
val anotherValue = "hello"
87-
val expectedError = "Exception while fetching data ($query) : " +
88-
"Unsupported value, expected=$expectedValue actual=$anotherValue"
87+
val expectedError = "Unsupported value, expected=$expectedValue actual=$anotherValue"
8988

9089
testClient.post()
9190
.uri(GRAPHQL_ENDPOINT)

‎examples/server/spring-server/src/test/kotlin/com/expediagroup/graphql/examples/server/spring/query/DataAndErrorsQueryIT.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ class DataAndErrorsQueryIT(@Autowired private val testClient: WebTestClient) {
4040
@ValueSource(strings = ["returnDataAndErrors", "completableFutureDataAndErrors"])
4141
fun `verify data and errors queries`(query: String) {
4242
val expectedData = "Hello from data fetcher"
43-
val expectedError = "Exception while fetching data () : data and errors"
43+
val expectedError = "data and errors"
4444

4545
testClient.post()
4646
.uri(GRAPHQL_ENDPOINT)

‎examples/server/spring-server/src/test/kotlin/com/expediagroup/graphql/examples/server/spring/query/ValidatedQueryIT.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,7 @@ class ValidatedQueryIT(@Autowired private val testClient: WebTestClient) {
5454
@ValueSource(strings = ["", " ", "Hello", "1234", "!@#$"])
5555
fun `verify argumentWithValidation query when argument is not valid`(argument: String) {
5656
val query = "argumentWithValidation"
57-
val expectedError = "Exception while fetching data (argumentWithValidation) : " +
58-
"argumentWithValidation.arg.lowerCaseOnly: Argument must be lowercase"
57+
val expectedError = "argumentWithValidation.arg.lowerCaseOnly: Argument must be lowercase"
5958

6059
testClient.post()
6160
.uri(GRAPHQL_ENDPOINT)

‎servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/exception/KotlinGraphQLError.kt‎

Lines changed: 0 additions & 47 deletions
This file was deleted.

‎servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/execution/GraphQLRequestHandler.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
package com.expediagroup.graphql.server.execution
1818

1919
import com.expediagroup.graphql.generator.execution.GraphQLContext
20-
import com.expediagroup.graphql.server.exception.KotlinGraphQLError
2120
import com.expediagroup.graphql.server.extensions.toExecutionInput
21+
import com.expediagroup.graphql.server.extensions.toGraphQLError
2222
import com.expediagroup.graphql.server.extensions.toGraphQLKotlinType
2323
import com.expediagroup.graphql.server.extensions.toGraphQLResponse
2424
import com.expediagroup.graphql.server.types.GraphQLRequest
@@ -44,8 +44,8 @@ open class GraphQLRequestHandler(
4444
return try {
4545
graphQL.executeAsync(executionInput).await().toGraphQLResponse()
4646
} catch (exception: Exception) {
47-
val graphKotlinQLError = KotlinGraphQLError(exception)
48-
GraphQLResponse<Any?>(errors = listOf(graphKotlinQLError.toGraphQLKotlinType()))
47+
val error = exception.toGraphQLError()
48+
GraphQLResponse<Any?>(errors = listOf(error.toGraphQLKotlinType()))
4949
}
5050
}
5151
}

‎servers/graphql-kotlin-server/src/main/kotlin/com/expediagroup/graphql/server/extensions/responseExtensions.kt‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,12 @@
1616

1717
package com.expediagroup.graphql.server.extensions
1818

19-
import com.expediagroup.graphql.server.types.GraphQLServerError
2019
import com.expediagroup.graphql.server.types.GraphQLResponse
20+
import com.expediagroup.graphql.server.types.GraphQLServerError
2121
import com.expediagroup.graphql.server.types.GraphQLSourceLocation
2222
import graphql.ExecutionResult
23-
import graphql.GraphQLError as GraphQLJavaError
23+
import graphql.GraphQLError
24+
import graphql.GraphqlErrorException
2425
import graphql.language.SourceLocation
2526

2627
/**
@@ -33,14 +34,27 @@ fun ExecutionResult.toGraphQLResponse(): GraphQLResponse<*> {
3334
return GraphQLResponse(data, filteredErrors, filteredExtensions)
3435
}
3536

37+
/**
38+
* Turn a thrown exception into a graphql-java [GraphQLError] that we can return in the response
39+
*/
40+
fun Throwable.toGraphQLError(): GraphQLError =
41+
if (this is GraphQLError) {
42+
this
43+
} else {
44+
GraphqlErrorException.newErrorException()
45+
.cause(this)
46+
.message(this.message)
47+
.build()
48+
}
49+
3650
/**
3751
* Convert the graphql-java type to the common serializable type [GraphQLServerError]
3852
*/
39-
fun GraphQLJavaError.toGraphQLKotlinType() = GraphQLServerError(
40-
message.orEmpty(),
41-
locations?.map { it.toGraphQLKotlinType() },
42-
path,
43-
extensions
53+
fun GraphQLError.toGraphQLKotlinType() = GraphQLServerError(
54+
this.message.orEmpty(),
55+
this.locations?.map { it.toGraphQLKotlinType() },
56+
this.path,
57+
this.extensions
4458
)
4559

4660
/**

‎servers/graphql-kotlin-server/src/test/kotlin/com/expediagroup/graphql/server/exception/KotlinGraphQLErrorTest.kt‎

Lines changed: 0 additions & 75 deletions
This file was deleted.

0 commit comments

Comments
 (0)