Skip to content

Commit 5aa576f

Browse files
committed
Register status handler exceptions in observations
Prior to this commit, `RestClientException` thrown by status handlers would not be registered as observation errors. This commit ensures that such exceptions are first caught, registered in the observation and rethrown as expected. Closes gh-32575
1 parent 0268180 commit 5aa576f

File tree

2 files changed

+26
-7
lines changed

2 files changed

+26
-7
lines changed

spring-web/src/main/java/org/springframework/web/client/DefaultRestClient.java

+9-3
Original file line numberDiff line numberDiff line change
@@ -228,13 +228,13 @@ private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runna
228228
}
229229
throw unknownContentTypeException;
230230
}
231-
catch (UncheckedIOException | IOException | HttpMessageNotReadableException ex) {
231+
catch (UncheckedIOException | IOException | HttpMessageNotReadableException exc) {
232232
Throwable cause;
233-
if (ex instanceof UncheckedIOException uncheckedIOException) {
233+
if (exc instanceof UncheckedIOException uncheckedIOException) {
234234
cause = uncheckedIOException.getCause();
235235
}
236236
else {
237-
cause = ex;
237+
cause = exc;
238238
}
239239
RestClientException restClientException = new RestClientException("Error while extracting response for type [" +
240240
ResolvableType.forType(bodyType) + "] and content type [" + contentType + "]", cause);
@@ -243,6 +243,12 @@ private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runna
243243
}
244244
throw restClientException;
245245
}
246+
catch (RestClientException restClientException) {
247+
if (observation != null) {
248+
observation.error(restClientException);
249+
}
250+
throw restClientException;
251+
}
246252
finally {
247253
if (observation != null) {
248254
observation.stop();

spring-web/src/test/java/org/springframework/web/client/RestClientObservationTests.java

+17-4
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ class RestClientObservationTests {
6666

6767
private final ClientHttpResponse response = mock();
6868

69-
private final ResponseErrorHandler errorHandler = mock();
70-
7169
private final HttpMessageConverter<String> converter = mock();
7270

7371
private RestClient client;
@@ -79,7 +77,6 @@ void setupEach() {
7977
this.client = RestClient.builder()
8078
.messageConverters(converters -> converters.add(0, this.converter))
8179
.requestFactory(this.requestFactory)
82-
.defaultStatusHandler(this.errorHandler)
8380
.observationRegistry(this.observationRegistry)
8481
.build();
8582
this.observationRegistry.observationConfig().observationHandler(new ContextAssertionObservationHandler());
@@ -122,6 +119,10 @@ void shouldContributeSuccessOutcome() throws Exception {
122119

123120
@Test
124121
void shouldContributeServerErrorOutcome() throws Exception {
122+
ResponseErrorHandler errorHandler = mock();
123+
given(errorHandler.hasError(response)).willReturn(true);
124+
this.client = this.client.mutate().defaultStatusHandler(errorHandler).build();
125+
125126
String url = "https://example.org";
126127
mockSentRequest(GET, url);
127128
mockResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR);
@@ -201,6 +202,19 @@ void shouldAddUnknownContentTypeErrorAsException() throws Exception {
201202
assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "UnknownContentTypeException");
202203
}
203204

205+
@Test
206+
void shouldAddRestClientExceptionAsError() throws Exception {
207+
String url = "https://example.org";
208+
mockSentRequest(GET, url);
209+
mockResponseStatus(HttpStatus.NOT_FOUND);
210+
mockResponseBody("Not Found", MediaType.TEXT_HTML);
211+
212+
assertThatExceptionOfType(RestClientException.class).isThrownBy(() ->
213+
client.get().uri(url).retrieve().toEntity(String.class));
214+
215+
assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "NotFound");
216+
}
217+
204218
@Test
205219
void registerObservationWhenReadingBody() throws Exception {
206220
mockSentRequest(GET, "https://example.org");
@@ -239,7 +253,6 @@ private void mockSentRequest(HttpMethod method, String uri, HttpHeaders requestH
239253

240254
private void mockResponseStatus(HttpStatus responseStatus) throws Exception {
241255
given(request.execute()).willReturn(response);
242-
given(errorHandler.hasError(response)).willReturn(responseStatus.isError());
243256
given(response.getStatusCode()).willReturn(responseStatus);
244257
given(response.getStatusText()).willReturn(responseStatus.getReasonPhrase());
245258
}

0 commit comments

Comments
 (0)