micronaut-projects / micronaut-core

Micronaut Application Framework
http://micronaut.io
Apache License 2.0
6.04k stars 1.06k forks source link

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

Open denis111 opened 3 years ago

denis111 commented 3 years ago

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

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

denis111 commented 3 years ago

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

graemerocher commented 3 years ago

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

denis111 commented 3 years ago

@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 commented 3 years ago

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 commented 3 years ago

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

graemerocher commented 2 years ago

@dstepanov this still an issue after your changes?

MajaStojkovic-TomTom commented 2 years ago

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?