graphql-java-kickstart / graphql-spring-boot

GraphQL and GraphiQL Spring Framework Boot Starters - Forked from oembedler/graphql-spring-boot due to inactivity.
https://www.graphql-java-kickstart.com/spring-boot/
MIT License
1.5k stars 325 forks source link

Forwarding of `RequestContext` not working in async mode #729

Open pelletier197 opened 3 years ago

pelletier197 commented 3 years ago

Describe the bug Many spring framework rely on the RequestContextHolder static class to access the current request attributes. One of them is the Spring-OAuth2 framework that uses it to relay user-token attributes stored in the session to other services in a micro-service architecture.

Only problem being that in async mode, the RequestContextHolder.getRequestAttributes() returns null. I see that something custom was required to do this with the security context, so would something similar be possible for RequestContext ?

To Reproduce Steps to reproduce the behavior:

  1. Enable async mode
  2. Perform any request, and just call RequestContextHolder.getRequestAttributes() from within any resolver. You should see that it's null.
  3. Disable async mode
  4. Do the same thing, and the attributes should be non-null

Expected behavior Expect the request attributes to be forwarded when the resolvers are called. Having the request in the DataFetchingEnvironment is unfortunately not enough for libraries.

markbanierink commented 2 years ago

We experience the same issue. For now we fixed it by creating a AsyncTaskDecorator which wraps the context, but I would expect the request context to be propagated by default.

Perhaps the Executor set in the GraphQLWebAutoConfiguration could be wrapped with something like the below?

  private Executor wrap(Executor executor) {
    return task -> executor.execute(wrap(task));
  }

  private Runnable wrap(Runnable task) {
    RequestAttributes requestAttributes = RequestContextHolder.getRequestAttributes();
    return () -> {
      try {
        RequestContextHolder.setRequestAttributes(requestAttributes);
        task.run();
      } finally {
        RequestContextHolder.resetRequestAttributes();
      }
    };
  }
pelletier197 commented 2 years ago

For now we fixed it by creating a AsyncTaskDecorator which wraps the context.

Thanks for the info. I did not spend a lot of time investigating on how to make it work, but that works quite well.

markbanierink commented 2 years ago

For anybody who needs it now:

    @Bean
    default AsyncTaskDecorator asyncTaskDecorator() {
        return task -> {
            var requestAttributes = RequestContextHolder.getRequestAttributes();
            return () -> {
                try {
                    RequestContextHolder.setRequestAttributes(requestAttributes);
                    task.run();
                }
                finally {
                    RequestContextHolder.resetRequestAttributes();
                }
            };
        };
    }
pelletier197 commented 2 years ago

Finally, there is an issue with this approach apparently. This works well for attributes that are scoped to the session (RequestAttributes.SCOPE_SESSION), but doesn't work well for RequestAttributes.SCOPE_REQUEST attributes. Even if the request is not yet completed, since the request changes thread and is executed asynchronously, it seems to cause some issues:

I receive this error when accessing RequestAttributes.SCOPE_REQUEST attributes.

Cannot ask for request attribute - request is not active anymore
ghost commented 2 years ago

We are facing the same issue. We have a central microservice to get specific data from the header and this is now not possible anymore. In the Datafeching-Environment is not enough for our purpose