Skip to content

Commit

Permalink
aws/request: Ensure New request handles nil retryer
Browse files Browse the repository at this point in the history
Adds additional default behavior to the SDK's New request constructor,
to handle the case where a nil Retryer was passed in. This error could
occur when the SDK's Request type was being used to create requests
directly, not through one of the SDK's client.

Fixes aws#2889
  • Loading branch information
jasdel committed Nov 7, 2019
1 parent 36b3b5f commit b6919f3
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 3 deletions.
12 changes: 10 additions & 2 deletions aws/request/request.go
Expand Up @@ -99,15 +99,23 @@ type Operation struct {
BeforePresignFn func(r *Request) error
}

// New returns a new Request pointer for the service API
// operation and parameters.
// New returns a new Request pointer for the service API operation and
// parameters.
//
// A Retryer should be provided to direct how the request is retried. If
// Retryer is nil, a default no retry value will be used. You can use
// NoOpRetryer in the Client package to disable retry behavior directly.
//
// Params is any value of input parameters to be the request payload.
// Data is pointer value to an object which the request's response
// payload will be deserialized to.
func New(cfg aws.Config, clientInfo metadata.ClientInfo, handlers Handlers,
retryer Retryer, operation *Operation, params interface{}, data interface{}) *Request {

if retryer == nil {
retryer = noOpRetryer{}
}

method := operation.HTTPMethod
if method == "" {
method = "POST"
Expand Down
27 changes: 26 additions & 1 deletion aws/request/retryer.go
Expand Up @@ -35,10 +35,35 @@ type Retryer interface {
}

// WithRetryer sets a Retryer value to the given Config returning the Config
// value for chaining.
// value for chaining. The value must not be nil.
func WithRetryer(cfg *aws.Config, retryer Retryer) *aws.Config {
cfg.Retryer = retryer
return cfg

}

// noOpRetryer is a internal no op retryer used when a request is created
// without a retryer.
//
// Provides a retryer that performs no retries.
// It should be used when we do not want retries to be performed.
type noOpRetryer struct{}

// MaxRetries returns the number of maximum returns the service will use to make
// an individual API; For NoOpRetryer the MaxRetries will always be zero.
func (d noOpRetryer) MaxRetries() int {
return 0
}

// ShouldRetry will always return false for NoOpRetryer, as it should never retry.
func (d noOpRetryer) ShouldRetry(_ *Request) bool {
return false
}

// RetryRules returns the delay duration before retrying this request again;
// since NoOpRetryer does not retry, RetryRules always returns 0.
func (d noOpRetryer) RetryRules(_ *Request) time.Duration {
return 0
}

// retryableCodes is a collection of service response codes which are retry-able
Expand Down
15 changes: 15 additions & 0 deletions aws/request/retryer_test.go
Expand Up @@ -5,7 +5,9 @@ import (
"fmt"
"testing"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/aws/awserr"
"github.com/aws/aws-sdk-go/aws/client/metadata"
)

func TestRequestThrottling(t *testing.T) {
Expand Down Expand Up @@ -64,3 +66,16 @@ func TestIsErrorRetryable(t *testing.T) {
}
}
}

func TestRequest_NilRetyer(t *testing.T) {
clientInfo := metadata.ClientInfo{Endpoint: "https://mock.region.amazonaws.com"}
req := New(aws.Config{}, clientInfo, Handlers{}, nil, &Operation{}, nil, nil)

if req.Retryer == nil {
t.Fatalf("expect retryer to be set")
}

if e, a := 0, req.MaxRetries(); e != a {
t.Errorf("expect no retries, got %v", a)
}
}

0 comments on commit b6919f3

Please sign in to comment.