I believe that clean and elegant retry code is notoriously difficult, so I'm wondering how the following code could be improved. TheretryGet function should have the same interface and contract ashttp.Get, except for the number of attempts argument, of course.
func retryGet(url string, maxAttempts int) (*http.Response, error) { attempts := 0 if maxAttempts < 1 { return nil, errors.New("maxAttempts must be at least 1") } for { attempts++ response, err := http.Get(url) if err == nil && response.StatusCode == http.StatusOK { return response, nil } delay, retry := shouldRetry(maxAttempts, attempts, response) if !retry { return response, err } if err == nil { defer response.Body.Close() } time.Sleep(delay) }}func shouldRetry(maxAttempts, attempts int, response *http.Response) (time.Duration, bool) { if attempts >= maxAttempts { return time.Duration(0), false } delay := time.Duration(attempts) * time.Second if response != nil && response.Header.Get("Retry-After") != "" { after, err := strconv.ParseInt(response.Header.Get("Retry-After"), 10, 64) if err != nil && after > 0 { delay = time.Duration(after) * time.Second } } return delay, true}
1 Answer1
I would recommend you to actually strictly match thehttp.Get signature using a struct:
type Getter func(url string) (*http.Response, error)type RetryingClient struct { Getter Getter MaxAttempts int}func (rc RetryingClient) Get(url string) (*http.Response, error) { ...}And instead of directly callinghttp.Get, the embeddedGetter interface gives you much more flexibility:
- for testing
- for using a proxy
Why do youdefer theresponse.Body.Close() ? Since it won't be use, you can close it immediately.
Using the interface, you could make a function embedded in the struct to compute a default delay instead of hard-codedtime.Duration(attempts) * time.Second.For instance:defaultDelay(attempt int) time.Duration
You mustlog in to answer this question.
Explore related questions
See similar questions with these tags.