fix(gensupport): allow initial request for resumable uploads to retry w/ non-nil getBody (#1690)

cojenco · tritone · web-flow · commit 2c3e8638afc6 · 2022-09-20T17:15:34.000-07:00
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>
diff --git a/internal/gensupport/media.go b/internal/gensupport/media.go
@@ -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())
@@ -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
 }
 
diff --git a/internal/gensupport/media_test.go b/internal/gensupport/media_test.go
@@ -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)))
