Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
bitbrain opened this issue Jun 30, 2022 · 3 comments
Closed

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

bitbrain opened this issue Jun 30, 2022 · 3 comments
Assignees
@rstoyanchev
Labels
type: enhancement A general enhancement
Milestone
1.0.1

Comments

@bitbrain
Copy link

bitbrain commented Jun 30, 2022 •
edited
Loading

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.
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 30, 2022
@kitkars
Copy link

kitkars commented Jun 30, 2022 •
edited
Loading

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
Copy link

benneq commented Jun 30, 2022 •
edited
Loading

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
Copy link
Contributor

rstoyanchev commented Jul 1, 2022 •
edited
Loading

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.

@rstoyanchev rstoyanchev self-assigned this Jul 1, 2022
@rstoyanchev rstoyanchev added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 1, 2022
@rstoyanchev rstoyanchev added this to the 1.0.1 milestone Jul 1, 2022
@rstoyanchev rstoyanchev changed the title NullPointerException when not using @Argument annotation Improve handling of case where a source/parent is expected and is null Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@rstoyanchev rstoyanchev

Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
1.0.1
Development

No branches or pull requests
5 participants
@rstoyanchev @bitbrain @benneq @spring-projects-issues @kitkars