Skip to content

Commit

Permalink
Fixes retry support to not fail due to body not being rewound between…
Browse files Browse the repository at this point in the history
… retry attempts.

Merged #196 and #204 together along with other changes to make sure rewind works in all cases,
Made sure the ordering of handler's setting Request.Retryable vs the Service.ShouldRetry are consitent.
A Request Handler setting Request.Retryable will prevent RetryHandler calling Service.ShouldRetry. The Service's
delay value wills still be applied though.
  • Loading branch information
jasdel committed Apr 29, 2015
1 parent 6bb619c commit cc754a4
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 15 deletions.
14 changes: 8 additions & 6 deletions aws/handler_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,25 +54,27 @@ func ValidateResponseHandler(r *Request) {
// this may be replaced by an UnmarshalError handler
r.Error = &APIError{
StatusCode: r.HTTPResponse.StatusCode,
Message: r.HTTPResponse.Status,
Code: "UnknownError",
Message: "unknown error",
}
}
}

func RetryHandler(r *Request) {
r.Retryable = r.Service.ShouldRetry(r)
r.RetryDelay = r.Service.RetryRules(r)
if r.Retryable {
r.ResetReaderBody()
// If one of the other handlers already set the retry state
// we don't want to override it based on the service's state
if !r.Retryable.IsSet() {
r.Retryable.Set(r.Service.ShouldRetry(r))
}

r.RetryDelay = r.Service.RetryRules(r)
}

func AfterRetryHandler(r *Request) {
if r.WillRetry() {
sleepDelay(r.RetryDelay)

r.RetryCount++
r.Retryable = false
r.Error = nil
}
}
Expand Down
16 changes: 9 additions & 7 deletions aws/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ type Request struct {
Data interface{}
RequestID string
RetryCount uint
Retryable bool
Retryable SettableBool
RetryDelay time.Duration

built bool
Expand Down Expand Up @@ -69,7 +69,7 @@ func NewRequest(service *Service, operation *Operation, params interface{}, data
}

func (r *Request) WillRetry() bool {
return r.Error != nil && r.Retryable && r.RetryCount < r.Service.MaxRetries()
return r.Error != nil && r.Retryable.Get() && r.RetryCount < r.Service.MaxRetries()
}

func (r *Request) ParamsFilled() bool {
Expand All @@ -93,11 +93,6 @@ func (r *Request) SetReaderBody(reader io.ReadSeeker) {
r.Body = reader
}

func (r *Request) ResetReaderBody() {
r.Body.Seek(r.bodyStart, 0)
r.HTTPRequest.Body = ioutil.NopCloser(r.Body)
}

func (r *Request) Presign(expireTime time.Duration) (string, error) {
r.ExpireTime = expireTime
r.Sign()
Expand Down Expand Up @@ -139,6 +134,13 @@ func (r *Request) Send() error {
}

for {
if r.Retryable.Get() {
// Re-seek the body back to the original point in for a retry so that
// send will send the body's contents again in the upcoming request.
r.Body.Seek(r.bodyStart, 0)
}
r.Retryable.Reset()

r.Handlers.Send.Run(r)
if r.Error != nil {
return r.Error
Expand Down
50 changes: 50 additions & 0 deletions aws/types.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package aws

import (
"fmt"
"io"
"time"
)
Expand Down Expand Up @@ -61,3 +62,52 @@ func (r ReaderSeekerCloser) Close() error {
}
return nil
}

// A SettableBool provides a boolean value which includes the state if
// the value was set or unset. The set state is in addition to the value's
// value(true|false)
type SettableBool struct {
value bool
set bool
}

// SetBool returns a SettableBool with a value set
func SetBool(value bool) SettableBool {
return SettableBool{value: value, set: true}
}

// Get returns the value. Will always be false if the SettableBool was not set.
func (b *SettableBool) Get() bool {
if !b.set {
return false
}
return b.value
}

// Set sets the value and updates the state that the value has been set.
func (b *SettableBool) Set(value bool) {
b.value = value
b.set = true
}

// IsSet returns if the value has been set
func (b *SettableBool) IsSet() bool {
return b.set
}

// Reset resets the state and value of the SettableBool to its initial default
// state of not set and zero value.
func (b *SettableBool) Reset() {
b.value = false
b.set = false
}

// String returns the string representation of the value if set. Zero if not set.
func (b *SettableBool) String() string {
return fmt.Sprintf("%t", b.Get())
}

// GoString returns the string representation of the SettableBool value and state
func (b *SettableBool) GoString() string {
return fmt.Sprintf("Bool{value:%t, set:%t}", b.value, b.set)
}
2 changes: 1 addition & 1 deletion service/dynamodb/customizations.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func validateCRC32(r *aws.Request) {

if crc != uint32(expected) {
// CRC does not match, set a retryable error
r.Retryable = true
r.Retryable.Set(true)
r.Error = &aws.APIError{
Code: "CRC32CheckFailed",
Message: "CRC32 integrity check failed",
Expand Down
2 changes: 1 addition & 1 deletion service/sqs/checksums.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func checksumsMatch(body, expectedMD5 *string) error {
}

func setChecksumError(r *aws.Request, format string, args ...interface{}) {
r.Retryable = true
r.Retryable.Set(true)
r.Error = &aws.APIError{
StatusCode: r.HTTPResponse.StatusCode,
Code: "InvalidChecksum",
Expand Down

0 comments on commit cc754a4

Please sign in to comment.