Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws/request: Ensure New request handles nil retryer #2934

Merged
merged 3 commits into from Nov 7, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG_PENDING.md
Expand Up @@ -3,3 +3,6 @@
### SDK Enhancements

### SDK Bugs
* `aws/request`: Ensure New request handles nil retryer ([#2934](https://github.com/aws/aws-sdk-go/pull/2934))
* 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 [#2889](https://github.com/aws/aws-sdk-go/issues/2889)
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 {
jasdel marked this conversation as resolved.
Show resolved Hide resolved
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)
}
}