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

Improve handling of unknown status codes by WebClient [SPR-16819] #21359

Closed
spring-issuemaster opened this issue May 13, 2018 · 2 comments
Assignees
Milestone

Comments

@spring-issuemaster
Copy link
Collaborator

@spring-issuemaster spring-issuemaster commented May 13, 2018

Denys Ivano opened SPR-16819 and commented

Currently, there are some issues with using WebClient for calling HTTP services which return unknown status codes (couldn't be resolved through HttpStatus enum).

  1. ClientResponse interface doesn't provide an opportunity to obtain status code value as int. Using exchange() method is safe with unknown status codes, but the raw status code value couldn't be obtained without reflexion. In 5.0.6 the method getRawStatusCode() was introduced in ClientHttpResponse (see #21289). DefaultClientResponse (default implementation of ClientResponse) wraps ClientHttpResponse, so the status code can be obtained by introducing new method in reactive ClientResponse interface.

  2. It's not possible to use retrieve() with unknown status codes because it always throws IllegalArgumentException:

java.lang.IllegalArgumentException: No matching constant for [999]
	at org.springframework.http.HttpStatus.valueOf(HttpStatus.java:520)
	at org.springframework.http.client.reactive.ReactorClientHttpResponse.getStatusCode(ReactorClientHttpResponse.java:71)
	at org.springframework.web.reactive.function.client.DefaultClientResponse.statusCode(DefaultClientResponse.java:72)
	at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultResponseSpec.lambda$bodyToPublisher$4(DefaultWebClient.java:438)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
	at java.util.ArrayList$ArrayListSpliterator.tryAdvance(ArrayList.java:1359)
	at java.util.stream.ReferencePipeline.forEachWithCancel(ReferencePipeline.java:126)
	at java.util.stream.AbstractPipeline.copyIntoWithCancel(AbstractPipeline.java:498)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:485)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.FindOps$FindOp.evaluateSequential(FindOps.java:152)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.findFirst(ReferencePipeline.java:464)
	at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultResponseSpec.bodyToPublisher(DefaultWebClient.java:439)
	at org.springframework.web.reactive.function.client.DefaultWebClient$DefaultResponseSpec.lambda$bodyToMono$0(DefaultWebClient.java:400)

That's because the default implementation uses error handling based on HttpStatus enum value. It would be a good idea to use raw status value instead and provide an opportunity to register error handlers based on raw status code value.

I've submited PR 1829 which is going to resolve these issues.


Affects: 5.0.6

Issue Links:

  • #20529 RestTemplate doesn't consistently tolerate unknown HTTP status codes
  • #20104 webflux handler fails in case invalid method was requested
  • #21289 No support for non-standard HTTP status codes in reactive ClientHttpResponse

Referenced from: pull request #1829, and commits 038af9a

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented May 14, 2018

Juergen Hoeller commented

Since WebClient hasn't been designed for non-standard HTTP status codes or non-standard HTTP methods yet, let's rather make this a 5.1 topic and aim for a comprehensive revision there... leaving enough time for revisiting some of the design compromises to make.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

@spring-issuemaster spring-issuemaster commented Jul 12, 2018

Brian Clozel commented

This is now fixed on master.

I've cherry-picked and reduced the initial pull request, which was in my opinion taking too far the support of unknown status codes. While we want the WebClient to be flexible and to tolerate such status codes, we don't want to promote such usage to many public methods on high-level parts of the API.

I've added details about the expected behavior in the commit message directly.

Thanks a lot for your contribution Denys Ivano!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.