Improve handling of case where a source/parent is expected and is null

[bitbrain]

Given the following schema:

type User {
   id: ID!
   name: String!
}

type Query {
   findUserById(id:ID!) : User
}

and the following controller

@Controller
public class UserController {

   @QueryMapping
   public User findUserById(final String id) {
      return userService.findUserById(id);
   }
}

it will explode with:

2022-06-30 13:51:33.824 ERROR 5374 --- [nio-8080-exec-2] s.g.e.ExceptionResolversExceptionHandler : Unresolved NullPointerException for executionId a7f5606d-90a2-f444-c38f-0d889a990676

java.lang.NullPointerException: Cannot invoke "Object.getClass()" because "source" is null
	at org.springframework.graphql.data.method.annotation.support.SourceMethodArgumentResolver.resolveArgument(SourceMethodArgumentResolver.java:71) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.data.method.HandlerMethodArgumentResolverComposite.resolveArgument(HandlerMethodArgumentResolverComposite.java:83) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.data.method.annotation.support.DataFetcherHandlerMethod.getMethodArgumentValues(DataFetcherHandlerMethod.java:170) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.data.method.annotation.support.DataFetcherHandlerMethod.invoke(DataFetcherHandlerMethod.java:115) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.data.method.annotation.support.AnnotatedControllerConfigurer$SchemaMappingDataFetcher.get(AnnotatedControllerConfigurer.java:497) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.execution.ContextDataFetcherDecorator.lambda$get$0(ContextDataFetcherDecorator.java:64) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.execution.ReactorContextManager.invokeCallable(ReactorContextManager.java:104) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.graphql.execution.ContextDataFetcherDecorator.get(ContextDataFetcherDecorator.java:63) ~[spring-graphql-1.0.0.jar:1.0.0]
	at graphql.execution.instrumentation.dataloader.DataLoaderDispatcherInstrumentation.lambda$instrumentDataFetcher$0(DataLoaderDispatcherInstrumentation.java:87) ~[graphql-java-18.1.jar:na]
	at graphql.execution.ExecutionStrategy.fetchField(ExecutionStrategy.java:279) ~[graphql-java-18.1.jar:na]
	at graphql.execution.ExecutionStrategy.resolveFieldWithInfo(ExecutionStrategy.java:210) ~[graphql-java-18.1.jar:na]
	at graphql.execution.ExecutionStrategy.resolveField(ExecutionStrategy.java:182) ~[graphql-java-18.1.jar:na]
	at graphql.execution.AsyncSerialExecutionStrategy.lambda$execute$1(AsyncSerialExecutionStrategy.java:43) ~[graphql-java-18.1.jar:na]
	at graphql.execution.Async.eachSequentiallyImpl(Async.java:80) ~[graphql-java-18.1.jar:na]
	at graphql.execution.Async.eachSequentially(Async.java:69) ~[graphql-java-18.1.jar:na]
	at graphql.execution.AsyncSerialExecutionStrategy.execute(AsyncSerialExecutionStrategy.java:38) ~[graphql-java-18.1.jar:na]
	at graphql.execution.Execution.executeOperation(Execution.java:160) ~[graphql-java-18.1.jar:na]
	at graphql.execution.Execution.execute(Execution.java:106) ~[graphql-java-18.1.jar:na]
	at graphql.GraphQL.execute(GraphQL.java:641) ~[graphql-java-18.1.jar:na]
	at graphql.GraphQL.lambda$parseValidateAndExecute$11(GraphQL.java:561) ~[graphql-java-18.1.jar:na]
	at java.base/java.util.concurrent.CompletableFuture.uniComposeStage(CompletableFuture.java:1187) ~[na:na]
	at java.base/java.util.concurrent.CompletableFuture.thenCompose(CompletableFuture.java:2309) ~[na:na]
	at graphql.GraphQL.parseValidateAndExecute(GraphQL.java:556) ~[graphql-java-18.1.jar:na]
	at graphql.GraphQL.executeAsync(GraphQL.java:524) ~[graphql-java-18.1.jar:na]
	at org.springframework.graphql.execution.DefaultExecutionGraphQlService.lambda$execute$2(DefaultExecutionGraphQlService.java:81) ~[spring-graphql-1.0.0.jar:1.0.0]
	at reactor.core.publisher.MonoDeferContextual.subscribe(MonoDeferContextual.java:47) ~[reactor-core-3.4.19.jar:3.4.19]
	at reactor.core.publisher.Mono.subscribe(Mono.java:4397) ~[reactor-core-3.4.19.jar:3.4.19]
	at reactor.core.publisher.Mono.subscribeWith(Mono.java:4512) ~[reactor-core-3.4.19.jar:3.4.19]
	at reactor.core.publisher.Mono.toFuture(Mono.java:4917) ~[reactor-core-3.4.19.jar:3.4.19]
	at org.springframework.core.ReactiveAdapterRegistry$ReactorRegistrar.lambda$registerAdapters$5(ReactiveAdapterRegistry.java:265) ~[spring-core-5.3.21.jar:5.3.21]
	at org.springframework.core.ReactiveAdapter.fromPublisher(ReactiveAdapter.java:121) ~[spring-core-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.function.DefaultAsyncServerResponse.create(DefaultAsyncServerResponse.java:188) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.function.ServerResponse.async(ServerResponse.java:239) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.graphql.server.webmvc.GraphQlHttpHandler.handleRequest(GraphQlHttpHandler.java:102) ~[spring-graphql-1.0.0.jar:1.0.0]
	at org.springframework.web.servlet.function.support.HandlerFunctionAdapter.handle(HandlerFunctionAdapter.java:106) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1067) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:963) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1006) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at org.springframework.web.servlet.FrameworkServlet.doPost(FrameworkServlet.java:909) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:681) ~[tomcat-embed-core-9.0.64.jar:4.0.FR]
	at org.springframework.web.servlet.FrameworkServlet.service(FrameworkServlet.java:883) ~[spring-webmvc-5.3.21.jar:5.3.21]
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:764) ~[tomcat-embed-core-9.0.64.jar:4.0.FR]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:227) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:53) ~[tomcat-embed-websocket-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.springframework.web.filter.RequestContextFilter.doFilterInternal(RequestContextFilter.java:100) ~[spring-web-5.3.21.jar:5.3.21]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.21.jar:5.3.21]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.springframework.web.filter.FormContentFilter.doFilterInternal(FormContentFilter.java:93) ~[spring-web-5.3.21.jar:5.3.21]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.21.jar:5.3.21]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.springframework.web.filter.CharacterEncodingFilter.doFilterInternal(CharacterEncodingFilter.java:201) ~[spring-web-5.3.21.jar:5.3.21]
	at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:117) ~[spring-web-5.3.21.jar:5.3.21]
	at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:189) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:162) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:197) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:97) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:541) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:135) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:92) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:78) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:360) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.coyote.http11.Http11Processor.service(Http11Processor.java:399) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:65) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:890) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1787) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.util.threads.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1191) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.util.threads.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:659) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61) ~[tomcat-embed-core-9.0.64.jar:9.0.64]
	at java.base/java.lang.Thread.run(Thread.java:833) ~[na:na]

The only way to fix this right now is to annotate the argument correctly with @Argument but new users of this framework might not know this.

[kitkars]

I believe we have similar behavior for @PathVariable / @ RequestBody with REST API. If we do not add the annotation, it would be treated as null.

[benneq]

If we do not add the annotation, it would be treated as null.

Without annotation it will be treated as "Source" object. And the source for fields of the Query type is null.

@QueryMapping
User findUserById(@Argument String id) { // here source would be null, because we are at the root 
  return userService.findUserById(id);
}

@SchemaMapping(type = "User", field = "name")
String userName(User source) {
  return source.getName().toUppercase();
}

See: https://docs.spring.io/spring-graphql/docs/current/reference/html/#controllers-schema-mapping

[rstoyanchev]

This is by design, having @Argument is expected and documented indeed.

For what it's worth @PathVariable and @RequestBody are both required. @RequestParam and @ModelAttribute are not, and between the two it's decided depending on whether it is a "simple" type vs command object. In the case here, however, the source/parent doesn't have to be complex type, see #370.

So at the end of the day, we can only do this for the source/parent or for arguments, but not both, and source/parent didn't even require an annotation for any customizations, unlike arguments.

We can't change this but we could have better handling for this illegal state case, avoiding the NPE, and having a better message to state clearly that a method parameter without an annotation is treated as the source.