spring-cloud / spring-cloud-commons

Common classes used in different Spring Cloud implementations
Apache License 2.0
701 stars 697 forks source link

"invalid scheme [lb]" when upgrading from boot 3.1.5 to 3.1.6 (also present in 3.1.7) when using SimpleReactiveDiscoveryClient #1320

Closed snussbaumer closed 7 months ago

snussbaumer commented 7 months ago

The code below works fine with Spring Cloud 2022.0.4 and Boot 3.1.5, but fails when upgrading to boot 3.1.6 or 3.1.7

return loadBalancedWebClientBuilder.build().get()
        .uri("lb://backend-service/test") //
        .exchangeToFlux( response -> response.bodyToFlux(String.class));

The error is :

org.springframework.web.reactive.function.client.WebClientRequestException: Invalid scheme [lb]
    at org.springframework.web.reactive.function.client.ExchangeFunctions$DefaultExchangeFunction.lambda$wrapException$9(ExchangeFunctions.java:136) ~[spring-webflux-6.0.14.jar:6.0.14]
    Suppressed: reactor.core.publisher.FluxOnAssembly$OnAssemblyException: 
Error has been observed at the following site(s):
    *__checkpoint ⇢ Request to GET lb://backend-service/test [DefaultWebClient]
    *__checkpoint ⇢ Handler com.example.demo.TestController#test() [DispatcherHandler]
    *__checkpoint ⇢ HTTP GET "/test" [ExceptionHandlingWebHandler]

This happens when using SimpleReactiveDiscoveryClient.

I have a project that reproduces the problem here https://github.com/snussbaumer/repro-invalid-scheme-lb (change spring-boot-parent version to 3.1.5 and the test will work properly)

The problems seems to come from DefaultServiceInstance.getScheme that is not implemented (and thus always returns null). In LoadBalancerUriTools.doReconstructURI the code below will then pick "lb" as scheme :

        String scheme = Optional.ofNullable(serviceInstance.getScheme())
                .orElse(computeScheme(original, serviceInstance)); 

The fix is quite simple, in DefaultServiceInstance it would simply be a matter of adding :


        @Override
        public String getScheme() {
            return isSecure() ? "https" : "http";
        }

but I'm not sure of the overall impact, you will now better :smile:

Two of us have encountered the problem (it comes from https://github.com/spring-projects/spring-boot/issues/39041)

OlgaMaciaszek commented 7 months ago

Hi @snussbaumer , thanks for reporting the issue. Please provide more details on what's your scenario here? We mainly use lb scheme for load-balanced calls within SC Gateway, but then only for internal processing and not directly in the calls from a client. That still works as it should with Boot 3.1.7, as you can see on this sample branch. To reproduce, run eureka-server, fraud-verifier and gateway-proxy and run curl --location 'http://localhost:9083/fraud-verifier/users/test'.

snussbaumer commented 7 months ago

Hello Olga,

Thanks for the answer. The problem does not happen with eureka, I saw it only with SimpleReactiveDiscoveryClient.

EurekaReactiveDiscoveryClient returns "EurekaServiceInstance" that properly implement getScheme()

SimpleReactiveDiscoveryClient returns DefaultServiceInstance that does not implement getScheme(), which is why there is a problem now.

In my case the scenario is mainly for integration tests where we do not want to run an eureka server for tests. This is what is illustrated with the repro I linked. It's also not a super fun "onboarding" experience for new users as vvalencia-cl reported in the issue I linked :

My problem is also in Integration Tests, using SimpleReactiveDiscoveryClient. I've never tried to run the app, because the tests weren't passing

OlgaMaciaszek commented 7 months ago

That's the one that the reactive GW proxy uses; configured here. If you run the GW app in debug mode and place a breakpoint on org.springframework.cloud.client.discovery.simple.reactive.SimpleReactiveDiscoveryClient#getInstances, you will see that. Since we only use that scheme for internal processing and not for actual calls, I don't think those tests reflect real-life scenarios. If you do have such scenarios, please provide more details and a sample app that reproduces the issue.

snussbaumer commented 7 months ago

well I don't know what more details I can give and I already provided a self contained app that reproduces the problem in my first comment ...

I've updated the README.md and renamed the test file (mvn test didn't pick the test before).

if you clone the project you can run it with mvn test and you'll get DemoApplicationTests.test:37 Status expected:<200 OK> but was:<500 INTERNAL_SERVER_ERROR> due to and error Invalid scheme [lb]

now change spring-boot-parent version to 3.1.5 in pom.xml and run mvn test again and you'll see that the test runs fine.

the problem exists in with versions 3.1.6 and 3.1.7

OlgaMaciaszek commented 7 months ago

I see the failing test. I'm just not sure in which scenario you would actually call anything with lb: scheme in it clients (other than just using the lb scheme in Gateway setup, which works fine, as in the linked sample). Please indicate what the scenario look like. If the only issue is that you can't do a call using lb scheme from a load-balanced WebClient, this is not a real production issue, since you would do calls using http or https scheme, so you would use the load-balanced WebClient and execute your request at [http/https]://[serviceId]/[path]. You change the host:port data for the serviceId, but the scheme remains the same as you'd normally use for calling that instance without load-balancing.

snussbaumer commented 7 months ago

Real production case : we inject the spring cloud gateway route properties, loop over all the routes, extract the uri of each route, make it distinct, and so end with a list like [lb://service-1, lb://service-2, etc...]. We then use a load balanced client to loop over all this services and call some api on each of these services.

I was not aware that "lb" was "spring cloud gateway" only, and not something in "spring cloud load balancer" directly. It seemed quite nice. We can strip the "lb" and replace it with "http(s)" easily.

Anyways, I only filled this bug because some else stumbled upon it, and it happened right after doing an upgrade of spring. Let's just close the issue then.

OlgaMaciaszek commented 7 months ago

Yes, it's not really a supported scheme for actual calls - just for Gateway config.

roussi commented 3 months ago

@OlgaMaciaszek If we can't use the load balanced webClient with lb "schema", how can we then hit the load balanced services urls ? is it only possible by injecting the SimpleDiscoveryProperties and then use the uri(s) of each service ? is this approach a production case ?

OlgaMaciaszek commented 3 months ago

@roussi I'm not entirely sure what you are trying to do. Please provide a minimal, complete, verifiable example that reproduces the issue and tag me in the comment - will take a look.