Skip to content

Commit 149ccbd

Browse files
author
Adrian Cole
committed
Skips error handling when return type is Response
In unsuccessful scenarios, such as redirects, error handling converts a `Response` into an exception. When a user defines a return type as `Response`, we can assume they will want access to things like headers even in an error scenario. See OpenFeign#249
1 parent a1754a2 commit 149ccbd

6 files changed

Lines changed: 55 additions & 25 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
### Version 8.9
2+
* Skips error handling when return type is `Response`
3+
14
### Version 8.8
25
* Adds jackson-jaxb codec
36
* Bumps dependency versions for integrations

‎core/src/main/java/feign/SynchronousMethodHandler.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,16 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
103103
response =
104104
logger.logAndRebufferResponse(metadata.configKey(), logLevel, response, elapsedTime);
105105
}
106+
if (Response.class == metadata.returnType()) {
107+
if (response.body() == null) {
108+
return response;
109+
}
110+
// Ensure the response body is disconnected
111+
byte[] bodyData = Util.toByteArray(response.body().asInputStream());
112+
return Response.create(response.status(), response.reason(), response.headers(), bodyData);
113+
}
106114
if (response.status() >= 200 && response.status() < 300) {
107-
if (Response.class == metadata.returnType()) {
108-
if (response.body() == null) {
109-
return response;
110-
}
111-
// Ensure the response body is disconnected
112-
byte[] bodyData = Util.toByteArray(response.body().asInputStream());
113-
return Response
114-
.create(response.status(), response.reason(), response.headers(), bodyData);
115-
} else if (void.class == metadata.returnType()) {
115+
if (void.class == metadata.returnType()) {
116116
return null;
117117
} else {
118118
return decode(response);

‎core/src/test/java/feign/FeignTest.java‎

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222
import com.squareup.okhttp.mockwebserver.SocketPolicy;
2323
import com.squareup.okhttp.mockwebserver.MockWebServer;
2424

25+
import java.util.Collection;
26+
import java.util.LinkedHashMap;
2527
import okio.Buffer;
2628
import org.junit.Rule;
2729
import org.junit.Test;
@@ -328,11 +330,28 @@ public Exception decode(String methodKey, Response response)
328330
}
329331
}).target(TestInterface.class, "http://localhost:" + server.getPort());
330332

331-
api.response();
332-
api.response(); // if retryer instance was reused, this statement will throw an exception
333+
api.post();
334+
api.post(); // if retryer instance was reused, this statement will throw an exception
333335
assertEquals(4, server.getRequestCount());
334336
}
335337

338+
@Test
339+
public void whenReturnTypeIsResponseNoErrorHandling() {
340+
Map<String, Collection<String>> headers = new LinkedHashMap<String, Collection<String>>();
341+
headers.put("Location", Arrays.asList("http://bar.com"));
342+
final Response response = Response.create(302, "Found", headers, new byte[0]);
343+
344+
TestInterface api = Feign.builder()
345+
.client(new Client() { // fake client as Client.Default follows redirects.
346+
public Response execute(Request request, Request.Options options) {
347+
return response;
348+
}
349+
})
350+
.target(TestInterface.class, "http://localhost:" + server.getPort());
351+
352+
assertEquals(api.response().headers().get("Location"), Arrays.asList("http://bar.com"));
353+
}
354+
336355
private static class MockRetryer implements Retryer
337356
{
338357
boolean tripped;

‎core/src/test/java/feign/client/DefaultClientTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,7 @@ public boolean verify(String s, SSLSession sslSession) {
6363
public void parsesRequestAndResponse() throws IOException, InterruptedException {
6464
server.enqueue(new MockResponse().setBody("foo").addHeader("Foo: Bar"));
6565

66-
TestInterface
67-
api =
66+
TestInterface api =
6867
Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
6968

7069
Response response = api.post("foo");
@@ -86,15 +85,14 @@ public void parsesRequestAndResponse() throws IOException, InterruptedException
8685
@Test
8786
public void parsesErrorResponse() throws IOException, InterruptedException {
8887
thrown.expect(FeignException.class);
89-
thrown.expectMessage("status 500 reading TestInterface#post(String); content:\n" + "ARGHH");
88+
thrown.expectMessage("status 500 reading TestInterface#get(); content:\n" + "ARGHH");
9089

9190
server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
9291

93-
TestInterface
94-
api =
92+
TestInterface api =
9593
Feign.builder().target(TestInterface.class, "http://localhost:" + server.getPort());
9694

97-
api.post("foo");
95+
api.get();
9896
}
9997

10098
/**
@@ -191,6 +189,10 @@ interface TestInterface {
191189
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"})
192190
Response post(String body);
193191

192+
@RequestLine("GET /")
193+
@Headers("Accept: text/plain")
194+
String get();
195+
194196
@RequestLine("PATCH /")
195197
@Headers("Accept: text/plain")
196198
String patch();

‎httpclient/src/test/java/feign/httpclient/ApacheHttpClientTest.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,15 +69,15 @@ public void parsesRequestAndResponse() throws IOException, InterruptedException
6969
@Test
7070
public void parsesErrorResponse() throws IOException, InterruptedException {
7171
thrown.expect(FeignException.class);
72-
thrown.expectMessage("status 500 reading TestInterface#post(String); content:\n" + "ARGHH");
72+
thrown.expectMessage("status 500 reading TestInterface#get(); content:\n" + "ARGHH");
7373

7474
server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
7575

7676
TestInterface api = Feign.builder()
7777
.client(new ApacheHttpClient())
7878
.target(TestInterface.class, "http://localhost:" + server.getPort());
7979

80-
api.post("foo");
80+
api.get();
8181
}
8282

8383
@Test
@@ -138,6 +138,10 @@ interface TestInterface {
138138
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"})
139139
Response post(String body);
140140

141+
@RequestLine("GET /")
142+
@Headers("Accept: text/plain")
143+
String get();
144+
141145
@RequestLine("PATCH /")
142146
@Headers("Accept: text/plain")
143147
String patch();

‎okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import com.squareup.okhttp.mockwebserver.MockResponse;
1919
import com.squareup.okhttp.mockwebserver.MockWebServer;
2020

21-
import org.junit.Ignore;
2221
import org.junit.Rule;
2322
import org.junit.Test;
2423
import org.junit.rules.ExpectedException;
@@ -72,19 +71,18 @@ public void parsesRequestAndResponse() throws IOException, InterruptedException
7271
@Test
7372
public void parsesErrorResponse() throws IOException, InterruptedException {
7473
thrown.expect(FeignException.class);
75-
thrown.expectMessage("status 500 reading TestInterface#post(String); content:\n" + "ARGHH");
74+
thrown.expectMessage("status 500 reading TestInterface#get(); content:\n" + "ARGHH");
7675

7776
server.enqueue(new MockResponse().setResponseCode(500).setBody("ARGHH"));
7877

7978
TestInterface api = Feign.builder()
8079
.client(new OkHttpClient())
8180
.target(TestInterface.class, "http://localhost:" + server.getPort());
8281

83-
api.post("foo");
82+
api.get();
8483
}
8584

8685
@Test
87-
@Ignore // TODO: Remove on OkHttp 2.5 https://github.com/square/okhttp/issues/1778
8886
public void patch() throws IOException, InterruptedException {
8987
server.enqueue(new MockResponse().setBody("foo"));
9088
server.enqueue(new MockResponse());
@@ -93,7 +91,7 @@ public void patch() throws IOException, InterruptedException {
9391
.client(new OkHttpClient())
9492
.target(TestInterface.class, "http://localhost:" + server.getPort());
9593

96-
assertEquals("foo", api.patch());
94+
assertEquals("foo", api.patch(""));
9795

9896
assertThat(server.takeRequest())
9997
.hasHeaders("Accept: text/plain", "Content-Length: 0") // Note: OkHttp adds content length.
@@ -142,8 +140,12 @@ interface TestInterface {
142140
@Headers({"Foo: Bar", "Foo: Baz", "Qux: ", "Content-Type: text/plain"})
143141
Response post(String body);
144142

143+
@RequestLine("GET /")
144+
@Headers("Accept: text/plain")
145+
String get();
146+
145147
@RequestLine("PATCH /")
146148
@Headers("Accept: text/plain")
147-
String patch();
149+
String patch(String body);
148150
}
149151
}

0 commit comments

Comments
 (0)