Retry GCS requests Go's HTTP/2 client ends with its connection

README.md

@@ -13,7 +13,7 @@ Object storage on S3 and S3-compatible stores, GCS, Azure Blob, OCI Object Stora
 - **S3 semantics** where stores differ. `DeleteObjects` of a missing path succeeds, as S3's delete does. It tries every path, and when some are not deleted fails with a `*DeleteError` whose `Failed` names each one with its error (`errors.As`); every other path is deleted, and a retry is safe.
 - **Keys** are the store's keys as given. The local directory, as a filesystem, drops a leading `/`, cleans `//` and `.`, refuses keys outside it, and cannot hold both `a` and `a/b`.
 - **Timeouts**: a connection that moves no bytes either way for 5 minutes is cut. An HTTP/2 connection with no inbound frame for 30 seconds is pinged and cut when the ping goes unanswered for 15 seconds, so a dead peer fails every request on it within 45 seconds with an error the SDKs retry. The ping waits behind the frame being written, so a peer accepting less than one 16 KiB frame per 15 seconds is cut as well; any faster transfer, however slow, never is. The context bounds the call.
-- **Retries**: every store retries throttling, 5xx and network errors with backoff, including AliOSS, whose SDK does not. A stream given to `UploadObject` is sent once on AliOSS.
+- **Retries**: every store retries throttling, 5xx and network errors with backoff, including AliOSS, whose SDK does not. GCS also retries the errors with which Go's HTTP/2 client ends every request on a connection it is closing (force closed, or the server's GOAWAY), which the SDK does not. A stream given to `UploadObject` is sent once on AliOSS.
 - **Proxy**: `ProxyConfig` (S3, GCS) carries its credentials for http and https targets, honors `NO_PROXY`, and never proxies the cloud metadata services.
 - **Local writes** go in place, as a file is written, so mounted volumes and file watchers see one write; `IfAbsent` writes a temporary file (`.upload-*`, never listed) and links it into place.
 

gcp.go

@@ -120,9 +120,25 @@ func NewGCP(conf *GCPConfig) (Storage, error) {
 	if u.client, err = storage.NewClient(ctx, opts...); err != nil {
 		return nil, err
 	}
+	u.client.SetRetry(storage.WithErrorFunc(gcsShouldRetry))
 	return u, nil
 }
 
+// gcsShouldRetry is the SDK's predicate plus the errors with which Go's HTTP/2
+// client ends every request on a connection it is closing. Such a request may
+// have been received, which an overwrite or a precondition tolerates.
+func gcsShouldRetry(err error) bool {
+	if err == nil {
+		return false
+	}
+	if storage.ShouldRetry(err) {
+		return true
+	}
+	msg := err.Error()
+	return strings.Contains(msg, "http2: client connection force closed") ||
+		strings.Contains(msg, "http2: server sent GOAWAY and closed the connection")
+}
+
 func (s *gcpStorage) UploadFile(ctx context.Context, storagePath, filepath string, opts PutOptions) (string, int64, error) {
 	file, err := os.Open(filepath)
 	if err != nil {

transport_test.go

@@ -17,10 +17,12 @@ package storage
 import (
 	"context"
 	"crypto/tls"
+	"errors"
 	"io"
 	"net"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"os"
 	"testing"
 	"time"
@@ -29,6 +31,52 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+// The errors with which Go's HTTP/2 client ends every request on a connection
+// it is closing are retried on GCS, where the SDK's own predicate leaves them.
+func TestGCSRetriesConnectionClose(t *testing.T) {
+	t.Run("server GOAWAY", func(t *testing.T) {
+		// The server completes the handshake, takes the request's HEADERS,
+		// then says goodbye with the request's stream still open and closes.
+		srv, client := h2Server(t, nil, func(_ *http.Server, c *tls.Conn, _ http.Handler) {
+			_, _ = c.Write([]byte{0, 0, 0, 0x4, 0, 0, 0, 0, 0}) // an empty SETTINGS frame
+			_, _ = io.ReadFull(c, make([]byte, 24))          // the client preface
+			hdr := make([]byte, 9)
+			for {
+				if _, err := io.ReadFull(c, hdr); err != nil {
+					return
+				}
+				payload := make([]byte, int(hdr[0])<<16|int(hdr[1])<<8|int(hdr[2]))
+				if _, err := io.ReadFull(c, payload); err != nil {
+					return
+				}
+				if hdr[3] == 0x1 { // HEADERS
+					break
+				}
+			}
+			// GOAWAY: last stream 1, NO_ERROR.
+			_, _ = c.Write([]byte{0, 0, 8, 0x7, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0})
+		})
+
+		_, err := client.Get(srv.URL)
+		require.ErrorContains(t, err, "http2: server sent GOAWAY and closed the connection")
+		require.False(t, gcs.ShouldRetry(err), "the SDK's predicate, which this package adds to")
+		require.True(t, gcsShouldRetry(err))
+	})
+
+	t.Run("client force closed", func(t *testing.T) {
+		err := &url.Error{Op: "Post", URL: "https://storage.googleapis.com/upload/storage/v1/b/bucket/o",
+			Err: errors.New("http2: client connection force closed via ClientConn.Close")}
+		require.False(t, gcs.ShouldRetry(err))
+		require.True(t, gcsShouldRetry(err))
+	})
+
+	t.Run("unchanged otherwise", func(t *testing.T) {
+		require.False(t, gcsShouldRetry(nil))
+		require.False(t, gcsShouldRetry(gcs.ErrObjectNotExist))
+		require.True(t, gcsShouldRetry(&url.Error{Op: "Post", URL: "https://storage.googleapis.com", Err: errors.New("read: connection reset by peer")}))
+	})
+}
+
 // A dead HTTP/2 peer, one that keeps the connection open but answers nothing,
 // not even pings, fails the request well before the idle timeout, with an
 // error the GCS SDK retries. A live one that is merely slow is never cut.