Skip to content

Commit

Permalink
service/s3/s3crypto Fix request retries (#3344)
Browse files Browse the repository at this point in the history
  • Loading branch information
gwatts authored Jul 24, 2020
1 parent a418ccb commit 698e6f0
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
2 changes: 2 additions & 0 deletions CHANGELOG_PENDING.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@
### SDK Enhancements

### SDK Bugs
* `service/s3/s3crypto`: Fix client's temporary file buffer error on retry ([#3344](https://github.com/aws/aws-sdk-go/pull/3344))
* Fixes the Crypto client's temporary file buffer cleanup returning an error when the request is retried.
2 changes: 1 addition & 1 deletion service/s3/s3crypto/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ func getWriterStore(req *request.Request, path string, useTempFile bool) (io.Rea
return nil, err
}

req.Handlers.Send.PushBack(func(r *request.Request) {
req.Handlers.Complete.PushBack(func(r *request.Request) {
// Close the temp file and cleanup
f.Close()
os.Remove(f.Name())
Expand Down
38 changes: 38 additions & 0 deletions service/s3/s3crypto/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package s3crypto

import (
"bytes"
"io/ioutil"
"net/http"
"os"
"testing"
Expand Down Expand Up @@ -118,6 +119,43 @@ func TestGetWriterStore_TempFile(t *testing.T) {
}
}

func TestGetWriterStore_TempFileWithRetry(t *testing.T) {
responses := []*http.Response{
{StatusCode: 500, Header: http.Header{}, Body: ioutil.NopCloser(&bytes.Buffer{})},
{StatusCode: 200, Header: http.Header{}, Body: ioutil.NopCloser(&bytes.Buffer{})},
}
s := awstesting.NewClient(aws.NewConfig().WithMaxRetries(10))
s.Handlers.Validate.Clear()
s.Handlers.Send.Clear() // mock sending
s.Handlers.Send.PushBack(func(r *request.Request) {
r.HTTPResponse = responses[0]
responses = responses[1:]
})
type testData struct {
Data string
}
out := &testData{}
r := s.NewRequest(&request.Operation{Name: "Operation"}, nil, out)
f, err := getWriterStore(r, "", true)
if err != nil {
t.Fatalf("expected no error, but received %v", err)
}
tempFile, ok := f.(*os.File)
if !ok {
t.Fatal("io.ReadWriteSeeker expected to be *os.file")
}
err = r.Send()
if err != nil {
t.Errorf("expected no error, but received %v", err)
}
if _, err := os.Stat(tempFile.Name()); !os.IsNotExist(err) {
t.Errorf("expected temp file be deleted, but still exists %v", tempFile.Name())
}
if v := len(responses); v != 0 {
t.Errorf("expect all retries to be used, have %v remaining", v)
}
}

func TestGetWriterStore_Memory(t *testing.T) {
response := http.Response{StatusCode: 200}
s := awstesting.NewClient(aws.NewConfig().WithMaxRetries(10))
Expand Down

0 comments on commit 698e6f0

Please sign in to comment.