Exception handlers not called when exception is thrown when parsing variables

[pelletier197]

Describe the bug When using request variables in GraphQL, the behaviour is really different than without variables. My model has a query using variables, and when passing null in a non-nullable field, my custom exception handler is not called. Even worse, the backend actually returns 500, instead of the normal 200.

I haven't yet found a way to handle this error correctly.

To Reproduce

This is a minimal example:

# schema.graphqls

extend type Query {
  groups(filters: GroupFilters): [Group!]!
}

input GroupFilters {
  ids: [String!] # Notice the non-nullable string array element
}

type Group {
   id: String!
   name: String!
}

Calling the model (using the playground) with the following query

# Query
query GetEntity($filters: GroupFilters) {
  groups(filters: $filters) {
    id,
    name
  }
}

and the following variables

{"filters": {"ids": [null]}}

End up in a 500, and my custom exception handler not being called.

Expected behavior The same request, but not using any variables (so passing the filters directly) returns the following response

{
  "errors": [
    {
      "message": "Validation error of type WrongType: argument 'filters.ids[0]' with value 'NullValue{}' must not be null @ 'groups'",
      "locations": []
    }
  ],
  "extensions": {},
  "data": null
}

Screenshots (My actual model is a bit more complex, but here's what I have) Using filters directly: it works

*Using variables: returns 500

[oliemansm]

Are you using spring-boot starter or webflux?

[pelletier197]

I am using spring-boot starter. And I'm also on version 11.0.0. Forgot to mention that!

[oliemansm]

And the problem is also reproducible using 11.1.0-SNAPSHOT? You'll have to add the snapshot repo to your build file to be able to test that.

[pelletier197]

Yes. Same error with 11.1.0-SNAPSHOT. Here is a more detailed stacktrace of what I get.

graphql.execution.NonNullableValueCoercedAsNullException: Field 'ids' of variable 'filters' has coerced Null value for NonNull type 'String!'
	at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:142)
	at graphql.execution.ValuesResolver.coerceValueForList(ValuesResolver.java:222)
	at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:156)
	at graphql.execution.ValuesResolver.coerceValueForInputObjectType(ValuesResolver.java:195)
	at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:159)
	at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:140)
	at graphql.execution.ValuesResolver.coerceVariableValues(ValuesResolver.java:70)
	at graphql.analysis.QueryTraverser.coerceVariables(QueryTraverser.java:64)
	at graphql.analysis.QueryTraverser.<init>(QueryTraverser.java:60)
	at graphql.analysis.QueryTraverser.<init>(QueryTraverser.java:40)
	at graphql.analysis.QueryTraverser$Builder.build(QueryTraverser.java:297)
	at graphql.analysis.MaxQueryDepthInstrumentation.newQueryTraverser(MaxQueryDepthInstrumentation.java:92)
	at graphql.analysis.MaxQueryDepthInstrumentation.lambda$beginValidation$2(MaxQueryDepthInstrumentation.java:57)
	at graphql.execution.instrumentation.SimpleInstrumentationContext.onCompleted(SimpleInstrumentationContext.java:48)
	at graphql.execution.instrumentation.ChainedInstrumentation$ChainedInstrumentationContext.lambda$onCompleted$1(ChainedInstrumentation.java:238)
	at graphql.com.google.common.collect.ImmutableList.forEach(ImmutableList.java:405)
	at graphql.execution.instrumentation.ChainedInstrumentation$ChainedInstrumentationContext.onCompleted(ChainedInstrumentation.java:238)
	at graphql.GraphQL.validate(GraphQL.java:544)
	at graphql.GraphQL.parseAndValidate(GraphQL.java:506)
	at graphql.GraphQL.lambda$parseValidateAndExecute$10(GraphQL.java:475)
	at graphql.execution.preparsed.NoOpPreparsedDocumentProvider.getDocument(NoOpPreparsedDocumentProvider.java:15)
	at graphql.GraphQL.parseValidateAndExecute(GraphQL.java:477)
	at graphql.GraphQL.executeAsync(GraphQL.java:446)
	at graphql.kickstart.execution.GraphQLInvoker.executeAsync(GraphQLInvoker.java:26)
	at graphql.kickstart.execution.GraphQLInvoker.queryAsync(GraphQLInvoker.java:35)
	at graphql.kickstart.servlet.HttpRequestInvokerImpl.invoke(HttpRequestInvokerImpl.java:78)
	at graphql.kickstart.servlet.HttpRequestInvokerImpl.execute(HttpRequestInvokerImpl.java:37)
	at graphql.kickstart.servlet.HttpRequestHandlerImpl.handle(HttpRequestHandlerImpl.java:39)
	at graphql.kickstart.servlet.AbstractGraphQLHttpServlet.doRequest(AbstractGraphQLHttpServlet.java:82)
	at graphql.kickstart.servlet.AbstractGraphQLHttpServlet.doPost(AbstractGraphQLHttpServlet.java:74)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:652)

It seems the code that parses the variables does not handle the exception. It's parsed really early in the GraphQL request, and it just bubbles up until it does a 500 (And it even takes a few seconds before the 500 arrives)

[fabianotorresrj]

Hello, how are you? I hope well.

I have the same problem. When a payload with incompatible types is sent, for example., A string for a BigDecimal, an exception occurs, the framework is able to detect, display a message in the system.out but it is not captured by the Error Handlers. In addition to the request returning only when a timeout occurs without any indication of the problem

This problem occurs both in version 11.0.0 and 11.1.0

2021-05-26 16:38:15.019 ERROR [intelipoint-api,,] 29138 --- [nio-8080-exec-3] o.a.c.c.C.[Tomcat].[localhost].[/] : Error during processing of asynchronous Runnable via AsyncContext.start() graphql.schema.CoercingParseValueException: Variable 'type' has an invalid value : Invalid input for Enum 'LocationType'. No value found for name '' at graphql.schema.CoercingParseValueException$Builder.build(CoercingParseValueException.java:46) at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:178) at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:140) at graphql.execution.ValuesResolver.coerceValueForInputObjectType(ValuesResolver.java:195) at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:159) at graphql.execution.ValuesResolver.coerceValueForInputObjectType(ValuesResolver.java:195) at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:159) at graphql.execution.ValuesResolver.coerceValue(ValuesResolver.java:140) at graphql.execution.ValuesResolver.coerceVariableValues(ValuesResolver.java:70) at graphql.analysis.QueryTraverser.coerceVariables(QueryTraverser.java:64) at graphql.analysis.QueryTraverser.<init>(QueryTraverser.java:60) at graphql.analysis.QueryTraverser.<init>(QueryTraverser.java:40) at graphql.analysis.QueryTraverser$Builder.build(QueryTraverser.java:297) at graphql.analysis.MaxQueryDepthInstrumentation.newQueryTraverser(MaxQueryDepthInstrumentation.java:92) at graphql.analysis.MaxQueryDepthInstrumentation.lambda$beginValidation$2(MaxQueryDepthInstrumentation.java:57) at graphql.execution.instrumentation.SimpleInstrumentationContext.onCompleted(SimpleInstrumentationContext.java:48) at graphql.execution.instrumentation.ChainedInstrumentation$ChainedInstrumentationContext.lambda$onCompleted$1(ChainedInstrumentation.java:238) at graphql.com.google.common.collect.ImmutableList.forEach(ImmutableList.java:405) at graphql.execution.instrumentation.ChainedInstrumentation$ChainedInstrumentationContext.onCompleted(ChainedInstrumentation.java:238) at graphql.GraphQL.validate(GraphQL.java:544) at graphql.GraphQL.parseAndValidate(GraphQL.java:506) at graphql.GraphQL.lambda$parseValidateAndExecute$10(GraphQL.java:475) at graphql.execution.preparsed.NoOpPreparsedDocumentProvider.getDocument(NoOpPreparsedDocumentProvider.java:15) at graphql.GraphQL.parseValidateAndExecute(GraphQL.java:477) at graphql.GraphQL.executeAsync(GraphQL.java:446) at graphql.kickstart.execution.GraphQLInvoker.executeAsync(GraphQLInvoker.java:37) at graphql.kickstart.execution.GraphQLInvoker.execute(GraphQLInvoker.java:28) at graphql.kickstart.servlet.HttpRequestInvokerImpl.invoke(HttpRequestInvokerImpl.java:159) at graphql.kickstart.servlet.HttpRequestInvokerImpl.lambda$invokeAndHandleAsync$2(HttpRequestInvokerImpl.java:77) at org.springframework.security.concurrent.DelegatingSecurityContextRunnable.run(DelegatingSecurityContextRunnable.java:82) at org.apache.catalina.core.AsyncContextImpl$RunnableWrapper.run(AsyncContextImpl.java:547) at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) at java.base/java.lang.Thread.run(Thread.java:829) 2021-05-26 16:38:24.895 WARN [intelipoint-api,,] 29138 --- [nio-8080-exec-4] g.k.servlet.HttpRequestInvokerImpl : GraphQL execution canceled because timeout of 30000 millis was reached. The following query was being executed when this happened: mutation ($input: CreateManagerInput!) { createManager(input: $input) { id } }

[KurtStauffer]

I've been running into this issue lately as well since upgrading to 11.1.0. From what I can tell MaxQueryDepthInstrumentation and MaxQueryComplexityInstrumentation will create a QueryTraverser that will attempt to coerce and validate the variables. It seems that these validation exceptions are not being handled properly thus causing the request to eventually timeout with a 500.

As of now I have found 2 temporary workarounds:

Workaround 1:

Omit the following properties:

graphql.servlet.max-query-complexity
graphql.servlet.max-query-depth

This will prevent those associated instrumentation beans from being wired up and the validation errors will come back properly without timing out. Obviously the downside to this is you lose the complexity and depth validation checks, which bring us to the next option...

Workaround 2:

This is a bit more work but it will continue to provide the complexity and depth checks as well as variable validation without causing the requests to timeout.

1. Omit the above properties
2. Add custom complexity and depth properties, i.e.

app.graphql.servlet.max-query-complexity
app.graphql.servlet.max-query-depth

3. Copy the above instrumentation classes into your own custom classes, i.e. CustomMaxQueryDepthInstrumentation & CustomMaxQueryComplexityInstrumentation
4. Modify the newQueryTraverser method of these classes to catch exceptions from QueryTraverser and wrap it in a new AbortExecutionException and throw it. This will allow the calling GraphQL class to handle it gracefully.
5. Finally, wire up your custom instrumentation beans:

@Bean
  public CustomMaxQueryDepthInstrumentation maxQueryDepthInstrumentation() {
    return new CustomMaxQueryDepthInstrumentation(<customMaxDepthFromProperties>);
  }
@Bean
  public CustomMaxQueryComplexityInstrumentation maxQueryComplexityInstrumentation() {
    return new CustomMaxQueryComplexityInstrumentation(<customMaxComplexityFromProperties>);
  }

After this the appropriate error message should returned, however, the downside is that the error classification is ExecutionAborted instead of ValidationError.

I know that these workarounds are less than ideal and are not meant to be a final solution by any means. But hopefully they will help some of you who are in a position where not upgrading is not option.

[andrewkcarter]

Thanks for the workaround @KurtStauffer. @oliemansm any hope of getting this built in?

[andrewkcarter]

This is still an issue in 12.0.0.

[arquadrado]

Also looking forward for a solution for this problem in 12.0.0. Also, is this solved in more recent versions?