feat(storage): export ShouldRetry (#6370)

Export the default func to determine whether an error is retryable.
This makes it easier for users to use the WithErrorFunc option
without copying a lot of code.

Fixes #6362

Author: tritone

storage/grpc_client.go

@@ -1379,7 +1379,7 @@ func (r *gRPCReader) Close() error {
 // an attempt to reopen the stream.
 func (r *gRPCReader) recv() (*storagepb.ReadObjectResponse, error) {
 	msg, err := r.stream.Recv()
-	if err != nil && shouldRetry(err) {
+	if err != nil && ShouldRetry(err) {
 		// This will "close" the existing stream and immediately attempt to
 		// reopen the stream, but will backoff if further attempts are necessary.
 		// Reopening the stream Recvs the first message, so if retrying is
@@ -1559,7 +1559,7 @@ func (w *gRPCWriter) uploadBuffer(recvd int, start int64, doneReading bool) (*st
 			// resend the entire buffer via a new stream.
 			// If not retriable, falling through will return the error received
 			// from closing the stream.
-			if shouldRetry(err) {
+			if ShouldRetry(err) {
 				sent = 0
 				finishWrite = false
 				// TODO: Add test case for failure modes of querying progress.
@@ -1590,7 +1590,7 @@ func (w *gRPCWriter) uploadBuffer(recvd int, start int64, doneReading bool) (*st
 		// resend the entire buffer via a new stream.
 		// If not retriable, falling through will return the error received
 		// from closing the stream.
-		if shouldRetry(err) {
+		if ShouldRetry(err) {
 			sent = 0
 			finishWrite = false
 			offset, err = w.determineOffset(start)

storage/invoke.go

@@ -57,7 +57,7 @@ func run(ctx context.Context, call func() error, retry *retryConfig, isIdempoten
 		bo.Initial = retry.backoff.Initial
 		bo.Max = retry.backoff.Max
 	}
-	var errorFunc func(err error) bool = shouldRetry
+	var errorFunc func(err error) bool = ShouldRetry
 	if retry.shouldRetry != nil {
 		errorFunc = retry.shouldRetry
 	}
@@ -89,7 +89,16 @@ func setRetryHeaderGRPC(_ context.Context) func(string, int) {
 	}
 }
 
-func shouldRetry(err error) bool {
+// ShouldRetry returns true if an error is retryable, based on best practice
+// guidance from GCS. See
+// https://cloud.google.com/storage/docs/retry-strategy#go for more information
+// on what errors are considered retryable.
+//
+// If you would like to customize retryable errors, use the WithErrorFunc to
+// supply a RetryOption to your library calls. For example, to retry additional
+// errors, you can write a custom func that wraps ShouldRetry and also specifies
+// additional errors that should return true.
+func ShouldRetry(err error) bool {
 	if err == nil {
 		return false
 	}
@@ -131,7 +140,7 @@ func shouldRetry(err error) bool {
 	}
 	// Unwrap is only supported in go1.13.x+
 	if e, ok := err.(interface{ Unwrap() error }); ok {
-		return shouldRetry(e.Unwrap())
+		return ShouldRetry(e.Unwrap())
 	}
 	return false
 }

storage/invoke_test.go

@@ -306,7 +306,7 @@ func TestShouldRetry(t *testing.T) {
 		},
 	} {
 		t.Run(test.desc, func(s *testing.T) {
-			got := shouldRetry(test.inputErr)
+			got := ShouldRetry(test.inputErr)
 
 			if got != test.shouldRetry {
 				s.Errorf("got %v, want %v", got, test.shouldRetry)

storage/storage.go

@@ -1875,8 +1875,8 @@ func (ws *withPolicy) apply(config *retryConfig) {
 
 // WithErrorFunc allows users to pass a custom function to the retryer. Errors
 // will be retried if and only if `shouldRetry(err)` returns true.
-// By default, the following errors are retried (see invoke.go for the default
-// shouldRetry function):
+// By default, the following errors are retried (see ShouldRetry for the default
+// function):
 //
 // - HTTP responses with codes 408, 429, 502, 503, and 504.
 //
@@ -1887,7 +1887,8 @@ func (ws *withPolicy) apply(config *retryConfig) {
 // - Wrapped versions of these errors.
 //
 // This option can be used to retry on a different set of errors than the
-// default.
+// default. Users can use the default ShouldRetry function inside their custom
+// function if they only want to make minor modifications to default behavior.
 func WithErrorFunc(shouldRetry func(err error) bool) RetryOption {
 	return &withErrorFunc{
 		shouldRetry: shouldRetry,

storage/storage_test.go

@@ -1139,10 +1139,10 @@ func TestRetryer(t *testing.T) {
 			name: "object retryer configures retry",
 			objectOptions: []RetryOption{
 				WithPolicy(RetryAlways),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 				policy:      RetryAlways,
 			},
 		},
@@ -1155,15 +1155,15 @@ func TestRetryer(t *testing.T) {
 					Multiplier: 6,
 				}),
 				WithPolicy(RetryAlways),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
 				backoff: &gax.Backoff{
 					Initial:    time.Minute,
 					Max:        time.Hour,
 					Multiplier: 6,
 				},
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 				policy:      RetryAlways,
 			},
 		},
@@ -1176,15 +1176,15 @@ func TestRetryer(t *testing.T) {
 					Multiplier: 6,
 				}),
 				WithPolicy(RetryAlways),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
 				backoff: &gax.Backoff{
 					Initial:    time.Minute,
 					Max:        time.Hour,
 					Multiplier: 6,
 				},
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 				policy:      RetryAlways,
 			},
 		},
@@ -1195,11 +1195,11 @@ func TestRetryer(t *testing.T) {
 			},
 			objectOptions: []RetryOption{
 				WithPolicy(RetryNever),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
 				policy:      RetryNever,
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 			},
 		},
 		{
@@ -1209,11 +1209,11 @@ func TestRetryer(t *testing.T) {
 			},
 			objectOptions: []RetryOption{
 				WithPolicy(RetryNever),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
 				policy:      RetryNever,
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 			},
 		},
 		{
@@ -1231,11 +1231,11 @@ func TestRetryer(t *testing.T) {
 					Initial: time.Nanosecond,
 					Max:     time.Microsecond,
 				}),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			want: &retryConfig{
 				policy:      RetryAlways,
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 				backoff: &gax.Backoff{
 					Initial: time.Nanosecond,
 					Max:     time.Microsecond,
@@ -1268,7 +1268,7 @@ func TestRetryer(t *testing.T) {
 			name: "object retryer does not override bucket retryer if option is not set",
 			bucketOptions: []RetryOption{
 				WithPolicy(RetryNever),
-				WithErrorFunc(shouldRetry),
+				WithErrorFunc(ShouldRetry),
 			},
 			objectOptions: []RetryOption{
 				WithBackoff(gax.Backoff{
@@ -1278,7 +1278,7 @@ func TestRetryer(t *testing.T) {
 			},
 			want: &retryConfig{
 				policy:      RetryNever,
-				shouldRetry: shouldRetry,
+				shouldRetry: ShouldRetry,
 				backoff: &gax.Backoff{
 					Initial: time.Nanosecond,
 					Max:     time.Second,