spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
707 stars 705 forks source link

Support @LoadBalanced WebClient #295

Closed spencergibb closed 6 years ago

SimonScholz commented 6 years ago

Hi, when I create a @LoadBalanced WebClient build like this:

    @Bean
    @LoadBalanced
    public WebClient.Builder getWebClient() {
        return WebClient.builder();
    }

and want to use it for Spring Eureka I still get the following Exception:

java.net.UnknownHostException: TELEGRAM-BOT-SPRING-DMI-WHEATER
    at java.net.InetAddress.getAllByName0(InetAddress.java:1280) ~[na:1.8.0_151]
    at java.net.InetAddress.getAllByName(InetAddress.java:1192) ~[na:1.8.0_151]
    at java.net.InetAddress.getAllByName(InetAddress.java:1126) ~[na:1.8.0_151]
    at java.net.InetAddress.getByName(InetAddress.java:1076) ~[na:1.8.0_151]
    at io.netty.util.internal.SocketUtils$8.run(SocketUtils.java:146) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.internal.SocketUtils$8.run(SocketUtils.java:143) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at java.security.AccessController.doPrivileged(Native Method) ~[na:1.8.0_151]
    at io.netty.util.internal.SocketUtils.addressByName(SocketUtils.java:143) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.DefaultNameResolver.doResolve(DefaultNameResolver.java:43) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.SimpleNameResolver.resolve(SimpleNameResolver.java:63) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.SimpleNameResolver.resolve(SimpleNameResolver.java:55) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:57) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.InetSocketAddressResolver.doResolve(InetSocketAddressResolver.java:32) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.resolver.AbstractAddressResolver.resolve(AbstractAddressResolver.java:108) ~[netty-resolver-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.bootstrap.Bootstrap.doResolveAndConnect0(Bootstrap.java:208) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.bootstrap.Bootstrap.access$000(Bootstrap.java:49) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.bootstrap.Bootstrap$1.operationComplete(Bootstrap.java:188) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.bootstrap.Bootstrap$1.operationComplete(Bootstrap.java:174) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.DefaultPromise.notifyListener0(DefaultPromise.java:512) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.DefaultPromise.notifyListenersNow(DefaultPromise.java:486) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.DefaultPromise.notifyListeners(DefaultPromise.java:425) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.DefaultPromise.trySuccess(DefaultPromise.java:104) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.DefaultChannelPromise.trySuccess(DefaultChannelPromise.java:84) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe.safeSetSuccess(AbstractChannel.java:978) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:512) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(AbstractChannel.java:423) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.AbstractChannel$AbstractUnsafe$1.run(AbstractChannel.java:482) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:163) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:404) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:463) ~[netty-transport-4.1.21.Final.jar:4.1.21.Final]
    at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:886) ~[netty-common-4.1.21.Final.jar:4.1.21.Final]
    at java.lang.Thread.run(Thread.java:748) ~[na:1.8.0_151]

The code, which uses this looks like this:

@RestController
public class TelegramBotController {

    private RestTemplate restTemplate;
    private WebClient webClient;

    public TelegramBotController(RestTemplate restTemplate, WebClient.Builder webClient) {
        this.restTemplate = restTemplate;
        this.webClient = webClient.defaultHeader(HttpHeaders.ACCEPT, MediaType.APPLICATION_JSON_VALUE)
                .defaultHeader(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE).build();
    }

    @PostMapping("/blockingdmiWebhook")
    public ResponseEntity<Mono<Void>> webhook(@RequestBody Update newUpdate) throws JsonProcessingException {

        HttpHeaders headers = new HttpHeaders();
        headers.setContentType(MediaType.APPLICATION_JSON);
        headers.setAccept(Collections.singletonList(MediaType.APPLICATION_JSON));

        HttpEntity<Update> requestEntity = new HttpEntity<>(newUpdate, headers);

        return restTemplate.exchange("http://TELEGRAM-BOT-SPRING-DMI-WHEATER/dmiWebhook", HttpMethod.POST,
                requestEntity, new ParameterizedTypeReference<Mono<Void>>() {
                });
    }

    @PostMapping("/dmiWebhook")
    public Mono<Void> dmiWeatherWebhook(@RequestBody Update newUpdate) {
        return webClient.post().uri("http://TELEGRAM-BOT-SPRING-DMI-WHEATER/dmiWebhook")
                .body(BodyInserters.fromObject(newUpdate)).exchange().then();
    }
}

While the version with the RestTemplate, which also has been created with @LoadBalanced, works totally fine the one with the WebClient does not.

I don't see anything wrong according to the documentation of your commit.

ryanjbaxter commented 6 years ago

Please open a separate issue with the code in a zip file or in a git repo

wojcikt commented 6 years ago

I think this issue is related to the fact, that you're building the WebClient too early (in constructor).

org.springframework.cloud.client.loadbalancer.reactive.ReactiveLoadBalancerAutoConfiguration is responsible for customizing the WebClient.Builder instances with LoadBalancerExchangeFilterFunction, which provides load balancing. From what I learned, ReactiveLoadBalancerAutoConfiguration runs after WebClient.Builders are injected (for example in your controllers).

The simple workaround is creating your clients when they are used (for example inside request handler method), not during initialization (since ReactiveLoadBalancerAutoConfiguration will be actually able to customize builders).

Is this a desired behaviour?

I've put together a quick code snippet:

@Controller
class BuggedController {
    private final WebClient webClient;

    @Autowired
    public BuggedController(WebClient.Builder lbWebClientBuilder) {
        // lbWebClientBuilder is not yet customized by ReactiveLoadBalancerAutoConfiguration
        this.webClient = lbWebClientBuilder.build();
    }

    @GetMapping("/")
    public Mono<?> handle() {
        // no load-balancing here
        return webClient.post().exchange(); // ...
    }
}

@Controller
class WorkingController {
    private final WebClient.Builder webClientBuilder;

    @Autowired
    public WorkingController(WebClient.Builder lbWebClientBuilder) {
        this.webClientBuilder = lbWebClientBuilder;
    }

    @GetMapping("/")
    public Mono<?> handle() {
        // working load-balancing
        return webClientBuilder.build().post().exchange(); // ...
    }
}
r6q commented 6 years ago

@wojcikt This is indeed a working solution, however not desirable in many cases. Recreating WebClient on every request is quite wasteful and there are cases when some other service requires to have WebClient dependency.

The reason why @LoadBalanced works for RestTemplate is that LoadBalancerAutoConfiguration works directly with the RestTemplate bean opposed to ReactiveLoadBalancerAutoConfiguration which works with the builder, this due to the fact that WebClient is immutable and can't be extended after creation like RestTemplate.

Here is a quick workaround:

    @Bean
    WebClient webClient(LoadBalancerClient loadBalancerClient) {
        return WebClient.builder()
                .filter(new LoadBalancerExchangeFilterFunction(loadBalancerClient))
                .build();
    }

This is essentially what happens in ReactiveLoadBalancerAutoConfiguration. Note! If you have other WebClientCustomizers in your project, you'll have to include them manually as well.

marbaez commented 5 years ago

@robertt1234 your workaround saved my day. This small differences between Reactive and non reactive Spring make sometimes really difficult to work with Reactive API