Add client-side support for datetime value of Retry-After header #131
Conversation
internal/retryafter.go
Outdated
| retryIntervalSec, err := strconv.Atoi(retryAfter) | ||
| if err == nil { | ||
| retryInterval := time.Duration(retryIntervalSec) * time.Second | ||
| return OptionalDuration{Defined: true, Duration: retryInterval} | ||
| } | ||
| // Try to parse retryAfterHTTPHeader as HTTP-date | ||
| // See https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3 | ||
| t, err := time.Parse(time.RFC1123, retryAfter) |
There was a problem hiding this comment.
The HTTP RFC7231 defines 3 different formats of HTTP-date that receivers MUST support. One of the formats is RFC5322. This uses RFC1123 which does not seem to be what RFC7231 requires.
Codecov ReportBase: 74.61% // Head: 76.22% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #131 +/- ##
==========================================
+ Coverage 74.61% 76.22% +1.61%
==========================================
Files 23 23
Lines 1765 1792 +27
==========================================
+ Hits 1317 1366 +49
+ Misses 334 319 -15
+ Partials 114 107 -7
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
internal/retryafter.go
Outdated
| // See https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3 | ||
| t, err := http.ParseTime(retryAfter) | ||
| if err == nil { | ||
| retryInterval := time.Until(t) |
There was a problem hiding this comment.
We should probably check that this is not negative. And the same in the case above with seconds. Seems like omission to me.
internal/retryafter_test.go
Outdated
| func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) { | ||
| // Generate random n > 0 int | ||
| rand.Seed(time.Now().UnixNano()) | ||
| retryIntervalSec := rand.Intn(9999) | ||
|
|
||
| // Generate a 503 status code response with Retry-After = n header | ||
| resp := response503() | ||
| resp.Header.Add(retryAfterHTTPHeader, strconv.Itoa(retryIntervalSec)) | ||
|
|
||
| expectedDuration := OptionalDuration{ | ||
| Defined: true, | ||
| Duration: time.Second * time.Duration(retryIntervalSec), | ||
| } | ||
| assert.Equal(t, expectedDuration, ExtractRetryAfterHeader(resp)) | ||
| } | ||
|
|
||
| func TestExtractRetryAfterHeader429StatusCode(t *testing.T) { | ||
| // Generate random n > 0 int | ||
| rand.Seed(time.Now().UnixNano()) | ||
| retryIntervalSec := rand.Intn(9999) | ||
|
|
||
| // Generate a 429 status code response with Retry-After = n header | ||
| resp := response503() | ||
| resp.StatusCode = http.StatusTooManyRequests | ||
| resp.Header.Add(retryAfterHTTPHeader, strconv.Itoa(retryIntervalSec)) | ||
|
|
||
| expectedDuration := OptionalDuration{ | ||
| Defined: true, | ||
| Duration: time.Second * time.Duration(retryIntervalSec), | ||
| } | ||
| assert.Equal(t, expectedDuration, ExtractRetryAfterHeader(resp)) | ||
| } |
There was a problem hiding this comment.
These 2 tests are almost identical. It would be nice to make them one test with 2 sub-tests. It will eliminate a lot of lines.
tigrannajaryan
changed the title
According to specifications, Retry-After header should be either seconds-delay or HTTP-date
https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3
Resolves #119