Allow for customization of statusCode interpretation#238
Allow for customization of statusCode interpretation#238nblair wants to merge 1 commit intoOpenFeign:8.xOpenFeign/feign:8.xfrom
Conversation
Adds new interface encapsulating logic of determining whether or not a response should be parsed or raise an exception. Previously, inline logic in SynchronousMethodHandler only treated 200 class status codes as something to parse; everything else, including 404, as exceptable. Callers can inject StatusInterpreter.Include404 if they desire 404 status codes to result in Feign returning null for their methods, rather than raising an Exception.
|
NetflixOSS » feign » feign-pull-requests #112 SUCCESS |
|
gonna sleep on this. I admit that I've a lot of catch blocks in denominator
about this, which boils down to a fallback value, most usually fallback to
null, void, or false.
|
|
So, this is definitely in the area of static-simple fallbacks, somewhat related to the hystrix discussion. When designing feign, originally, the error decoder had fallback handling as a part of it. This was inspired by prior art in jclouds, intended to be a bit simpler. When doing an implementation with an Async pattern (Observer), we realized that this fallback handling was something invocation-specific, so in order to reuse the ErrorDecoder, we had to decouple it from fallbacks (46be6bd) If we were to resurrect fallback handling, I'd prefer to not conflate it with the decoder. For example, if fallback processing occurs with context of the request, it could be statically mapped (ex. 404 -> default value or null), or others could choose to compose one out of a decoder. For now, I'd recommend either working off your fork or abusing InvocationHandlerFactory to supply default values :) It is likely something related will come out of #189 so I'd likely watch there. |
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. For example, we have a fast-path for addressing absence which doesn't accidentally turn bad request, redirect, silently to null. Processing 404 is a very safe policy: it doesn't overlap with retries or other behavior in feign which could turn into support questions. Also, we don't create a cliff, where folks looking for error handling eventually realize they can't address all fallback policy with only an http response code (bc exceptions can happen far before you get a response from the server). By noting 404 as a special case, we avoid a slippery slope of incrementally half-implementing Hystrix or similar. This design supports the common pattern of empty semantics with the least code on both users and the maintainers of Feign. We can still recommend other libraries such as Hystrix or Ribbon for advanced retry or fallback policy. Most importantly, addressing 404 is extremely high value. You are just the last to mention it. For example, I would use this in denominator. Moreover, it doesn't require us to introduce or break an interface. This means all users, such as spring-cloud, can easily opt-into this. It could substantially reduce their code in the same way it can reduce your code, but without new types. See #238 #287
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. For example, we have a fast-path for addressing absence which doesn't accidentally turn bad request, redirect, silently to null. Processing 404 is a very safe policy: it doesn't overlap with retries or other behavior in feign which could turn into support questions. Also, we don't create a cliff, where folks looking for error handling eventually realize they can't address all fallback policy with only an http response code (bc exceptions can happen far before you get a response from the server). By noting 404 as a special case, we avoid a slippery slope of incrementally half-implementing Hystrix or similar. This design supports the common pattern of empty semantics with the least code on both users and the maintainers of Feign. We can still recommend other libraries such as Hystrix or Ribbon for advanced retry or fallback policy. See #238 #287
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
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
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
We just started looking at Feign for implementing client libraries for some REST APIs.
We wanted to setup an interface like so:
The REST API's contract states that
/api/v1/foo/{id}will return a 404 if no resource exists with that id.In the current 8.3 release of Feign, a return code of 404 is treated as something that should raise an Exception. Our method signature would have to look like this instead:
We'd prefer not to have to create an exception for this scenario.
This pull request adds a new interface encapsulating logic currently at line 106 of:
https://github.com/Netflix/feign/blob/8.x/core/src/main/java/feign/SynchronousMethodHandler.java
Previously, inline
logic in SynchronousMethodHandler only treated 200 class status codes as
something to parse; everything else, including 404, as reason to throw exception (ErrorDecoder then takes over).
This change allows callers to inject StatusInterpreter.Include404 if they desire a contract that can return null for 404 like the one presented at the top rather than raising an Exception. The interface allows for further customization if desired.
The default behavior is unchanged.