Skip to content

Commit

Permalink
fix(gensupport): allow initial request for resumable uploads to retry…
Browse files Browse the repository at this point in the history
… w/ non-nil getBody (googleapis#1690)

This allows retries when initiating a resumable upload session.

Add support in media.go to return a getBody function so the request body can be recreated during retry attempts.

Tests pass locally in google-cloud-go/storage

Updates googleapis/google-cloud-go#6652

Co-authored-by: Chris Cotter <cjcotter@google.com>
  • Loading branch information
2 people authored and Rui Hu committed Nov 4, 2022
1 parent d8d1dd6 commit 1b62b8e
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
29 changes: 20 additions & 9 deletions internal/gensupport/media.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,13 +289,12 @@ func (mi *MediaInfo) UploadRequest(reqHeaders http.Header, body io.Reader) (newB
// be retried because the data is stored in the MediaBuffer.
media, _, _, _ = mi.buffer.Chunk()
}
toCleanup := []io.Closer{}
if media != nil {
fb := readerFunc(body)
fm := readerFunc(media)
combined, ctype := CombineBodyMedia(body, "application/json", media, mi.mType)
toCleanup := []io.Closer{
combined,
}
toCleanup = append(toCleanup, combined)
if fb != nil && fm != nil {
getBody = func() (io.ReadCloser, error) {
rb := ioutil.NopCloser(fb())
Expand All @@ -309,18 +308,30 @@ func (mi *MediaInfo) UploadRequest(reqHeaders http.Header, body io.Reader) (newB
return r, nil
}
}
cleanup = func() {
for _, closer := range toCleanup {
_ = closer.Close()
}

}
reqHeaders.Set("Content-Type", ctype)
body = combined
}
if mi.buffer != nil && mi.mType != "" && !mi.singleChunk {
// This happens when initiating a resumable upload session.
// The initial request contains a JSON body rather than media.
// It can be retried with a getBody function that re-creates the request body.
fb := readerFunc(body)
if fb != nil {
getBody = func() (io.ReadCloser, error) {
rb := ioutil.NopCloser(fb())
toCleanup = append(toCleanup, rb)
return rb, nil
}
}
reqHeaders.Set("X-Upload-Content-Type", mi.mType)
}
// Ensure that any bodies created in getBody are cleaned up.
cleanup = func() {
for _, closer := range toCleanup {
_ = closer.Close()
}

}
return body, getBody, cleanup
}

Expand Down
5 changes: 2 additions & 3 deletions internal/gensupport/media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -309,12 +309,11 @@ func TestUploadRequestGetBody(t *testing.T) {
wantGetBody: true,
},
{
desc: "chunk size < data size: MediaBuffer, >1 chunk, no getBody",
// No getBody here, because the initial request contains no media data
desc: "chunk size < data size: MediaBuffer, >1 chunk, getBody",
// Note that ChunkSize = 1 is rounded up to googleapi.MinUploadChunkSize.
r: &nullReader{2 * googleapi.MinUploadChunkSize},
chunkSize: 1,
wantGetBody: false,
wantGetBody: true,
},
} {
cryptorand.Reader = mathrand.New(mathrand.NewSource(int64(i)))
Expand Down

0 comments on commit 1b62b8e

Please sign in to comment.