Closed pelepelin closed 1 month ago
Expectation is actually incorrect, because defaultStatusHandler
is default in the sense "applied to all exchanges" and not in the sense "overrides previous default".
However, the bottom line is the same: I see no proper method to remove default handling of 4xx/5xx statuses.
Thanks for the report. I agree that it's a little odd that the only way to disable the behavior is to implement an error handler that claims there is an error for every request. Looking at WebClient
and its statusHandlers
however, I can see that the same thing happens in DefaultResponseSpec
: https://github.com/spring-projects/spring-framework/blob/fc28926c1527bd82197ebbf3cec7679e772e56e6/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultWebClient.java#L549-L550
I think the difference here is that the status handler uses a predicate based on the status code and you can indeed disable everything by using a predicate that always match. Perhaps NoOpResponseErrorHandler
is named poorly, or we should change its implementation?
Wondering what the rest of the team thinks.
ResponseErrorHandler
was the error mechanism of the RestTemplate
. You could register one of these. The main error mechanism in RestClient is ResponseSpec.ErrorHandler
, and you can register any number of these each with a Predicate<HttpStatusCode>
that helps to decide which status handler to use. ResponseErrorHandler
is adapted to a status handler where the hasError
method used as the predicate.
That means a ResponseErrorHandler
has to return true for the responses it wants to handle. If it wants to handle all error responses, then it should return true for status 400 and above, which is what the default status handler at the end of the chain does. Ideally though register a statusHandler with a predicate and a ResponseSpec.ErrorHandler
. Maybe we can update the Javadoc to make all this more clear.
Sounds good. Probably the linked reference page could have an explicit example that in order to disable status handling one needs configuration like this (I have not checked)
RestClient restClient = RestClient.builder()
.defaultStatusHandler((statusCode) -> true, (request, response) -> {})
.build();
Thanks for the suggestion. I think the main goal here is to clarify that NoOpResponseErrorHandler
is meant to be used with the RestTemplate
and that it can be used with the RestClient
only to ease the migration of the former to the latter. With the Javadoc in place, I don't think there is a need of more content than that.
Affects: spring-web-6.1.11
Code
Let's pretend I don't want to handle status codes in the status handlers.
Expected
Default status handler is overridden, there should be no status code handling, no exception thrown.
Actual
Reason: while the built
RestClient
contains only a singleNoOpResponseErrorHandler
, aretrieve()
call creates anorg.springframework.web.client.DefaultRestClient.DefaultResponseSpec
which puts an additional defaultStatusHandler
at the end of the handlers list.https://github.com/spring-projects/spring-framework/blob/fc28926c1527bd82197ebbf3cec7679e772e56e6/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java#L651-L652
and the handlers are tried sequentially
https://github.com/spring-projects/spring-framework/blob/fc28926c1527bd82197ebbf3cec7679e772e56e6/spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java#L747-L752
So, it's only possible to override the default error handler but not possible to replace it.
Workaround
Implement a handler so it returns true from
hasError
but does nothing in the handler. This looks more like abuse than following a contract.