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

Add method to ClientResponse that returns Mono terminating with createException #27637

Closed
jwChung opened this issue Nov 4, 2021 · 4 comments
Closed
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: feedback-provided Feedback has been provided type: enhancement A general enhancement
Milestone

Comments

@jwChung
Copy link

jwChung commented Nov 4, 2021

The method ClientResponse.createException() returns the Mono<WebClientResponseException> type. Mono<Exception> feels like Either<Exception, Exception>. I think that the Mono<RESULT> type is actually needed. So, what if there is the createError method that returns the Mono<RESULT> type in the ClientResponse interface?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 4, 2021
@rstoyanchev rstoyanchev added the in: web Issues in web modules (web, webmvc, webflux, websocket) label Nov 10, 2021
@poutsma
Copy link
Contributor

poutsma commented Nov 19, 2021

I can see the point for having a different exception, but can't you just map the WebClientResponseException to the type you'd like?

@snicoll snicoll changed the title Suggestion - ClientResponse.createError to return Mono<RESULT> ClientResponse.createError to return Mono<RESULT> Nov 19, 2021
@rstoyanchev rstoyanchev added the status: waiting-for-feedback We need additional information before we can continue label Nov 19, 2021
@jwChung
Copy link
Author

jwChung commented Nov 26, 2021

@poutsma Isn't Mono<T>.onErrorMap/onErrorResume for that?

In many cases, a returning value of createException() is changed to Mono<RESULT> in the following way.

Mono<RESULT> mono = clientResponse.createException().flatMap(e -> Mono.error(new MyException("message", e)));

I wish I could just write it down as the following.

Mono<RESULT> mono = clientResponse.createError().onErrorMap(e -> new MyException("message", e));

@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 Nov 26, 2021
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Nov 30, 2021

There is a related example in #27645 that doesn't even involve changing the exception. If you use createException from exchangeToMono and exchangeToFlux we have it documented as returning Mono.error(response.createException()) but actually you need response.createException().flatMap(Mono::error) and that's only to be able to match the generic type of the result. It could also be response.createException().cast(...) but either way it's inconvenient.

It would make sense to align createException with Mono#error in terms of being able to cast to anything. After all an error signal switches from some type T to an Exception so this is to be expected. WDYT @poutsma?

@rstoyanchev rstoyanchev added this to the Triage Queue milestone Nov 30, 2021
@rstoyanchev rstoyanchev added the type: enhancement A general enhancement label Nov 30, 2021
@poutsma poutsma removed the status: waiting-for-triage An issue we've not yet triaged or decided on label Dec 1, 2021
@poutsma poutsma modified the milestones: Triage Queue, 6.0 M1 Dec 1, 2021
@poutsma
Copy link
Contributor

poutsma commented Dec 1, 2021

Personally I prefer the explicitness that the Mono<WebClientResponseException> return type offers, as opposed to a generic type. In the latter case, reading the javadoc of the method is pretty much required to see what it does, and that is not true for the former, which pretty much does what you expect from createException.

That said, by looking at #27645, I realise that the current signature is not ideal either and requires the use of a flatMap, which is not obvious. There is no reason against having both createException and createError, so I will add it.

@poutsma poutsma changed the title ClientResponse.createError to return Mono<RESULT> Add method to ClientResponse that returns Mono terminating with createException Dec 1, 2021
@poutsma poutsma closed this as completed in 7794606 Dec 1, 2021
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: feedback-provided Feedback has been provided type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants