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

Extend WebClient with createException(HttpStatus code) #24126

Closed
membersound opened this issue Dec 3, 2019 · 9 comments
Closed

Extend WebClient with createException(HttpStatus code) #24126

membersound opened this issue Dec 3, 2019 · 9 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement

Comments

@membersound
Copy link

membersound commented Dec 3, 2019

It would be nice if you could extend WebClient with createException(HttpStatus code).

Because eg I have an external webservice that I have no control of, and it always returns 200 OK EVEN in case of error responses in body and an error hint in the header.

So when I evaluate the response with a ExchangeFilterFunction, I might want to return a clientResponse.createException(BAD_GATEWAY) based on the received content.

This is impossible by simply calling clientResponse.createException(), which would create a json exception, BUT with status 200 OK proxied through!

In my case, I'd like to write as follows:

private static ExchangeFilterFunction errorHandler() {
	return ExchangeFilterFunction.ofResponseProcessor(clientResponse -> {
                  HttpHeaders headers = clientResponse.headers().asHttpHeaders();
                  return (headers.getFirst("error").equals("true")) 
			? clientResponse.createException(BAD_GATEWAY).flatMap(ex -> Mono.error(ex)) //this is not possible atm
			: Mono.just(clientResponse);
	});
}
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 3, 2019
@poutsma poutsma added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement labels Dec 6, 2019
@poutsma poutsma self-assigned this Dec 6, 2019
@poutsma
Copy link
Contributor

poutsma commented Dec 6, 2019

I think you should be able to achieve your goal by using the builder, like so:

if (headers.getFirst("error").equals("true")) {
  return ClientResponse.from(clientResponse)
    .statusCode(BAD_GATEWAY)
    .build()
    .createException()
    .flatMap(ex -> Mono.error(ex));
}
else {
  return Mono.just(clientResponse);
}

@poutsma poutsma added the status: waiting-for-feedback We need additional information before we can continue label Dec 6, 2019
@rstoyanchev
Copy link
Contributor

Don't you also need to set the body?

ClientResponse.from(clientResponse)
    .statusCode(BAD_GATEWAY)
    .body(clientResponse.bodyToFlux(DataBuffer.class))
    .build()
    .createException()
    .flatMap(ex -> Mono.error(ex));

@poutsma
Copy link
Contributor

poutsma commented Dec 12, 2019

Good point, I forgot that from does not copy the body.

@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Dec 13, 2019
@membersound
Copy link
Author

This is great, but how could I add a custom error text into the WebClientResponseException created with this builder?

@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 status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Dec 16, 2019
@membersound
Copy link
Author

membersound commented Feb 17, 2020

It would still be nice if one could just supply a custom statusCode during createException() so that the exception generating code is inherited from ClientResponse, but simply the code is changed. Because bugs are likely being introduced by eg forgetting to set the body, as seen above.

@rstoyanchev
Copy link
Contributor

rstoyanchev commented Feb 20, 2020

As an API I'm not keen on exposing such a method at this level. It is an exceptional situation, not something that should be a common case. Furthermore it leads down a path of having more overloaded methods. You've already mentioned a custom error text in addition to the status code, and even for the status code, it could be HttpStatus or a raw status value.

ClientResponse provides a method to create an exception from its internal state. If necessary you can then map it to a different exception and change any of the bits:

response.createException().map(ex -> 
        new WebClientResponseException(
                502, "custom status", ex.getHeaders(), ex.getResponseBodyAsByteArray(), UTF_8))

This gives you full control over the mapping and is more readable in the sense that it's clear what's being done vs a createException(HttpStatus) which raises questions.

The only small thing here is that WebClientResponseException doesn't expose its responseCharset field. This is something we can correct.

@rstoyanchev rstoyanchev assigned rstoyanchev and unassigned poutsma Feb 20, 2020
@rstoyanchev rstoyanchev added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Feb 20, 2020
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 27, 2020
@spring-projects-issues
Copy link
Collaborator

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.

@spring-projects-issues spring-projects-issues removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 5, 2020
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) type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants