Skip to content

Commit

Permalink
Add retryable transport for errors (#1704)
Browse files Browse the repository at this point in the history
* Add retryable transport for errors

In order to address the issue #210
I have added 3 new parameters to the provider

- retry_delay_ms
- max_retries
- retryable_errors

In case max_retries is > 0 (defaults to zero)
it will retry the request in case it is an error
the retryable_errors defaults to [500, 502, 503, 504]

It retries after the ms specified in retry_delay_ms (defaults to 1000)

* Update documentation with new parameters for retry

* Change default of max_retries to 3

* Fix typo in naming

* Update github/transport_test.go

* Add error check for data seek

* Consolidate the default retriable errors on a function

* Fix typo on the comments

Co-authored-by: Keegan Campbell <me@kfcampbell.com>

* Update vendor

* Fix merging conflicts

* Update documentation with new parameters for retry

* Change default of max_retries to 3

* Fix typo in naming

* Add error check for data seek

* Update github/transport_test.go

* Consolidate the default retriable errors on a function

* Fix typo on the comments

Co-authored-by: Keegan Campbell <me@kfcampbell.com>

* Don't run go mod tidy on release (#1788)

* Don't run go mod tidy on release

* Be more specific about releases

* Fix lint with APIMeta deprecation

---------

Co-authored-by: Keegan Campbell <me@kfcampbell.com>
Co-authored-by: Nick Floyd <139819+nickfloyd@users.noreply.github.com>
  • Loading branch information
3 people authored Jan 10, 2024
1 parent 5752b25 commit 2dddeb3
Show file tree
Hide file tree
Showing 37 changed files with 3,334 additions and 2,534 deletions.
13 changes: 10 additions & 3 deletions github/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ type Config struct {
Insecure bool
WriteDelay time.Duration
ReadDelay time.Duration
RetryDelay time.Duration
RetryableErrors map[int]bool
MaxRetries int
ParallelRequests bool
}

Expand All @@ -33,7 +36,7 @@ type Owner struct {
IsOrganization bool
}

func RateLimitedHTTPClient(client *http.Client, writeDelay time.Duration, readDelay time.Duration, parallelRequests bool) *http.Client {
func RateLimitedHTTPClient(client *http.Client, writeDelay time.Duration, readDelay time.Duration, retryDelay time.Duration, parallelRequests bool, retryableErrors map[int]bool, maxRetries int) *http.Client {

client.Transport = NewEtagTransport(client.Transport)
client.Transport = NewRateLimitTransport(client.Transport, WithWriteDelay(writeDelay), WithReadDelay(readDelay), WithParallelRequests(parallelRequests))
Expand All @@ -43,6 +46,10 @@ func RateLimitedHTTPClient(client *http.Client, writeDelay time.Duration, readDe
"Accept": "application/vnd.github.stone-crop-preview+json",
}, client.Transport)

if maxRetries > 0 {
client.Transport = NewRetryTransport(client.Transport, WithRetryDelay(retryDelay), WithRetryableErrors(retryableErrors), WithMaxRetries(maxRetries))
}

return client
}

Expand All @@ -54,7 +61,7 @@ func (c *Config) AuthenticatedHTTPClient() *http.Client {
)
client := oauth2.NewClient(ctx, ts)

return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay, c.ParallelRequests)
return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay, c.RetryDelay, c.ParallelRequests, c.RetryableErrors, c.MaxRetries)
}

func (c *Config) Anonymous() bool {
Expand All @@ -63,7 +70,7 @@ func (c *Config) Anonymous() bool {

func (c *Config) AnonymousHTTPClient() *http.Client {
client := &http.Client{Transport: &http.Transport{}}
return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay, c.ParallelRequests)
return RateLimitedHTTPClient(client, c.WriteDelay, c.ReadDelay, c.RetryDelay, c.ParallelRequests, c.RetryableErrors, c.MaxRetries)
}

func (c *Config) NewGraphQLClient(client *http.Client) (*githubv4.Client, error) {
Expand Down
25 changes: 25 additions & 0 deletions github/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,31 @@ func TestAccConfigMeta(t *testing.T) {

})

t.Run("returns a v3 REST API client with max retries", func(t *testing.T) {

config := Config{
Token: testToken,
BaseURL: "https://api.github.com/",
RetryableErrors: map[int]bool{
500: true,
502: true,
},
MaxRetries: 3,
}
meta, err := config.Meta()
if err != nil {
t.Fatalf("failed to return meta without error: %s", err.Error())
}

ctx := context.Background()
client := meta.(*Owner).v3client
_, _, err = client.Meta.Get(ctx)
if err != nil {
t.Fatalf("failed to validate returned client without error: %s", err.Error())
}

})

t.Run("returns a v4 GraphQL API client to manage individual resources", func(t *testing.T) {

config := Config{
Expand Down
60 changes: 60 additions & 0 deletions github/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,26 @@ func Provider() terraform.ResourceProvider {
DefaultFunc: schema.EnvDefaultFunc("GITHUB_OWNER", nil),
Description: descriptions["owner"],
},
"retryable_errors": {
Type: schema.TypeList,
Elem: &schema.Schema{Type: schema.TypeInt},
Optional: true,
DefaultFunc: func() (interface{}, error) {
defaultErrors := []int{500, 502, 503, 504}
errorInterfaces := make([]interface{}, len(defaultErrors))
for i, v := range defaultErrors {
errorInterfaces[i] = v
}
return errorInterfaces, nil
},
Description: descriptions["retryable_errors"],
},
"max_retries": {
Type: schema.TypeInt,
Optional: true,
Default: 3,
Description: descriptions["max_retries"],
},
"organization": {
Type: schema.TypeString,
Optional: true,
Expand Down Expand Up @@ -60,6 +80,12 @@ func Provider() terraform.ResourceProvider {
Default: 0,
Description: descriptions["read_delay_ms"],
},
"retry_delay_ms": {
Type: schema.TypeInt,
Optional: true,
Default: 1000,
Description: descriptions["retry_delay_ms"],
},
"parallel_requests": {
Type: schema.TypeBool,
Optional: true,
Expand Down Expand Up @@ -264,11 +290,17 @@ func init() {
"Defaults to 1000ms or 1s if not set.",
"read_delay_ms": "Amount of time in milliseconds to sleep in between non-write requests to GitHub API. " +
"Defaults to 0ms if not set.",
"retry_delay_ms": "Amount of time in milliseconds to sleep in between requests to GitHub API after an error response. " +
"Defaults to 1000ms or 1s if not set, the max_retries must be set to greater than zero.",
"parallel_requests": "Allow the provider to make parallel API calls to GitHub. " +
"You may want to set it to true when you have a private Github Enterprise without strict rate limits. " +
"Although, it is not possible to enable this setting on github.com " +
"because we enforce the respect of github.com's best practices to avoid hitting abuse rate limits" +
"Defaults to false if not set",
"retryable_errors": "Allow the provider to retry after receiving an error status code, the max_retries should be set for this to work" +
"Defaults to [500, 502, 503, 504]",
"max_retries": "Number of times to retry a request after receiving an error status code" +
"Defaults to 3",
}
}

Expand Down Expand Up @@ -364,6 +396,31 @@ func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
}
log.Printf("[DEBUG] Setting read_delay_ms to %d", readDelay)

retryDelay := d.Get("read_delay_ms").(int)
if retryDelay < 0 {
return nil, fmt.Errorf("retry_delay_ms must be greater than or equal to 0ms")
}
log.Printf("[DEBUG] Setting retry_delay_ms to %d", retryDelay)

maxRetries := d.Get("max_retries").(int)
if maxRetries < 0 {
return nil, fmt.Errorf("max_retries must be greater than or equal to 0")
}
log.Printf("[DEBUG] Setting max_retries to %d", maxRetries)
retryableErrors := make(map[int]bool)
if maxRetries > 0 {
reParam := d.Get("retryable_errors").([]interface{})
if len(reParam) == 0 {
retryableErrors = getDefaultRetriableErrors()
} else {
for _, status := range reParam {
retryableErrors[status.(int)] = true
}
}

log.Printf("[DEBUG] Setting retriableErrors to %v", retryableErrors)
}

parallelRequests := d.Get("parallel_requests").(bool)

if parallelRequests && isGithubDotCom {
Expand All @@ -378,6 +435,9 @@ func providerConfigure(p *schema.Provider) schema.ConfigureFunc {
Owner: owner,
WriteDelay: time.Duration(writeDelay) * time.Millisecond,
ReadDelay: time.Duration(readDelay) * time.Millisecond,
RetryDelay: time.Duration(retryDelay) * time.Millisecond,
RetryableErrors: retryableErrors,
MaxRetries: maxRetries,
ParallelRequests: parallelRequests,
}

Expand Down
25 changes: 25 additions & 0 deletions github/provider_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,29 @@ func TestAccProviderConfigure(t *testing.T) {
})

})

t.Run("can be configured with max retries", func(t *testing.T) {

config := fmt.Sprintf(`
provider "github" {
token = "%s"
owner = "%s"
max_retries = 3
}`,
testToken, testOwnerFunc(),
)

resource.Test(t, resource.TestCase{
PreCheck: func() { skipUnlessMode(t, individual) },
Providers: testAccProviders,
Steps: []resource.TestStep{
{
Config: config,
ExpectNonEmptyPlan: false,
},
},
})

})

}
10 changes: 10 additions & 0 deletions github/repository_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,13 @@ func listAutolinks(client *github.Client, owner, repo string) ([]*github.Autolin

return allAutolinks, nil
}

// get the list of retriable errors
func getDefaultRetriableErrors() map[int]bool {
return map[int]bool{
500: true,
502: true,
503: true,
504: true,
}
}
96 changes: 96 additions & 0 deletions github/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,3 +197,99 @@ func isWriteMethod(method string) bool {
}
return false
}

type RetryTransport struct {
transport http.RoundTripper
retryDelay time.Duration
maxRetries int
retryableErrors map[int]bool
}

type RetryTransportOption func(*RetryTransport)

// NewRetryTransport takes in an http.RoundTripper and a variadic list of
// optional functions that modify the RetryTransport struct itself. This
// may be used to retry after response errors 5xx, for example.
func NewRetryTransport(rt http.RoundTripper, options ...RetryTransportOption) *RetryTransport {
// Default to no retry if none is provided
defaultErrors := getDefaultRetriableErrors()
rlt := &RetryTransport{transport: rt, retryDelay: time.Second, maxRetries: 0, retryableErrors: defaultErrors}

for _, opt := range options {
opt(rlt)
}

return rlt
}

func (t *RetryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
var err error
var resp *http.Response
var dataBuffer *bytes.Reader

for retry := 0; retry <= t.maxRetries; retry++ {
// Reset the body
// Code from httpretry (https://github.com/ybbus/httpretry/blob/master/roundtripper.go#L60)
// if request provides GetBody() we use it as Body,
// because GetBody can be retrieved arbitrary times for retry
if req.GetBody != nil {
bodyReadCloser, _ := req.GetBody()
req.Body = bodyReadCloser
} else if req.Body != nil {

// we need to store the complete body, since we need to reset it if a retry happens
// but: not very efficient because:
// a) huge stream data size will all be buffered completely in the memory
// imagine: 1GB stream data would work efficiently with io.Copy, but has to be buffered completely in memory
// b) unnecessary if first attempt succeeds
// a solution would be to at least support more types for GetBody()

// store it for the first time
if dataBuffer == nil {
data, err := io.ReadAll(req.Body)
req.Body.Close()
if err != nil {
return nil, err
}
dataBuffer = bytes.NewReader(data)
req.ContentLength = int64(dataBuffer.Len())
req.Body = io.NopCloser(dataBuffer)
}

// reset the request body
if _, err = dataBuffer.Seek(0, io.SeekStart); err != nil {
return nil, err
}
}

resp, err = t.transport.RoundTrip(req)
if resp != nil && !t.retryableErrors[resp.StatusCode] {
return resp, err
}

time.Sleep(t.retryDelay)
}

return resp, err
}

// WithMaxRetries is used to set the max number of retries when encountering an error
func WithMaxRetries(d int) RetryTransportOption {
return func(rt *RetryTransport) {
rt.maxRetries = d
}
}

// WithRetryableErrors is used to set status codes to retry
func WithRetryableErrors(d map[int]bool) RetryTransportOption {
return func(rt *RetryTransport) {
rt.retryableErrors = d
}
}

// WithRetryDelay is used to set the delay between requests for retrying
func WithRetryDelay(d time.Duration) RetryTransportOption {
return func(rt *RetryTransport) {
rt.retryDelay = d
}
}
Loading

0 comments on commit 2dddeb3

Please sign in to comment.