-
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
12.4 -> 12.5 changes behaviour with (not) using fallback client? (Resilience4jFeign) #2163
Comments
Well; I did notice we use version |
Unfortunately I was too hastily with my conclusion. Even with |
See resilience4j/resilience4j#2024 for example of integration test I am running that fails |
I don't familiar with WireMock. Is there any chance that WireMock does not expect second call? |
I bet this is related to #2117 where The difference is that these insertions suppose to happen before The way I made it work is to move my custom Feign builders for Resilience4j to
That's not pretty, though, and might not work for the original |
My main concern is that the fallback mechanism should continue to work. I can live with our tests being broken - we can probably fix that in a different way. But if the actual fallback mechanism is broken, I don't think I'd like to merge this. Although this probably is more of a |
Hi @stefanhendriks , maybe, Resileance4J should become a capability for feign, instead of a custom builder. So user would just:
|
12.5 has made a breaking change by making the public build method final, which broke Resilience4j ( |
12.5 is on maven central and once it's out there is no easy way to take it down. Anyway, ideally this wouldn't happen, but it did. I recommend moving Resilient4J work to capability API which will be much more stable then extending our builder. I don't think there is anything that can be done at this stage, so I will be closing this ticket. But, feel free to open a PR with any changes needed. |
This is how we implemented Resilience4jFeignCapability (in Kotlin) as @stefanhendriks suggested:
|
Looks perfect |
Hey @alfredalbrecht, I have the exact same issue and I tried with your latest solution, however, |
Hey, you have just to use the same Package as DecoratorInvocationHandler. In Kotlin you can just rename it (the linter could be upset). In Java you will have to create the folder Structure AFAIK.Am 15.02.2024 um 22:57 schrieb Alessandro Bagnoli ***@***.***>:
Hey @alfredalbrecht, I have the exact same issue and I tried with your latest solution, however, DecoratorInvocationHandler is package-private and cannot be instantiated. How did you solve this? Thanks in advance.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>
|
FYI: I made a PR that addresses this here in resilience4j resilience4j/resilience4j#2127 |
I noticed after upgrading from
12.4
to12.5
that one integration test fails.The integration test basically stubs one endpoint for the client to call, but, then later other calls by the client are not stubbed hence it falls back to a fallback client returning default responses.
However, in
12.5
I noticed that the 2nd call on our client (which is not stubbed), will simply return a404
. As if thefallbackClient
is not used there (set up using Resilience4jFeign)?Some code snippets:
Creation of our feign client:
Fallback client:
Although I am unsure if this is a bug in
Resilience4jFeign
; since I noticed it happening after upgrading to the latest openFeign library I thought of posting it here first.The text was updated successfully, but these errors were encountered: