Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics#290
Merged
Conversation
Author
|
Thanks to @uschi2000 @nblair, who both asked for this and offered ideas about it, also @spencergibb and @jdamick, who were involved in recent brainstorming |
cfbecf0 to
096c4f3
Compare
Author
|
@FrEaKmAn your work is also related. |
This adds the `Feign.Builder.decode404()` flag which indicates decoders should process responses with 404 status. It also changes all first-party decoders (like gson) to return well-known empty values by default. Further customization is possible by wrapping or creating a custom decoder. Prior to this change, we used custom invocation handlers as the way to add fallback values based on exception or return status. `feign-hystrix` uses this to return `HystrixCommand<X>`, but the general pattern applies to anything that has a type representing both success and failure, such as `Try<X>` or `Observable<X>`. As we define it here, 404 status is not a retry or fallback policy, it is just empty semantics. By limiting Feign's special processing to 404, we gain a lot with very little supporting code. If instead we opened all codes, Feign could easily turn bad request, redirect, or server errors silently to null. This sort of configuration issue is hard to troubleshoot. 404 -> empty is a very safe policy vs all codes. Moreover, we don't create a cliff, where folks seeking fallback policy eventually realize they can't if only given a response code. Fallback systems like Hystrix address exceptions that occur before or in lieu of a response. By special-casing 404, we avoid a slippery slope of half- implementing Hystrix. Finally, 404 handling has been commonly requested: it has a clear use- case, and through that value. This design supports that without breaking compatibility, or impacting existing integrations such as Hystrix or Ribbon. See #238 #287
096c4f3 to
24885fe
Compare
|
NetflixOSS » feign » feign-pull-requests #173 SUCCESS |
|
I like this approach, 👍 Thanks! |
Contributor
|
👍 |
codefromthecrypt
pushed a commit
that referenced
this pull request
Oct 31, 2015
Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics
|
Wow, that was quick. Cheers Adrian! |
Author
|
quick is relative: I'm sure there are folks who think this took too long
(as asked earlier) :) I'll take the cheers, regardless!
|
velo
pushed a commit
that referenced
this pull request
Oct 8, 2024
Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds the
Feign.Builder.decode404()flag which indicates decodersshould process responses with 404 status. It also changes all
first-party decoders (like gson) to return well-known empty values by
default. Further customization is possible by wrapping or creating a
custom decoder.
On why we aren't opening all codes
Prior to this change, we used custom invocation handlers as the way to
add fallback values based on exception or return status.
feign-hystrixuses this to return
HystrixCommand<X>, but the general pattern appliesto anything that has a type representing both success and failure, such
as
Try<X>orObservable<X>.As we define it here, 404 status is not a retry or fallback policy, it
is just empty semantics. By limiting Feign's special processing to 404,
we gain a lot with very little supporting code.
If instead we opened all codes, Feign could easily turn bad request,
redirect, or server errors silently to null. This sort of configuration
issue is hard to troubleshoot. 404 -> empty is a very safe policy vs
all codes.
Moreover, we don't create a cliff, where folks seeking fallback policy
eventually realize they can't if only given a response code. Fallback
systems like Hystrix address exceptions that occur before or in lieu of
a response. By special-casing 404, we avoid a slippery slope of half-
implementing Hystrix.
Finally, 404 handling has been commonly requested: it has a clear use-
case, and through that value. This design supports that without breaking
compatibility, or impacting existing integrations such as Hystrix or
Ribbon.
See #238 #287