From 3ae07c5fc8e9ba5395714a89173f8858c5481a69 Mon Sep 17 00:00:00 2001 From: Jonny Eskew <53191627+jeskew-gov@users.noreply.github.com> Date: Wed, 31 Mar 2021 12:48:09 -0400 Subject: [PATCH 1/2] Only call GraphqlErrorBuilder with non-null data The methods on GraphqlErrorBuilder each contain [an assertion that the supplied data is non-null](https://github.com/graphql-java/graphql-java/blob/master/src/main/java/graphql/GraphqlErrorBuilder.java#L77), but it is not required that these elements be non-null on every throw GraphQLError. For example, errors often omit a `path` property, which will lead to an unhandled `graphql.AssertException` when the error is handled by `GraphQLErrorFromExceptionHandler.java`. --- .../GraphQLErrorFromExceptionHandler.java | 16 ++++---- .../GraphQLErrorFromExceptionHandlerTest.java | 39 +++++++++++++++++++ 2 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java diff --git a/graphql-kickstart-spring-support/src/main/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandler.java b/graphql-kickstart-spring-support/src/main/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandler.java index 7bf8cd3c..7c0b66d9 100644 --- a/graphql-kickstart-spring-support/src/main/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandler.java +++ b/graphql-kickstart-spring-support/src/main/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandler.java @@ -71,15 +71,13 @@ private Collection withThrowable(Throwable throwable, ErrorContext Map extensions = Optional.ofNullable(errorContext.getExtensions()) .orElseGet(HashMap::new); extensions.put("type", throwable.getClass().getSimpleName()); - return singletonList( - GraphqlErrorBuilder.newError() - .message(throwable.getMessage()) - .errorType(errorContext.getErrorType()) - .locations(errorContext.getLocations()) - .path(errorContext.getPath()) - .extensions(extensions) - .build() - ); + GraphqlErrorBuilder builder = GraphqlErrorBuilder.newError() + .extensions(extensions); + Optional.ofNullable(throwable.getMessage()).ifPresent(builder::message); + Optional.ofNullable(errorContext.getErrorType()).ifPresent(builder::errorType); + Optional.ofNullable(errorContext.getLocations()).ifPresent(builder::locations); + Optional.ofNullable(errorContext.getPath()).ifPresent(builder::path); + return singletonList(builder.build()); } } diff --git a/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java b/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java new file mode 100644 index 00000000..0a3213cd --- /dev/null +++ b/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java @@ -0,0 +1,39 @@ +package graphql.kickstart.spring.error; + +import graphql.ErrorType; +import graphql.GraphQLError; +import graphql.GraphqlErrorException; +import graphql.language.SourceLocation; +import java.util.List; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class GraphQLErrorFromExceptionHandlerTest { + @Test + void allows_errors_with_null_path() { + GraphQLErrorFromExceptionHandler sut = new GraphQLErrorFromExceptionHandler(List.of()); + + List errors = List.of( + GraphqlErrorException.newErrorException() + .message("Error without a path") + .sourceLocation(new SourceLocation(0, 0)) + .build(), + GraphqlErrorException.newErrorException() + .message("Error with path") + .sourceLocation(new SourceLocation(0, 0)) + .errorClassification(ErrorType.ValidationError) + .path(List.of()) + .build()); + List processedErrors = sut.filterGraphQLErrors(errors); + + for (int i = 0; i < errors.size(); i++) { + GraphQLError error = errors.get(i); + GraphQLError processedError = processedErrors.get(i); + assertEquals(error.getMessage(), processedError.getMessage()); + assertEquals(error.getErrorType(), processedError.getErrorType()); + assertEquals(error.getLocations(), processedError.getLocations()); + assertEquals(error.getPath(), processedError.getPath()); + } + } +} From 140d935b11643c1a8cf004d42e624c67c99e8a9e Mon Sep 17 00:00:00 2001 From: Jonathan Eskew Date: Wed, 31 Mar 2021 13:54:58 -0400 Subject: [PATCH 2/2] Java 8 compat --- .../GraphQLErrorFromExceptionHandlerTest.java | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java b/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java index 0a3213cd..724c4135 100644 --- a/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java +++ b/graphql-kickstart-spring-support/src/test/java/graphql/kickstart/spring/error/GraphQLErrorFromExceptionHandlerTest.java @@ -4,6 +4,7 @@ import graphql.GraphQLError; import graphql.GraphqlErrorException; import graphql.language.SourceLocation; +import java.util.ArrayList; import java.util.List; import org.junit.jupiter.api.Test; @@ -12,19 +13,20 @@ class GraphQLErrorFromExceptionHandlerTest { @Test void allows_errors_with_null_path() { - GraphQLErrorFromExceptionHandler sut = new GraphQLErrorFromExceptionHandler(List.of()); + GraphQLErrorFromExceptionHandler sut = new GraphQLErrorFromExceptionHandler(new ArrayList<>()); + + List errors = new ArrayList<>(); + errors.add(GraphqlErrorException.newErrorException() + .message("Error without a path") + .sourceLocation(new SourceLocation(0, 0)) + .build()); + errors.add(GraphqlErrorException.newErrorException() + .message("Error with path") + .sourceLocation(new SourceLocation(0, 0)) + .errorClassification(ErrorType.ValidationError) + .path(new ArrayList<>()) + .build()); - List errors = List.of( - GraphqlErrorException.newErrorException() - .message("Error without a path") - .sourceLocation(new SourceLocation(0, 0)) - .build(), - GraphqlErrorException.newErrorException() - .message("Error with path") - .sourceLocation(new SourceLocation(0, 0)) - .errorClassification(ErrorType.ValidationError) - .path(List.of()) - .build()); List processedErrors = sut.filterGraphQLErrors(errors); for (int i = 0; i < errors.size(); i++) {