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

WebClientResponseException.getResponseBodyAsString() returns an empty String #30986

Closed
Gieted opened this issue Aug 3, 2023 · 10 comments
Closed
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: invalid An issue that we don't feel is valid

Comments

@Gieted
Copy link

Gieted commented Aug 3, 2023

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.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Aug 3, 2023
@sbrannen sbrannen added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Aug 3, 2023
@sbrannen sbrannen changed the title WebClientResponseException.getResponseBodyAsString() returns an empty String WebClientResponseException.getResponseBodyAsString() returns an empty String Aug 3, 2023
@rstoyanchev
Copy link
Contributor

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.

@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Sep 7, 2023
@Gieted
Copy link
Author

Gieted commented Sep 8, 2023

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.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Sep 8, 2023
@rstoyanchev
Copy link
Contributor

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.

@rstoyanchev rstoyanchev closed this as not planned Won't fix, can't repro, duplicate, stale Sep 19, 2023
@rstoyanchev rstoyanchev added status: invalid An issue that we don't feel is valid and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Sep 19, 2023
@Gieted
Copy link
Author

Gieted commented Sep 23, 2023

@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).

@rstoyanchev
Copy link
Contributor

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?

@Gieted
Copy link
Author

Gieted commented Oct 8, 2023

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.

@Gieted
Copy link
Author

Gieted commented Oct 31, 2023

@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.

@rstoyanchev
Copy link
Contributor

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

@Gieted
Copy link
Author

Gieted commented Nov 1, 2023

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.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Nov 2, 2023

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.

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

4 participants