chore(storage): remove runWithRetry (#5255)

Co-authored-by: Chris Cotter <cjcotter@google.com>
googleapis · Jan 6, 2022 · bc97804 · bc97804
1 parent bf8795a
commit bc97804
Show file tree

Hide file tree

Showing 2 changed files with 89 additions and 39 deletions.
diff --git a/storage/invoke.go b/storage/invoke.go
@@ -55,21 +55,6 @@ func run(ctx context.Context, call func() error, retry *retryConfig, isIdempoten
 	})
 }
 
-// runWithRetry calls the function until it returns nil or a non-retryable error, or
-// the context is done.
-func runWithRetry(ctx context.Context, call func() error) error {
-	return internal.Retry(ctx, gax.Backoff{}, func() (stop bool, err error) {
-		err = call()
-		if err == nil {
-			return true, nil
-		}
-		if shouldRetry(err) {
-			return false, err
-		}
-		return true, err
-	})
-}
-
 func shouldRetry(err error) bool {
 	if err == nil {
 		return false

diff --git a/storage/invoke_test.go b/storage/invoke_test.go
@@ -33,32 +33,97 @@ func TestInvoke(t *testing.T) {
 	// returns with the right error.
 
 	for _, test := range []struct {
-		count      int   // Number of times to return retryable error.
-		initialErr error // Error to return initially.
-		finalErr   error // Error to return after count returns of retryCode.
+		count             int   // Number of times to return retryable error.
+		initialErr        error // Error to return initially.
+		finalErr          error // Error to return after count returns of retryCode.
+		retry             *retryConfig
+		isIdempotentValue bool
+		expectFinalErr    bool
 	}{
-		{0, &googleapi.Error{Code: 0}, nil},
-		{0, &googleapi.Error{Code: 0}, errors.New("foo")},
-		{1, &googleapi.Error{Code: 429}, nil},
-		{1, &googleapi.Error{Code: 429}, errors.New("bar")},
-		{2, &googleapi.Error{Code: 518}, nil},
-		{2, &googleapi.Error{Code: 599}, &googleapi.Error{Code: 428}},
-		{1, &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")}, nil},
-		{1, io.ErrUnexpectedEOF, nil},
-		{1, xerrors.Errorf("Test unwrapping of a temporary error: %w", &googleapi.Error{Code: 500}), nil},
-		{0, xerrors.Errorf("Test unwrapping of a non-retriable error: %w", &googleapi.Error{Code: 400}), &googleapi.Error{Code: 400}},
+		{
+			count:             0,
+			initialErr:        &googleapi.Error{Code: 0},
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             0,
+			initialErr:        &googleapi.Error{Code: 0},
+			finalErr:          errors.New("foo"),
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             1,
+			initialErr:        &googleapi.Error{Code: 429},
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             1,
+			initialErr:        &googleapi.Error{Code: 429},
+			finalErr:          errors.New("bar"),
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             2,
+			initialErr:        &googleapi.Error{Code: 518},
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             2,
+			initialErr:        &googleapi.Error{Code: 599},
+			finalErr:          &googleapi.Error{Code: 428},
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             1,
+			initialErr:        &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")},
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             1,
+			initialErr:        io.ErrUnexpectedEOF,
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             1,
+			initialErr:        xerrors.Errorf("Test unwrapping of a temporary error: %w", &googleapi.Error{Code: 500}),
+			finalErr:          nil,
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
+		{
+			count:             0,
+			initialErr:        xerrors.Errorf("Test unwrapping of a non-retriable error: %w", &googleapi.Error{Code: 400}),
+			finalErr:          &googleapi.Error{Code: 400},
+			isIdempotentValue: true,
+			expectFinalErr:    true,
+		},
 	} {
-		counter := 0
-		call := func() error {
-			counter++
-			if counter <= test.count {
-				return test.initialErr
+		t.Run("", func(s *testing.T) {
+			counter := 0
+			call := func() error {
+				counter++
+				if counter <= test.count {
+					return test.initialErr
+				}
+				return test.finalErr
 			}
-			return test.finalErr
-		}
-		got := runWithRetry(ctx, call)
-		if got != test.finalErr {
-			t.Errorf("%+v: got %v, want %v", test, got, test.finalErr)
-		}
+			got := run(ctx, call, test.retry, test.isIdempotentValue)
+			if got != test.finalErr {
+				s.Errorf("%+v: got %v, want %v", test, got, test.finalErr)
+			}
+		})
 	}
 }