Skip to content

Commit acf326d

Browse files
authored
Clarify availability of a Response Body in FeignException (#1149)
Fixes #920 FeignException may contain the data from the response if the response is available and contains data. However, the method `content` is ambiguious and does not reveal it's intent. User's have expressed confusion as to if it is for the Request or the Response. This change adds a new method `responseBody` to addresse this. Use of content is now `@deprecated`.
1 parent dd1d6cf commit acf326d

File tree

2 files changed

+82
-12
lines changed

2 files changed

+82
-12
lines changed

core/src/main/java/feign/FeignException.java

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,12 @@
2222
import java.io.InputStreamReader;
2323
import java.io.Reader;
2424
import java.nio.Buffer;
25+
import java.nio.ByteBuffer;
2526
import java.nio.CharBuffer;
2627
import java.nio.charset.Charset;
2728
import java.util.Collection;
2829
import java.util.Map;
30+
import java.util.Optional;
2931
import java.util.regex.Matcher;
3032
import java.util.regex.Pattern;
3133

@@ -36,7 +38,7 @@ public class FeignException extends RuntimeException {
3638
"request should not be null";
3739
private static final long serialVersionUID = 0;
3840
private int status;
39-
private byte[] content;
41+
private byte[] responseBody;
4042
private Request request;
4143

4244
protected FeignException(int status, String message, Throwable cause) {
@@ -45,10 +47,10 @@ protected FeignException(int status, String message, Throwable cause) {
4547
this.request = null;
4648
}
4749

48-
protected FeignException(int status, String message, Throwable cause, byte[] content) {
50+
protected FeignException(int status, String message, Throwable cause, byte[] responseBody) {
4951
super(message, cause);
5052
this.status = status;
51-
this.content = content;
53+
this.responseBody = responseBody;
5254
this.request = null;
5355
}
5456

@@ -58,10 +60,10 @@ protected FeignException(int status, String message) {
5860
this.request = null;
5961
}
6062

61-
protected FeignException(int status, String message, byte[] content) {
63+
protected FeignException(int status, String message, byte[] responseBody) {
6264
super(message);
6365
this.status = status;
64-
this.content = content;
66+
this.responseBody = responseBody;
6567
this.request = null;
6668
}
6769

@@ -72,10 +74,10 @@ protected FeignException(int status, String message, Request request, Throwable
7274
}
7375

7476
protected FeignException(
75-
int status, String message, Request request, Throwable cause, byte[] content) {
77+
int status, String message, Request request, Throwable cause, byte[] responseBody) {
7678
super(message, cause);
7779
this.status = status;
78-
this.content = content;
80+
this.responseBody = responseBody;
7981
this.request = checkRequestNotNull(request);
8082
}
8183

@@ -85,10 +87,10 @@ protected FeignException(int status, String message, Request request) {
8587
this.request = checkRequestNotNull(request);
8688
}
8789

88-
protected FeignException(int status, String message, Request request, byte[] content) {
90+
protected FeignException(int status, String message, Request request, byte[] responseBody) {
8991
super(message);
9092
this.status = status;
91-
this.content = content;
93+
this.responseBody = responseBody;
9294
this.request = checkRequestNotNull(request);
9395
}
9496

@@ -100,8 +102,27 @@ public int status() {
100102
return this.status;
101103
}
102104

105+
/**
106+
* The Response Body, if present.
107+
*
108+
* @return the body of the response.
109+
* @deprecated use {@link #responseBody()} instead.
110+
*/
111+
@Deprecated
103112
public byte[] content() {
104-
return this.content;
113+
return this.responseBody;
114+
}
115+
116+
/**
117+
* The Response body.
118+
*
119+
* @return an Optional wrapping the response body.
120+
*/
121+
public Optional<ByteBuffer> responseBody() {
122+
if (this.responseBody == null) {
123+
return Optional.empty();
124+
}
125+
return Optional.of(ByteBuffer.wrap(this.responseBody));
105126
}
106127

107128
public Request request() {
@@ -113,8 +134,8 @@ public boolean hasRequest() {
113134
}
114135

115136
public String contentUTF8() {
116-
if (content != null) {
117-
return new String(content, UTF_8);
137+
if (responseBody != null) {
138+
return new String(responseBody, UTF_8);
118139
} else {
119140
return "";
120141
}

core/src/test/java/feign/FeignExceptionTest.java

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,59 @@
1313
*/
1414
package feign;
1515

16+
import static org.assertj.core.api.Assertions.assertThat;
17+
18+
import java.io.IOException;
19+
import java.nio.charset.StandardCharsets;
20+
import java.util.Collections;
1621
import org.junit.Test;
1722

1823
public class FeignExceptionTest {
1924

25+
@Test
26+
public void canCreateWithRequestAndResponse() {
27+
Request request =
28+
Request.create(
29+
Request.HttpMethod.GET,
30+
"/home",
31+
Collections.emptyMap(),
32+
"data".getBytes(StandardCharsets.UTF_8),
33+
StandardCharsets.UTF_8,
34+
null);
35+
36+
Response response =
37+
Response.builder()
38+
.status(400)
39+
.body("response".getBytes(StandardCharsets.UTF_8))
40+
.request(request)
41+
.build();
42+
43+
FeignException exception =
44+
FeignException.errorReading(request, response, new IOException("socket closed"));
45+
assertThat(exception.responseBody()).isNotEmpty();
46+
assertThat(exception.hasRequest()).isTrue();
47+
assertThat(exception.request()).isNotNull();
48+
}
49+
50+
@Test
51+
public void canCreateWithRequestOnly() {
52+
Request request =
53+
Request.create(
54+
Request.HttpMethod.GET,
55+
"/home",
56+
Collections.emptyMap(),
57+
"data".getBytes(StandardCharsets.UTF_8),
58+
StandardCharsets.UTF_8,
59+
null);
60+
61+
FeignException exception =
62+
FeignException.errorExecuting(request, new IOException("connection timeout"));
63+
assertThat(exception.responseBody()).isEmpty();
64+
assertThat(exception.content()).isNullOrEmpty();
65+
assertThat(exception.hasRequest()).isTrue();
66+
assertThat(exception.request()).isNotNull();
67+
}
68+
2069
@Test(expected = NullPointerException.class)
2170
public void nullRequestShouldThrowNPEwThrowable() {
2271
new Derived(404, "message", null, new Throwable());

0 commit comments

Comments
 (0)