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.

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

Project Reactor ServerRequestContext lost when using Mono.fromFeature() #5549

Open
denis111 opened this issue Jun 7, 2021 · 7 comments
Open

Comments

@denis111
Copy link

denis111 commented Jun 7, 2021

When using project Reactor flow ServerRequestContext is lost when Mono is created by Mono.fromFuture. Here's the example code (i think it's too small to publish on github):

@Controller("/context")
public class ContextController {

  @Get(value = "/flux", produces = MediaType.TEXT_PLAIN)
  Flux<String> flux() {
    return Mono
        .fromFuture(calculateAsync())
        .flux()
        .map(s -> s + " ")
        .flatMap(v -> Flux.just(v + (ServerRequestContext.currentRequest().isPresent() ? "ok" : "nok")));
  }

  @Get(value = "/flowable", produces = MediaType.TEXT_PLAIN)
  Flowable<String> flowable() {
    return Single
        .fromFuture(calculateAsync())
        .toFlowable()
        .map(s -> s + " ")
        .flatMap(v -> Flux.just(v + (ServerRequestContext.currentRequest().isPresent() ? "ok" : "nok")));
  }

  public CompletableFuture<String> calculateAsync() {
    CompletableFuture<String> completableFuture = new CompletableFuture<>();

    Executors.newCachedThreadPool().submit(() -> {
      Thread.sleep(500);
      completableFuture.complete("Hello ");
      return null;
    });

    return completableFuture;
  }
}

If you go to /context/flowable it's ok, context present but in /context/flux not.

Task List

  • [ x ] Steps to reproduce provided
  • [ x ] Example that reproduces the problem uploaded to Github
  • [ x ] Full description of the issue provided (see below)

Steps to Reproduce

  1. Go to /context/flux

Expected Behaviour

The output should be Hello ok

Actual Behaviour

The output is Hello nok

Environment Information

  • Micronaut Version: v2.5.5
  • JDK Version: 11
@denis111
Copy link
Author

denis111 commented Jun 8, 2021

If i change .fromFuture(calculateAsync()) to .fromCallable(() -> calculateAsync().get()) then it works fine

@graemerocher
Copy link
Contributor

I think that is the way to go. Using fromFuture you would have to make sure the thread pool is instrumented

@denis111
Copy link
Author

denis111 commented Jun 8, 2021

@graemerocher yes, I already changed everything in my "real" project to fromCallable. But with flowable it worked fine, the CompletableFuture there is not mine but returned by a third party library.

@graemerocher
Copy link
Contributor

so in general as part of the shift to reactor we need to rewrite the instrumentation logic in reactor to use the context https://projectreactor.io/docs/core/release/api/reactor/util/context/Context.html

This work is not undertaken yet but will solve the issue you see for Reactor in the future

@denis111
Copy link
Author

denis111 commented Jun 8, 2021

@graemerocher ok, thank you! I'll just use fromCallable for now.

@graemerocher
Copy link
Contributor

@dstepanov this still an issue after your changes?

@MajaStojkovic-TomTom
Copy link

We notice a similar issue with Mono response which we notice only occurs when the runtime is jetty or azure_function but for netty, it works fine. Are there any updates on the issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants