zmb3
Add Configurable Retry Parameters
Currently, the client supports retry with WithRetry(true), but retry behavior (attempts, delays, backoff strategy) is not configurable.
Add options to configure retry parameters:
- Maximum retry attempts
- Initial retry delay
- Backoff strategy (exponential, linear, fixed)
- Maximum retry delay
Proposed API
client := spotify.New(auth.Client(ctx, token),
spotify.WithRetry(true),
spotify.WithMaxRetries(3),
spotify.WithInitialDelay(time.Second),
spotify.WithMaxDelay(30*time.Second),
spotify.WithBackoffStrategy(spotify.ExponentialBackoff),
)
// OR
retryCfg := &spotify.RetryConfig{
MaxRetries: 3,
InitialDelay: time.Second,
MaxDelay: 30 * time.Second,
Backoff: spotify.ExponentialBackoff,
}
client := spotify.New(auth.Client(ctx, token),
spotify.WithRetry(true),
spotify.WithRetryConfig(retryCfg),
)
Applications with different reliability requirements need control over retry behaviour to balance between request success rate and response latency.
I would be happy to contribute and work on this feature.
Rather than baking all that logic into this library, perhaps WithRetry could accept a callback function, to allow the library's consumer to add whatever arbitrary logic they want?
Maybe something like:
// RetryCallback is called whenever the Spotify API returns a non-success error code. If the callback returns true, the
// request will be retried.
//
// Caution: This could lead to infinite loops if not handled carefully!
type RetryCallback func(*http.Request, *http.Response) bool
We may need to expose other information to the callback as well. Or add a unique request ID to the request header, for example, to allow tracking number of retries for a given request, etc.
I think such an approach would easily solve #282 as well.
Thanks for the suggestion! However, the callback approach doesn't quite solve configurable retry - it pushes the complexity (attempt counting, backoff delays, timers) to users.
The goal is to configure retry behavior (max attempts, backoff strategy) without implementing retry algorithms from scratch. Both AWS SDK and Kubernetes client-go use structured retry config for exactly this reason:
- https://pkg.go.dev/github.com/aws/aws-sdk-go-v2/aws/retry
- https://pkg.go.dev/k8s.io/client-go/util/retry
But possibly a combination could solve this as well as #282.
The two examples you provide are separate libraries, which provide pretty advanced retry logic. I'm questioning whether a simple client library like this is the appropriate place to house such logic. My goal with a callback approach would be to give the caller full flexibility to use whichever retry logic/library they want.
If the proposed callback function signature does not work with popular retry libraries, I would suggest we should find a signature that would.
Do you have a reason to believe that it makes more sense to add that complexity to this library?
Sorry - those libraries were just some examples of a configurable retry strategy, not necessarily a desired implementation for this library. I understand the scope concern, but basic retry config (max attempts, delay) is pretty standard in HTTP clients.
The callback approach would work well for the #282 use case to retry specific responses or errors. For general retry configuration though, it doesn't handle the mechanics - just returns true/false without managing delays or attempt counting, etc.
Maybe a combination of both here? I don't see a problem with the callback as well as basic retry config parameters.
just returns true/false without managing delays or attempt counting, etc.
I would expect the callback itself to handle the delay and attempt counting. I mentioned in my original comment that we may need to expose additional information, such as a request ID, to the callback, to make attempt counting possible.
Another approach could be to wrap the entire HTTP query cycle in a callback--this is effectively already possible by setting a custom HTTP client, and assigning a custom transport. This is, in fact, how I've solved #282 in my own code up to now. This approach could be made cleaner with support from this library, since not all HTTP requests are safely retried without a lot of data duplication.