-
Notifications
You must be signed in to change notification settings - Fork 1.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds Feign.Builder.decode404() to reduce boilerplate for empty semantics #290
Merged
Conversation
This file contains 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
Thanks to @uschi2000 @nblair, who both asked for this and offered ideas about it, also @spencergibb and @jdamick, who were involved in recent brainstorming |
codefromthecrypt
force-pushed
the
not-found
branch
from
October 31, 2015 16:35
cfbecf0
to
096c4f3
Compare
@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
codefromthecrypt
force-pushed
the
not-found
branch
from
October 31, 2015 16:53
096c4f3
to
24885fe
Compare
NetflixOSS » feign » feign-pull-requests #173 SUCCESS |
I like this approach, 👍 Thanks! |
👍 |
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! |
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
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-hystrix
uses 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