spring-cloud-function icon indicating copy to clipboard operation
spring-cloud-function copied to clipboard

Review use of WebClient exchange to ensure body is always consumed

Open rstoyanchev opened this issue 5 years ago • 3 comments

The exchange() method of WebClient allows more advanced usage with access to status and headers before the body is retrieved. It is also requires making sure the body is always consumed because otherwise the connection remains active and cannot be re-used.

For example this code https://github.com/spring-cloud/spring-cloud-function/blob/9434a68bd2b1d73fb3c0af38396e1ffee50a51e9/spring-cloud-function-web/src/main/java/org/springframework/cloud/function/web/source/HttpSupplier.java#L63-L82 exits on line 78 without doing anything to the body. Also what the code seems to be trying to do (delayed retry) is something that can be done more simply with the retry operators.

The retrieve() method of WebClient on the other hand is safe because the workflow requires choosing what to do with the body. For most cases there should be no reason not to prefer it over exchange. If you need to deal with the status you can use retrieve with onStatus handlers but again for the above the retry operator could be used to retry on WebClientResponseException which is raised on status > 400.

There maybe other places in the code too that have similar issues.

Some further references:

rstoyanchev avatar Mar 27 '20 14:03 rstoyanchev

The retry() idea sounds interesting. Where would you put it though? Would you need to call response.toEntity() and work with the result so you can base the retry on the HTTP status?

dsyer avatar Mar 27 '20 15:03 dsyer

Yes, after the request declaration, i.e. retrieve().bodyToMono(...).

rstoyanchev avatar Mar 27 '20 15:03 rstoyanchev

Actually never mind the part about retry. It's repeating rather. My bad.

rstoyanchev avatar Mar 27 '20 15:03 rstoyanchev