Skip to content

Commit 8682c71

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 d5ea49b commit 8682c71

File tree

2 files changed

+74
-12
lines changed

2 files changed

+74
-12
lines changed

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

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,12 @@
1818
import java.io.InputStreamReader;
1919
import java.io.Reader;
2020
import java.nio.Buffer;
21+
import java.nio.ByteBuffer;
2122
import java.nio.CharBuffer;
2223
import java.nio.charset.Charset;
2324
import java.util.Collection;
2425
import java.util.Map;
26+
import java.util.Optional;
2527
import java.util.regex.Matcher;
2628
import java.util.regex.Pattern;
2729
import static feign.Util.UTF_8;
@@ -37,7 +39,7 @@ public class FeignException extends RuntimeException {
3739
"request should not be null";
3840
private static final long serialVersionUID = 0;
3941
private int status;
40-
private byte[] content;
42+
private byte[] responseBody;
4143
private Request request;
4244

4345
protected FeignException(int status, String message, Throwable cause) {
@@ -46,10 +48,10 @@ protected FeignException(int status, String message, Throwable cause) {
4648
this.request = null;
4749
}
4850

49-
protected FeignException(int status, String message, Throwable cause, byte[] content) {
51+
protected FeignException(int status, String message, Throwable cause, byte[] responseBody) {
5052
super(message, cause);
5153
this.status = status;
52-
this.content = content;
54+
this.responseBody = responseBody;
5355
this.request = null;
5456
}
5557

@@ -59,10 +61,10 @@ protected FeignException(int status, String message) {
5961
this.request = null;
6062
}
6163

62-
protected FeignException(int status, String message, byte[] content) {
64+
protected FeignException(int status, String message, byte[] responseBody) {
6365
super(message);
6466
this.status = status;
65-
this.content = content;
67+
this.responseBody = responseBody;
6668
this.request = null;
6769
}
6870

@@ -73,10 +75,10 @@ protected FeignException(int status, String message, Request request, Throwable
7375
}
7476

7577
protected FeignException(int status, String message, Request request, Throwable cause,
76-
byte[] content) {
78+
byte[] responseBody) {
7779
super(message, cause);
7880
this.status = status;
79-
this.content = content;
81+
this.responseBody = responseBody;
8082
this.request = checkRequestNotNull(request);
8183
}
8284

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

89-
protected FeignException(int status, String message, Request request, byte[] content) {
91+
protected FeignException(int status, String message, Request request, byte[] responseBody) {
9092
super(message);
9193
this.status = status;
92-
this.content = content;
94+
this.responseBody = responseBody;
9395
this.request = checkRequestNotNull(request);
9496
}
9597

@@ -101,8 +103,27 @@ public int status() {
101103
return this.status;
102104
}
103105

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

108129
public Request request() {
@@ -114,8 +135,8 @@ public boolean hasRequest() {
114135
}
115136

116137
public String contentUTF8() {
117-
if (content != null) {
118-
return new String(content, UTF_8);
138+
if (responseBody != null) {
139+
return new String(responseBody, UTF_8);
119140
} else {
120141
return "";
121142
}

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

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,50 @@
1414
package feign;
1515

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

1822
public class FeignExceptionTest {
1923

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

0 commit comments

Comments
 (0)