Affects: 5.3.22

webClient
.get()
.uri(orderPreviewFunction(request))
.header(HeaderConstants.Middleware.CALL_ID, callIdProvider.callId())
.header(HttpHeaders.CONTENT_TYPE, MediaType.APPLICATION_JSON_VALUE)
.accept(MediaType.APPLICATION_JSON)
.retrieve()
.toEntity(OrderPreviewsListReply.class) // let's say decoding failed, because response didn't match format of this class
.doOnError(WebClientResponseException.class, e -> {
    System.out.println(e.getResponseBodyAsString()); // returns "" (empty String)
});

Some people are mentioning that this functionality was broken by 24ebb5e.

The whole purpose of this exception is to be able to get some information (like response body) in case of response processing failure, so it's makes absolutely no sense that it returns an empty String in this situation.

Comment From: rstoyanchev

Have you confirmed a previous version where this worked? Generally, I don't see how it could since the response body can only be read once, and in this scenario it was already partially read before a decoding failure occurred.

Comment From: Gieted

I haven't checked that, but It's very likely it was working before the change I've mentioned in my bug report. See this.

Comment From: rstoyanchev

As I said the body can only be read once, so if we started decoding it and failed, there is no way to re-read again. The issue that you're referring is a functionally neutral refactoring. The SO post linked does not mention anything about a response that's already partially consumed so probably related but not the same scenario.

I'm closing this as there is not much we can do.

Comment From: Gieted

@rstoyanchev What you mean by

there is not much we can do

???

Even if it's not a regression problem it still is a bug, because just as I said, it makes absolutely no sense, that you cannot get the response as text in this scenario.

WebClient's internal implementation shouldn't consume the response when decoding fails.

This way, this exception would be actually useful. Right now, the behavior of .getResponseBodyAsString() is inconsistent, because it sometimes returns the response and sometimes nothing (depending on what caused the exception).

Comment From: rstoyanchev

WebClient's internal implementation shouldn't consume the response when decoding fails.

I'm not sure I understand what you are suggesting here. Unless the response is consumed, how can we know decoding will fail?

Comment From: Gieted

I'm not sure I understand what you are suggesting here. Unless the response is consumed, how can we know decoding will fail?

You can code it however you want, you can for example save it in some temp buffer and discard it once decoding fully successes.

I'm saying that it doesn't make sense to consume response in case of error from an API design perspective.

Comment From: Gieted

@rstoyanchev To be honest, it's the first time I've spent my time reporting an issue (which I personally don't even care about) and it was just completely ignored. I seriously don't have any idea why you're treating contributors like that.

Comment From: rstoyanchev

I'm not ignoring. It's just that what you're saying doesn't make sense.

Comment From: Gieted

What doesn't make sense?

Our discussion TLDR: You should change the implementation, so the response can be read in case of an error. But you can read response only once. But I'm not reading the response, you're doing it internally inside your code. Sorry, there's nothing we can do. You can just change your code!

Edit: I'm technically reading the response via .toEntity(), but the call never successes, because the decoding fails. I'm saying that the response body should not be consumed in such case.

Comment From: rstoyanchev

But I'm not reading the response, you're doing it internally inside your code.

It doesn't matter who is reading the response. It doesn't change the fact you can't know if the response is well formed or not, and will lead to a parse error until you actually read the response, and if you can only read it once, there is no way to back out. Technically, the response content could be buffered allowing multiple reads, but that means using more memory and is not something we would enable be enabled by default.

You can just change your code!

I can't make changes if I don't see what changes can be made.