Skip to content

Commit

Permalink
SynchronousMethodHandler does not wrap exceptions thrown by Decoder#d…
Browse files Browse the repository at this point in the history
…ecode in decode404 mode

Removing exception wrapping adds flexibility to the interplay of Decoder and ErrorDecoder:
A Decoder can now delegate to an ErrorDecoder and the original ErrorDecoder exception gets
thrown rather than a Feign-specific DecodeException. An example use-case is a Jackson/Gson
implementation of special 404-handling of Optional<Foo> methods: when the Feign client has
decode404() enabled, then the Decoder is in charge of dispatching 404 to Optional#absent
where applicable, or to a delegate ErrorDecoder otherwise; consistent exception handling
requires that the exception produced by the ErrorDecoder does not wrapped.
  • Loading branch information
Robert Fink committed Nov 3, 2015
1 parent 40b32ec commit 544820c
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 1 deletion.
2 changes: 1 addition & 1 deletion core/src/main/java/feign/SynchronousMethodHandler.java
Expand Up @@ -121,7 +121,7 @@ Object executeAndDecode(RequestTemplate template) throws Throwable {
return decode(response);
}
} else if (decode404 && response.status() == 404) {
return decode(response);
return decoder.decode(response, metadata.returnType());
} else {
throw errorDecoder.decode(metadata.configKey(), response);
}
Expand Down
4 changes: 4 additions & 0 deletions core/src/main/java/feign/codec/Decoder.java
Expand Up @@ -18,6 +18,7 @@
import java.io.IOException;
import java.lang.reflect.Type;

import feign.Feign;
import feign.FeignException;
import feign.Response;
import feign.Util;
Expand Down Expand Up @@ -49,6 +50,9 @@
* feign.Target#type() interface} processed by {@link feign.Feign#newInstance(feign.Target)}. When
* writing your implementation of Decoder, ensure you also test parameterized types such as {@code
* List<Foo>}.
* <br/> <h3>Note on exception propagation</h3> Exceptions thrown by {@link Decoder}s get wrapped in
* a {@link DecodeException} unless they are a subclass of {@link FeignException} already, and unless
* the client was configured with {@link Feign.Builder#decode404()}.
*/
public interface Decoder {

Expand Down
23 changes: 23 additions & 0 deletions core/src/test/java/feign/FeignTest.java
Expand Up @@ -36,6 +36,7 @@
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.concurrent.atomic.AtomicReference;

import feign.Target.HardCodedTarget;
Expand Down Expand Up @@ -387,6 +388,23 @@ public Object decode(Response response, Type type) throws IOException {
api.post();
}

@Test
public void decoderCanThrowUnwrappedExceptionInDecode404Mode() throws Exception {
server.enqueue(new MockResponse().setResponseCode(404));
thrown.expect(NoSuchElementException.class);

TestInterface api = new TestInterfaceBuilder()
.decode404()
.decoder(new Decoder() {
@Override
public Object decode(Response response, Type type) throws IOException {
assertEquals(404, response.status());
throw new NoSuchElementException();
}
}).target("http://localhost:" + server.getPort());
api.post();
}

@Test
public void okIfEncodeRootCauseHasNoMessage() throws Exception {
server.enqueue(new MockResponse().setBody("success!"));
Expand Down Expand Up @@ -594,6 +612,11 @@ TestInterfaceBuilder errorDecoder(ErrorDecoder errorDecoder) {
return this;
}

TestInterfaceBuilder decode404() {
delegate.decode404();
return this;
}

TestInterface target(String url) {
return delegate.target(TestInterface.class, url);
}
Expand Down

0 comments on commit 544820c

Please sign in to comment.