diff --git a/storage/client_test.go b/storage/client_test.go index 6731e30f7940..4124c22d0a0f 100644 --- a/storage/client_test.go +++ b/storage/client_test.go @@ -16,8 +16,10 @@ package storage import ( "context" + "errors" "fmt" "log" + "net/url" "os" "strconv" "strings" @@ -26,7 +28,10 @@ import ( "cloud.google.com/go/iam/apiv1/iampb" "github.com/google/go-cmp/cmp" + "github.com/googleapis/gax-go/v2/apierror" + "github.com/googleapis/gax-go/v2/callctx" "google.golang.org/api/iterator" + "google.golang.org/grpc/codes" ) var emulatorClients map[string]storageClient @@ -1342,6 +1347,47 @@ func TestObjectConditionsEmulated(t *testing.T) { }) } +// Test that RetryNever prevents any retries from happening in both transports. +func TestRetryNeverEmulated(t *testing.T) { + transportClientTest(t, func(t *testing.T, project, bucket string, client storageClient) { + ctx := context.Background() + + attrs, err := client.CreateBucket(ctx, project, bucket, &BucketAttrs{}, nil) + if err != nil { + t.Fatalf("creating bucket: %v", err) + } + + // Need the HTTP hostname to set up a retry test, as well as knowledge of + // underlying transport to specify instructions. + host := os.Getenv("STORAGE_EMULATOR_HOST") + endpoint, err := url.Parse(host) + if err != nil { + t.Fatalf("parsing endpoint: %v", err) + } + var transport string + if _, ok := client.(*httpStorageClient); ok { + transport = "http" + } else { + transport = "grpc" + } + + et := emulatorTest{T: t, name: "testRetryNever", resources: resources{}, + host: endpoint} + et.create(map[string][]string{"storage.buckets.get": {"return-503"}}, transport) + ctx = callctx.SetHeaders(ctx, "x-retry-test-id", et.id) + _, err = client.GetBucket(ctx, attrs.Name, nil, withRetryConfig(&retryConfig{policy: RetryNever})) + + var ae *apierror.APIError + if errors.As(err, &ae) { + // We espect a 503/UNAVAILABLE error. For anything else including a nil + // error, the test should fail. + if ae.GRPCStatus().Code() != codes.Unavailable && ae.HTTPCode() != 503 { + t.Errorf("GetBucket: got unexpected error %v; want 503", err) + } + } + }) +} + // createObject creates an object in the emulator and returns its name, generation, and // metageneration. func createObject(ctx context.Context, bucket string) (string, int64, int64, error) { diff --git a/storage/grpc_client.go b/storage/grpc_client.go index e337213f03f2..1f34dff53029 100644 --- a/storage/grpc_client.go +++ b/storage/grpc_client.go @@ -116,6 +116,8 @@ type grpcStorageClient struct { func newGRPCStorageClient(ctx context.Context, opts ...storageOption) (storageClient, error) { s := initSettings(opts...) s.clientOption = append(defaultGRPCOptions(), s.clientOption...) + // Disable all gax-level retries in favor of retry logic in the veneer client. + s.gax = append(s.gax, gax.WithRetry(nil)) config := newStorageConfig(s.clientOption...) if config.readAPIWasSet { diff --git a/storage/retry_conformance_test.go b/storage/retry_conformance_test.go index 950b542e2c87..b6f7de8c4229 100644 --- a/storage/retry_conformance_test.go +++ b/storage/retry_conformance_test.go @@ -576,6 +576,9 @@ var methods = map[string][]retryFunc{ } func TestRetryConformance(t *testing.T) { + // This endpoint is used only to call the testbench retry test API, which is HTTP + // based. The endpoint called by the client library is determined inside of the + // client constructor and will differ depending on the transport. host := os.Getenv("STORAGE_EMULATOR_HOST") if host == "" { t.Skip("This test must use the testbench emulator; set STORAGE_EMULATOR_HOST to run.") @@ -759,7 +762,7 @@ func (et *emulatorTest) create(instructions map[string][]string, transport strin et.host.Path = "retry_test" resp, err := c.Post(et.host.String(), "application/json", buf) - if resp.StatusCode == 501 { + if resp != nil && resp.StatusCode == 501 { et.T.Skip("This retry test case is not yet supported in the testbench.") } if err != nil || resp.StatusCode != 200 {