aws/request: Ensure New request handles nil retryer (#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 #288

CHANGELOG_PENDING.md

@@ -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)

aws/request/request.go

@@ -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"

aws/request/retryer.go

@@ -35,10 +35,41 @@ 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 {
+	if retryer == nil {
+		if cfg.Logger != nil {
+			cfg.Logger.Log("ERROR: Request.WithRetryer called with nil retryer. Replacing with retry disabled Retryer.")
+		}
+		retryer = noOpRetryer{}
+	}
 	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

aws/request/retryer_test.go

@@ -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) {
@@ -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)
+	}
+}