Skip to content

Commit 0893cf9

Browse files
bclozelbclozel
committed
Merge branch '6.1.x'
2 parents 62a1b55 + da4547a commit 0893cf9

File tree

2 files changed

+102
-19
lines changed

2 files changed

+102
-19
lines changed

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

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ public Builder mutate() {
196196
@Nullable
197197
@SuppressWarnings({"rawtypes", "unchecked"})
198198
private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runnable callback, Type bodyType,
199-
Class<T> bodyClass) {
199+
Class<T> bodyClass, @Nullable Observation observation) {
200200

201201
MediaType contentType = getContentType(clientResponse);
202202

@@ -224,9 +224,13 @@ private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runna
224224
return (T) messageConverter.read((Class)bodyClass, responseWrapper);
225225
}
226226
}
227-
throw new UnknownContentTypeException(bodyType, contentType,
227+
UnknownContentTypeException unknownContentTypeException = new UnknownContentTypeException(bodyType, contentType,
228228
responseWrapper.getStatusCode(), responseWrapper.getStatusText(),
229229
responseWrapper.getHeaders(), RestClientUtils.getBody(responseWrapper));
230+
if (observation != null) {
231+
observation.error(unknownContentTypeException);
232+
}
233+
throw unknownContentTypeException;
230234
}
231235
catch (UncheckedIOException | IOException | HttpMessageNotReadableException ex) {
232236
Throwable cause;
@@ -236,8 +240,17 @@ private <T> T readWithMessageConverters(ClientHttpResponse clientResponse, Runna
236240
else {
237241
cause = ex;
238242
}
239-
throw new RestClientException("Error while extracting response for type [" +
243+
RestClientException restClientException = new RestClientException("Error while extracting response for type [" +
240244
ResolvableType.forType(bodyType) + "] and content type [" + contentType + "]", cause);
245+
if (observation != null) {
246+
observation.error(restClientException);
247+
}
248+
throw restClientException;
249+
}
250+
finally {
251+
if (observation != null) {
252+
observation.stop();
253+
}
241254
}
242255
}
243256

@@ -476,28 +489,30 @@ private <T> T exchangeInternal(ExchangeFunction<T> exchangeFunction, boolean clo
476489
}
477490
clientResponse = clientRequest.execute();
478491
observationContext.setResponse(clientResponse);
479-
ConvertibleClientHttpResponse convertibleWrapper = new DefaultConvertibleClientHttpResponse(clientResponse);
492+
ConvertibleClientHttpResponse convertibleWrapper = new DefaultConvertibleClientHttpResponse(clientResponse, observation);
480493
return exchangeFunction.exchange(clientRequest, convertibleWrapper);
481494
}
482495
catch (IOException ex) {
483496
ResourceAccessException resourceAccessException = createResourceAccessException(uri, this.httpMethod, ex);
484497
if (observation != null) {
485498
observation.error(resourceAccessException);
499+
observation.stop();
486500
}
487501
throw resourceAccessException;
488502
}
489503
catch (Throwable error) {
490504
if (observation != null) {
491505
observation.error(error);
506+
observation.stop();
492507
}
493508
throw error;
494509
}
495510
finally {
496511
if (close && clientResponse != null) {
497512
clientResponse.close();
498-
}
499-
if (observation != null) {
500-
observation.stop();
513+
if (observation != null) {
514+
observation.stop();
515+
}
501516
}
502517
}
503518
}
@@ -667,7 +682,7 @@ public ResponseEntity<Void> toBodilessEntity() {
667682
@Nullable
668683
private <T> T readBody(Type bodyType, Class<T> bodyClass) {
669684
return DefaultRestClient.this.readWithMessageConverters(this.clientResponse, this::applyStatusHandlers,
670-
bodyType, bodyClass);
685+
bodyType, bodyClass, getCurrentObservation());
671686

672687
}
673688

@@ -688,31 +703,43 @@ private void applyStatusHandlers() {
688703
throw new UncheckedIOException(ex);
689704
}
690705
}
706+
707+
@Nullable
708+
private Observation getCurrentObservation() {
709+
if (this.clientResponse instanceof DefaultConvertibleClientHttpResponse convertibleResponse) {
710+
return convertibleResponse.observation;
711+
}
712+
return null;
713+
}
714+
691715
}
692716

693717

694718
private class DefaultConvertibleClientHttpResponse implements RequestHeadersSpec.ConvertibleClientHttpResponse {
695719

696720
private final ClientHttpResponse delegate;
697721

722+
private final Observation observation;
723+
698724

699-
public DefaultConvertibleClientHttpResponse(ClientHttpResponse delegate) {
725+
public DefaultConvertibleClientHttpResponse(ClientHttpResponse delegate, Observation observation) {
700726
this.delegate = delegate;
727+
this.observation = observation;
701728
}
702729

703730

704731
@Nullable
705732
@Override
706733
public <T> T bodyTo(Class<T> bodyType) {
707-
return readWithMessageConverters(this.delegate, () -> {} , bodyType, bodyType);
734+
return readWithMessageConverters(this.delegate, () -> {} , bodyType, bodyType, this.observation);
708735
}
709736

710737
@Nullable
711738
@Override
712739
public <T> T bodyTo(ParameterizedTypeReference<T> bodyType) {
713740
Type type = bodyType.getType();
714741
Class<T> bodyClass = bodyClass(type);
715-
return readWithMessageConverters(this.delegate, () -> {} , type, bodyClass);
742+
return readWithMessageConverters(this.delegate, () -> {}, type, bodyClass, this.observation);
716743
}
717744

718745
@Override
@@ -738,6 +765,7 @@ public String getStatusText() throws IOException {
738765
@Override
739766
public void close() {
740767
this.delegate.close();
768+
this.observation.stop();
741769
}
742770

743771
}

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

Lines changed: 63 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.io.ByteArrayInputStream;
2020
import java.io.IOException;
2121
import java.net.URI;
22+
import java.nio.charset.StandardCharsets;
2223
import java.util.Map;
2324
import java.util.UUID;
2425

@@ -40,6 +41,7 @@
4041
import org.springframework.http.client.observation.ClientRequestObservationConvention;
4142
import org.springframework.http.client.observation.DefaultClientRequestObservationConvention;
4243
import org.springframework.http.converter.HttpMessageConverter;
44+
import org.springframework.util.StreamUtils;
4345

4446
import static org.assertj.core.api.Assertions.assertThat;
4547
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
@@ -84,7 +86,7 @@ void setupEach() {
8486
}
8587

8688
@Test
87-
void executeVarArgsAddsUriTemplateAsKeyValue() throws Exception {
89+
void shouldContributeTemplateWhenUriVariables() throws Exception {
8890
mockSentRequest(GET, "https://example.com/hotels/42/bookings/21");
8991
mockResponseStatus(HttpStatus.OK);
9092

@@ -95,7 +97,7 @@ void executeVarArgsAddsUriTemplateAsKeyValue() throws Exception {
9597
}
9698

9799
@Test
98-
void executeArgsMapAddsUriTemplateAsKeyValue() throws Exception {
100+
void shouldContributeTemplateWhenMap() throws Exception {
99101
mockSentRequest(GET, "https://example.com/hotels/42/bookings/21");
100102
mockResponseStatus(HttpStatus.OK);
101103

@@ -107,9 +109,8 @@ void executeArgsMapAddsUriTemplateAsKeyValue() throws Exception {
107109
assertThatHttpObservation().hasLowCardinalityKeyValue("uri", "/hotels/{hotel}/bookings/{booking}");
108110
}
109111

110-
111112
@Test
112-
void executeAddsSuccessAsOutcome() throws Exception {
113+
void shouldContributeSuccessOutcome() throws Exception {
113114
mockSentRequest(GET, "https://example.org");
114115
mockResponseStatus(HttpStatus.OK);
115116
mockResponseBody("Hello World", MediaType.TEXT_PLAIN);
@@ -120,7 +121,7 @@ void executeAddsSuccessAsOutcome() throws Exception {
120121
}
121122

122123
@Test
123-
void executeAddsServerErrorAsOutcome() throws Exception {
124+
void shouldContributeServerErrorOutcome() throws Exception {
124125
String url = "https://example.org";
125126
mockSentRequest(GET, url);
126127
mockResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR);
@@ -134,7 +135,7 @@ void executeAddsServerErrorAsOutcome() throws Exception {
134135
}
135136

136137
@Test
137-
void executeAddsExceptionAsKeyValue() throws Exception {
138+
void shouldContributeDecodingError() throws Exception {
138139
mockSentRequest(POST, "https://example.org/resource");
139140
mockResponseStatus(HttpStatus.OK);
140141

@@ -150,7 +151,7 @@ void executeAddsExceptionAsKeyValue() throws Exception {
150151
}
151152

152153
@Test
153-
void executeWithIoExceptionAddsUnknownOutcome() throws Exception {
154+
void shouldContributeIOError() throws Exception {
154155
String url = "https://example.org/resource";
155156
mockSentRequest(GET, url);
156157
given(request.execute()).willThrow(new IOException("Socket failure"));
@@ -161,7 +162,7 @@ void executeWithIoExceptionAddsUnknownOutcome() throws Exception {
161162
}
162163

163164
@Test
164-
void executeWithCustomConventionUsesCustomObservationName() throws Exception {
165+
void shouldUseCustomConvention() throws Exception {
165166
ClientRequestObservationConvention observationConvention =
166167
new DefaultClientRequestObservationConvention("custom.requests");
167168
RestClient restClient = this.client.mutate().observationConvention(observationConvention).build();
@@ -174,6 +175,56 @@ void executeWithCustomConventionUsesCustomObservationName() throws Exception {
174175
.hasObservationWithNameEqualTo("custom.requests");
175176
}
176177

178+
@Test
179+
void shouldAddClientDecodingErrorAsException() throws Exception {
180+
String url = "https://example.org";
181+
mockSentRequest(GET, url);
182+
mockResponseStatus(HttpStatus.OK);
183+
mockResponseBody("INVALID", MediaType.APPLICATION_JSON);
184+
185+
assertThatExceptionOfType(RestClientException.class).isThrownBy(() ->
186+
client.get().uri(url).retrieve().body(User.class));
187+
188+
assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "RestClientException");
189+
}
190+
191+
@Test
192+
void shouldAddUnknownContentTypeErrorAsException() throws Exception {
193+
String url = "https://example.org";
194+
mockSentRequest(GET, url);
195+
mockResponseStatus(HttpStatus.OK);
196+
mockResponseBody("Not Found", MediaType.TEXT_HTML);
197+
198+
assertThatExceptionOfType(RestClientException.class).isThrownBy(() ->
199+
client.get().uri(url).retrieve().body(User.class));
200+
201+
assertThatHttpObservation().hasLowCardinalityKeyValue("exception", "UnknownContentTypeException");
202+
}
203+
204+
@Test
205+
void registerObservationWhenReadingBody() throws Exception {
206+
mockSentRequest(GET, "https://example.org");
207+
mockResponseStatus(HttpStatus.OK);
208+
mockResponseBody("Hello World", MediaType.TEXT_PLAIN);
209+
210+
client.get().uri("https://example.org").exchange((request, response) -> response.bodyTo(String.class));
211+
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS");
212+
}
213+
214+
@Test
215+
void registerObservationWhenReadingStream() throws Exception {
216+
mockSentRequest(GET, "https://example.org");
217+
mockResponseStatus(HttpStatus.OK);
218+
mockResponseBody("Hello World", MediaType.TEXT_PLAIN);
219+
220+
client.get().uri("https://example.org").exchange((request, response) -> {
221+
String result = StreamUtils.copyToString(response.getBody(), StandardCharsets.UTF_8);
222+
response.close();
223+
return result;
224+
}, false);
225+
assertThatHttpObservation().hasLowCardinalityKeyValue("outcome", "SUCCESS");
226+
}
227+
177228

178229
private void mockSentRequest(HttpMethod method, String uri) throws Exception {
179230
mockSentRequest(method, uri, new HttpHeaders());
@@ -220,4 +271,8 @@ public void onStart(ClientRequestObservationContext context) {
220271
}
221272
}
222273

274+
record User(String name) {
275+
276+
}
277+
223278
}

0 commit comments

Comments
 (0)