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

Possible bug/regression in WebClient on spring-webflux 6.1.3 inherited by Spring Boot 3.2.2 #32096

Closed
jonathanmdr opened this issue Jan 23, 2024 · 3 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@jonathanmdr
Copy link

jonathanmdr commented Jan 23, 2024

Stack:

  • Java 21
  • Spring Boot 3.2.2
  • Spring Webflux 6.1.3 (inherited by Spring Boot Webflux Starter)

I have a WebClient to make requests with query params:
e.g.

// Simulate a scenario when server returns a 503

public List<MyResponse> findAnyData(final String myParam) { // The String myParam contains "FooBar" value
        try {
            return this.webClient.get()
                .uri(uriBuilder -> uriBuilder.path("/v1/any-api").queryParam("myParam", myParam).build())
                .retrieve()
                .bodyToFlux(MyResponse.class)
                .collectList()
                .block();
        } catch (final WebClientResponseException ex) { // 503 is captured here
            log.error(
                "My client failed with HTTP status: '{}' and message: '{}'",
                ex.getStatusCode(),
                ex.getMessage(), // at this point returns: 503 Service Unavailable from GET http://my-server-example.com/v1/any-api?myParam=FooBar
                ex
            );
            return Collections.emptyList();
        }
}

In the previous releases of Spring Boot version <= 3.2.1 (Spring Webflux <= 6.1.2) I received the exact message commented in the code, but after the update project to Spring Boot version 3.2.2 (Spring Webflux 6.1.3) the message doesn't contain the same message, it presents the: 503 Service Unavailable from GET http://my-server-example.com/v1/any-api without used query params.

In my integration test with MockWebServer, when I assert the request path I obtain:

  • previous versions: /v1/any-api?myParam=FooBar
  • latest version: /v1/any-api

e.g.

final RecordedRequest recordedRequest = mockWebServer.takeRequest();
assertThat(recordedRequest.getHeaders()).containsAll(expectedHeaders);
assertThat(recordedRequest.getMethod()).isEqualTo(expectedHttpMethod);
assertThat(recordedRequest.getPath()).isEqualTo(expectedPath); // at this point

Possible change reference: 2b4ffe0

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jan 23, 2024
@jonathanmdr jonathanmdr changed the title Possible bug in WebClient on spring-webflux 6.1.3 inherited by Spring Boot 3.2.2 Possible bug/regression in WebClient on spring-webflux 6.1.3 inherited by Spring Boot 3.2.2 Jan 23, 2024
@rstoyanchev rstoyanchev self-assigned this Jan 24, 2024
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Jan 24, 2024
@rstoyanchev
Copy link
Contributor

Thanks for the report.

There was a change in preparation of the URI_TEMPLATE attribute to include the baseUrl, but that should not affect the uri(Function<UriBuilder, URI>) where there is no URI template. Also, we have a test that calls this with a query parameter.

There is something less obvious going on. You can check if the behavior changes by varying the Spring Framework version only to see if it is potentially another dependency upgrade. If you can't narrow it down, please, provide more details on how to reproduce, a sample we can run would be best.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Jan 24, 2024
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jan 24, 2024

I've been able to find out more. The actual URL used is fine as confirmed by the logs and also by MockWebServer with server.takeRequest().getRequestUrl(). It's just the exception message that excludes the query, but that's intentional and a result of #31992 that in turn completes the work started in #29148. In short, we intentionally avoid including the query in anything that may result in logging sensitive information.

@jonathanmdr
Copy link
Author

Thank you for your feedback @rstoyanchev

The mentioned issues helped me to understand the actual behavior after changes and It looks ok to me. I adjusted all my integration test scenarios to consider this.

@jhoeller jhoeller added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Jan 24, 2024
@bclozel bclozel added status: invalid An issue that we don't feel is valid and removed status: feedback-provided Feedback has been provided labels Jan 24, 2024
@bclozel bclozel closed this as not planned Won't fix, can't repro, duplicate, stale Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid
Projects
None yet
Development

No branches or pull requests

5 participants