Added support for HTTP 429 Retry-After as per Issue #99

[mariotoffia]

This was added in DefaultPolicy and DefaultBackoff. The latter will
examine the Retry-After response header (if existant) and do a backoff
for the specified amount of time. Otherwise it will default to default
backoff behaviour.

This addresses Issue #99.

[hashicorp-cla]

All committers have signed the CLA.

[theckman]

There's PR #70 which adds a new retry policy function, based on the one in master. If that gets merged before this one, we'll need to update this PR to include adding it in the new function (in 70).

[jefferai]

@mariotoffia #70 was merged. Additionally the CLA isn't signed. Please update/fix and we can look again!

[mariotoffia]

Thanks! :)

I've merged and committed - still there's two test errors The report I could download (Artifacts) do not describe the output of the error, just that it has errors on following methods:

<testsuites>
<testsuite tests="3" failures="0" time="0.000000" name="github.com/hashicorp/go-retryablehttp">
<properties>
<property name="go.version" value="go1.14.2 linux/amd64"/>
</properties>
<testcase classname="" name="TestMain" time="0.000000">
<failure message="Failed" type="">FAIL github.com/hashicorp/go-retryablehttp 0.007s </failure>
</testcase>
<testcase classname="github.com/hashicorp/go-retryablehttp" name="TestRequest" time="0.000000"/>
<testcase classname="github.com/hashicorp/go-retryablehttp" name="TestFromRequest" time="0.000000"/>
</testsuite>
</testsuites>

I don't know why those two functions get's errors since I've not altered those. If I e.g. run TestFromRequest locally it works just fine:

go test -v -run TestFromRequest
=== RUN   TestFromRequest
--- PASS: TestFromRequest (0.00s)
PASS
ok      github.com/hashicorp/go-retryablehttp   0.004s

Cheers,
Mario

[mateusrangel]

Seems like he didn't signed the CLA and one test is failing.

Any news?

[mateusrangel]

So @mariotoffia, i am checking your code and already detected the point of panic.

[mateusrangel]

locally you are testing only TestFromRequest, i did a pure go test before and after your changes and got a

[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x6db207]

goroutine 9 [running]:
github.com/hashicorp/go-retryablehttp.DefaultBackoff(0x989680, 0x2faf080, 0x0, 0x0, 0x3)

When i comment your code at:

if resp.StatusCode == http.StatusTooManyRequests {
	if s, ok := resp.Header["Retry-After"]; ok {
		if sleep, err := strconv.ParseInt(s[0], 10, 32); err == nil {
			return time.Duration(int64(time.Second) * sleep)
		}
	}
}

every thing runs fine, seems like we have a problem in the header check or in the parsing

[mariotoffia]

@mateusrangel That's strange - when I do a

martoffi@dell:~/progs/github/go-retryablehttp$ go test -v -run TestClient_DefaultBackoff429TooManyRequest

I do get

=== RUN   TestClient_DefaultBackoff429TooManyRequest
2020/06/17 21:10:10 [DEBUG] GET http://127.0.0.1:40959
--- PASS: TestClient_DefaultBackoff429TooManyRequest (0.00s)
PASS
ok      github.com/hashicorp/go-retryablehttp   0.005s

When I run it locally. However, I was a bit curious how to get the test output from your CI tool - how do I do that?

Cheers,
Mario

[mariotoffia]

@mateusrangel Ok, I've got it - the

resp *http.Response

is nil. I'm committing a fix now.

[mateusrangel]

@mariotoffia Happy to help the debugging 😄

[mariotoffia]

@jefferai is it good to go?
(the core of it is here)

[mariotoffia]

@jefferai committed your review points - I'd guess this PR needs to be squashed since it is insane amount of commits :)

[jefferai]

LGTM, thanks!