Skip to content
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

Add Resilience configuration options #1854

Closed
mtamagnini opened this issue Nov 30, 2021 · 10 comments
Closed

Add Resilience configuration options #1854

mtamagnini opened this issue Nov 30, 2021 · 10 comments
Assignees
Labels
feature request Requests for new functionality

Comments

@mtamagnini
Copy link

mtamagnini commented Nov 30, 2021

Is your feature request related to a problem? Please describe.

When dealing with Destination services, it is quite often necessary to re-send an API call against a given endpoint. In our case, we (i.e. SAP Graph Team) have the need of sending "hundreds" - and potentially "thousands" - of requests against Destination service.
To reach such behaviour, today a developer needs to implement it by his/her-self with an additional layer of custom code (and it should not be build on every SDK-based project again and again...).

Describe the solution you'd like

As already in place in Java version of Cloud SDK (see ResilienceConfiguration.RetryConfiguration), it would be great to add ResilienceConfigurationOptions - in particular, the retry config - also to JS/TS version of CloudSDK, enabling (at least) the followings:

  • option-set from Java version;
  • non linear delay in the resilience;
  • option for switching on in getDestination, executeHttp, execute and fetchDestination;
  • possibility to add an interceptor's callback method.

Impact / Priority

Affected development phase: Product Release

Impact: Impaired and feature fix Blocked

Timeline: Upcoming Product release.

Additional context
Already had a short discussion with @FrankEssenberger.

@mtamagnini mtamagnini added the feature request Requests for new functionality label Nov 30, 2021
@FrankEssenberger
Copy link
Contributor

This could be a quick win to include it: https://www.npmjs.com/package/axios-retry

@artemkovalyov
Copy link
Member

Hey, @mtamagnini

We use Resilience4j in Java which simplifies things a lot. When starting the JS SDK there was no good library to integrate.
We might reconsider it in 2022 but I doubt we can release anything soon. That's why I'd recommend using available libraries together with SDK. Frank already suggested axiso-retry. Here is a couple of the most popular libs:

Please, share the results of your experiments, or let's have a call when you gain some experience to discuss potential convenience options.

@artemkovalyov
Copy link
Member

My teammates also mentioned that circuit breaker is integrated in the SDK with https://github.com/nodeshift/opossum
Sadly there's no developer documentation on the portal, you'll have to take a look in the API docs. cc @marikaner

@FrankEssenberger
Copy link
Contributor

Here a second use asked about resilience: https://answers.sap.com/questions/13572567/cap-transient-error-handling.html# Potentially we should increase priority on this.

@FrankEssenberger
Copy link
Contributor

So after a long time there is some progress and we created a ADR to implement a resilience on a flexible middle ware approach: #2320 as graph team colleagues @mtamagnini @Johannes-Schneider could you perhaps give quick feedback if the ADR goes in the right direction?

@Johannes-Schneider
Copy link
Member

Hey @FrankEssenberger,

Did you actually mean to ping me in this issue?

@FrankEssenberger
Copy link
Contributor

FrankEssenberger commented Apr 21, 2022

No I wanted to ping @johenning from the sap graph team. Sorry.

@johenning
Copy link

@FrankEssenberger I wasn't super involved with this topic, but as far as I can tell the ADR looks good 👍

@FrankEssenberger
Copy link
Contributor

Thanks for the feedback. We tried to make it possible to also bring a custom solution in case the options are not flexible enough.

@fwilhe
Copy link
Contributor

fwilhe commented Dec 21, 2022

Hi @mtamagnini

I know this issue has been open for a long time. Wanted to let you know that for version 3 of the sdk we are working on a generic middleware/resilience approach that also includes retry. It is implemented in this pr but not yet released. If nothing unexpected happens this should be part of v3 of the sdk. I can't promise a specific release date.

Hope this helps,
Florian

@fwilhe fwilhe closed this as completed Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new functionality
Projects
None yet
Development

No branches or pull requests

6 participants