decode404 API is broken

[uschi2000]

Hi @adriancole and other feign folks,

Trying to implement an "OptionalAwareJacksonDecoder", it seems that the new decode404 API is rather inflexible. Here's the scenario: Assume a server interface with two methods:

Optional<Foo> getOptionalFoo();
Foo getFoo();

, and assume a Feign client with decode404() option enabled. Now, when OptionalAwareJacksonDecoder#decode gets hit, it has to decide what to return, roughly like this:

    @Override
    public Object decode(Response response, Type type) throws IOException, FeignException {
        if (response.status() == 404) {
            if (Types.getRawType(type).equals(Optional.class)) {
                return Optional.absent();
            } else {
                // ??? should call client's ErrorDecoder and throw the error supplied, but cannot.
            }
        } else {
            return delegate.decode(response, type); // delegate to injected JacksonDecoder
        }
    }

It all works fine for the getOptionalFoo() interface, but fails for the getFoo() interface, because we should really be delegating to the configured ErrorDecoder, but cannot (even if we inject it, we're not allowed to throw here).

Thoughts?

Thanks!
Robert

[codefromthecrypt]

If you want to do something besides return absent (in this case null), just throw a FeignException like the error decoder would.

[codefromthecrypt]

Or just add error decoder as a field to your decoder. That would do it, too!

[uschi2000]

That does not do the same thing since exceptions thrown inside the Decoder get wrapped in a DecodeException.

[uschi2000]

(Also, you cannot really delegate naturally to an ErrorDecoder inside a Decoder since you don't have access to a methodKey).

[codefromthecrypt]

We can change the code to not wrap FeignException in a DecodeException!

[uschi2000]

That would be grand :)

[codefromthecrypt]

should be pretty easy.. wanna give it a go?

[uschi2000]

#293