gojek / heimdall

An enhanced HTTP client for Go
http://gojek.tech
Apache License 2.0
2.63k stars 214 forks source link

Retrier called even with 0 retry count #89

Open sunjayaali opened 4 years ago

sunjayaali commented 4 years ago
  server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.WriteHeader(500)
    }))
    defer server.Close()

    retryCalled := 0
    httpclient := httpclient.NewClient(
        httpclient.WithRetrier(heimdall.NewRetrierFunc(func(_ int) time.Duration {
            retryCalled++
            return 1 * time.Second
        })),
        httpclient.WithRetryCount(0),
    )
    req, err := http.NewRequest(http.MethodGet, server.URL, nil)
    assert.NoError(t, err)
    res, err := httpclient.Do(req)
    assert.NoError(t, err)
    _ = res
    assert.Equal(t, 0, retryCalled)

retrier still called with 0 retry count, is this expected?

vthiery commented 4 years ago

Another problem that is linked is that time.Sleep(backoffTime) will be called even when the retries are exhausted, which can lead to quite a waste of time for the caller.

If we agree on both these issues, I'd be happy to propose a fix.