kontent-ai / management-sdk-net

Kontent.ai Management .NET SDK
https://www.nuget.org/packages/Kontent.Ai.Management
MIT License
7 stars 31 forks source link

Propagate Retry-After header value #39

Closed matus12 closed 2 years ago

matus12 commented 5 years ago

Motivation

When I disable the default resilience logic in the ContentManagementClient and I exceed the API rate limit, I have no way to find out how long I should wait before retrying the request.

Proposed solution

I should be able to get the value of the Retry-After header in the ContentManagementException, e.g., in the JavaScript Content Management SDK, the original error is included.

PetrSvirak commented 5 years ago

It would be a nice touch if the SDK provided an option to retry 429 automatically as well…

matus12 commented 5 years ago

The SDK actually does have the 429 in the list of status codes to retry in the default retry policy, however, it doesn't take the retry-after value into consideration when to retry the request.

PetrSvirak commented 5 years ago

That's actually what I mean; while I'm not sure whether it should be part of default retry policy per se, SDK should IMO be able to handle retry-after in the response header (right it is just to some extent).

While allowing SDK users to reach the value feels like a bare minimum to me, it would be great if they didn't need to upscale retry count to about 10 artificially just to get past CM API rate limiting via default retry policy (that also applies to 429 by default as of now).

Simply007 commented 2 years ago

Just a couple of notes.

There is already a default implementation for retry logic https://github.com/Kentico/kontent-management-sdk-net/blob/master/Kentico.Kontent.Management/Modules/ResiliencePolicy/DefaultResiliencePolicyProvider.cs

It is also configurable and the custom setup is also covered by tests: https://github.com/Kentico/kontent-management-sdk-net/blob/master/Kentico.Kontent.Management.Tests/ManagementHttpClientTests.cs

Soo allowing to provide a second resilience policy, or a custom one would be a good start.