diff --git a/storage/example_test.go b/storage/example_test.go index 467891bcda47..a188b45ca9e5 100644 --- a/storage/example_test.go +++ b/storage/example_test.go @@ -17,6 +17,7 @@ package storage_test import ( "bytes" "context" + "errors" "fmt" "hash/crc32" "io" @@ -878,7 +879,7 @@ func ExampleBucketHandle_exists() { } attrs, err := client.Bucket("my-bucket").Attrs(ctx) - if err == storage.ErrBucketNotExist { + if errors.Is(err, storage.ErrBucketNotExist) { fmt.Println("The bucket does not exist") return } @@ -896,7 +897,7 @@ func ExampleObjectHandle_exists() { } attrs, err := client.Bucket("my-bucket").Object("my-object").Attrs(ctx) - if err == storage.ErrObjectNotExist { + if errors.Is(err, storage.ErrObjectNotExist) { fmt.Println("The object does not exist") return } diff --git a/storage/grpc_client.go b/storage/grpc_client.go index 35c5c9fb8b7d..fa9fcc2b8bac 100644 --- a/storage/grpc_client.go +++ b/storage/grpc_client.go @@ -305,17 +305,11 @@ func (c *grpcStorageClient) GetBucket(ctx context.Context, bucket string, conds var battrs *BucketAttrs err := run(ctx, func(ctx context.Context) error { res, err := c.raw.GetBucket(ctx, req, s.gax...) - battrs = newBucketFromProto(res) - return err }, s.retry, s.idempotent) - if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, ErrBucketNotExist - } - - return battrs, err + return battrs, formatBucketError(err) } func (c *grpcStorageClient) UpdateBucket(ctx context.Context, bucket string, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) { s := callSettings(c.settings, opts...) @@ -474,10 +468,7 @@ func (c *grpcStorageClient) ListObjects(ctx context.Context, bucket string, q *Q return err }, s.retry, s.idempotent) if err != nil { - if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound { - err = ErrBucketNotExist - } - return "", err + return "", formatBucketError(err) } for _, obj := range objects { @@ -519,7 +510,7 @@ func (c *grpcStorageClient) DeleteObject(ctx context.Context, bucket, object str return c.raw.DeleteObject(ctx, req, s.gax...) }, s.retry, s.idempotent) if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return ErrObjectNotExist + return formatObjectErr(err) } return err } @@ -554,7 +545,7 @@ func (c *grpcStorageClient) GetObject(ctx context.Context, params *getObjectPara }, s.retry, s.idempotent) if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, ErrObjectNotExist + return nil, formatObjectErr(err) } return attrs, err @@ -650,7 +641,7 @@ func (c *grpcStorageClient) UpdateObject(ctx context.Context, params *updateObje return err }, s.retry, s.idempotent) if e, ok := status.FromError(err); ok && e.Code() == codes.NotFound { - return nil, ErrObjectNotExist + return nil, formatObjectErr(err) } return attrs, err @@ -677,7 +668,7 @@ func (c *grpcStorageClient) RestoreObject(ctx context.Context, params *restoreOb return err }, s.retry, s.idempotent) if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, ErrObjectNotExist + return nil, formatObjectErr(err) } return attrs, err } @@ -707,7 +698,7 @@ func (c *grpcStorageClient) MoveObject(ctx context.Context, params *moveObjectPa return err }, s.retry, s.idempotent) if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, ErrObjectNotExist + return nil, formatObjectErr(err) } return attrs, err } @@ -950,7 +941,7 @@ func (c *grpcStorageClient) ComposeObject(ctx context.Context, req *composeObjec return err }, s.retry, s.idempotent); err != nil { if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err) + return nil, formatObjectErr(err) } return nil, err } @@ -1002,7 +993,7 @@ func (c *grpcStorageClient) RewriteObject(ctx context.Context, req *rewriteObjec if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil { if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err) + return nil, formatObjectErr(err) } return nil, err } @@ -1584,7 +1575,7 @@ func (c *grpcStorageClient) NewRangeReader(ctx context.Context, params *newRange // These types of errors show up on the RecvMsg call, rather than the // initialization of the stream via BidiReadObject above. if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return ErrObjectNotExist + return formatObjectErr(err) } if err != nil { return err diff --git a/storage/grpc_reader.go b/storage/grpc_reader.go index e1dd397819bb..eaa35fea6316 100644 --- a/storage/grpc_reader.go +++ b/storage/grpc_reader.go @@ -26,10 +26,8 @@ import ( "cloud.google.com/go/storage/internal/apiv2/storagepb" "github.com/googleapis/gax-go/v2" "google.golang.org/grpc" - "google.golang.org/grpc/codes" "google.golang.org/grpc/encoding" "google.golang.org/grpc/mem" - "google.golang.org/grpc/status" "google.golang.org/protobuf/encoding/protowire" "google.golang.org/protobuf/proto" ) @@ -146,13 +144,10 @@ func (c *grpcStorageClient) NewRangeReaderReadObject(ctx context.Context, params // use a custom decoder to avoid an extra copy at the protobuf layer. databufs := mem.BufferSlice{} err := stream.RecvMsg(&databufs) - // These types of errors show up on the Recv call, rather than the - // initialization of the stream via ReadObject above. - if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound { - return ErrObjectNotExist - } if err != nil { - return err + // NotFound types of errors show up on the Recv call, rather than the + // initialization of the stream via ReadObject above. + return formatObjectErr(err) } // Use a custom decoder that uses protobuf unmarshalling for all // fields except the object data. Object data is handled separately diff --git a/storage/http_client.go b/storage/http_client.go index f29de2fef978..93a2f901973c 100644 --- a/storage/http_client.go +++ b/storage/http_client.go @@ -287,12 +287,8 @@ func (c *httpStorageClient) GetBucket(ctx context.Context, bucket string, conds return err }, s.retry, s.idempotent) - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - return nil, ErrBucketNotExist - } if err != nil { - return nil, err + return nil, formatBucketError(err) } return newBucket(resp) } @@ -382,11 +378,7 @@ func (c *httpStorageClient) ListObjects(ctx context.Context, bucket string, q *Q return err }, s.retry, s.idempotent) if err != nil { - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - err = ErrBucketNotExist - } - return "", err + return "", formatBucketError(err) } for _, item := range resp.Items { it.items = append(it.items, newObject(item)) @@ -416,11 +408,7 @@ func (c *httpStorageClient) DeleteObject(ctx context.Context, bucket, object str req.UserProject(s.userProject) } err := run(ctx, func(ctx context.Context) error { return req.Context(ctx).Do() }, s.retry, s.idempotent) - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - return ErrObjectNotExist - } - return err + return formatObjectErr(err) } func (c *httpStorageClient) GetObject(ctx context.Context, params *getObjectParams, opts ...storageOption) (*ObjectAttrs, error) { @@ -445,12 +433,8 @@ func (c *httpStorageClient) GetObject(ctx context.Context, params *getObjectPara obj, err = req.Context(ctx).Do() return err }, s.retry, s.idempotent) - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - return nil, ErrObjectNotExist - } if err != nil { - return nil, err + return nil, formatObjectErr(err) } return newObject(obj), nil } @@ -555,12 +539,8 @@ func (c *httpStorageClient) UpdateObject(ctx context.Context, params *updateObje var obj *raw.Object var err error err = run(ctx, func(ctx context.Context) error { obj, err = call.Context(ctx).Do(); return err }, s.retry, s.idempotent) - var e *googleapi.Error - if errors.As(err, &e) && e.Code == http.StatusNotFound { - return nil, ErrObjectNotExist - } if err != nil { - return nil, err + return nil, formatObjectErr(err) } return newObject(obj), nil } @@ -585,9 +565,8 @@ func (c *httpStorageClient) RestoreObject(ctx context.Context, params *restoreOb var obj *raw.Object var err error err = run(ctx, func(ctx context.Context) error { obj, err = req.Context(ctx).Do(); return err }, s.retry, s.idempotent) - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - return nil, ErrObjectNotExist + if err != nil { + return nil, formatObjectErr(err) } return newObject(obj), err } @@ -610,9 +589,8 @@ func (c *httpStorageClient) MoveObject(ctx context.Context, params *moveObjectPa var obj *raw.Object var err error err = run(ctx, func(ctx context.Context) error { obj, err = req.Context(ctx).Do(); return err }, s.retry, s.idempotent) - var e *googleapi.Error - if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound { - return nil, ErrObjectNotExist + if err != nil { + return nil, formatObjectErr(err) } return newObject(obj), err } @@ -800,11 +778,7 @@ func (c *httpStorageClient) ComposeObject(ctx context.Context, req *composeObjec retryCall := func(ctx context.Context) error { obj, err = call.Context(ctx).Do(); return err } if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil { - var e *googleapi.Error - if errors.As(err, &e) && e.Code == http.StatusNotFound { - return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err) - } - return nil, err + return nil, formatObjectErr(err) } return newObject(obj), nil } @@ -851,11 +825,7 @@ func (c *httpStorageClient) RewriteObject(ctx context.Context, req *rewriteObjec retryCall := func(ctx context.Context) error { res, err = call.Context(ctx).Do(); return err } if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil { - var e *googleapi.Error - if errors.As(err, &e) && e.Code == http.StatusNotFound { - return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err) - } - return nil, err + return nil, formatObjectErr(err) } r := &rewriteObjectResponse{ @@ -1420,13 +1390,7 @@ func readerReopen(ctx context.Context, header http.Header, params *newRangeReade err = run(ctx, func(ctx context.Context) error { res, err = doDownload(ctx) if err != nil { - var e *googleapi.Error - if errors.As(err, &e) { - if e.Code == http.StatusNotFound { - return ErrObjectNotExist - } - } - return err + return formatObjectErr(err) } if res.StatusCode == http.StatusNotFound { @@ -1435,7 +1399,7 @@ func readerReopen(ctx context.Context, header http.Header, params *newRangeReade return ErrObjectNotExist } if res.StatusCode < 200 || res.StatusCode > 299 { - body, _ := ioutil.ReadAll(res.Body) + body, _ := io.ReadAll(res.Body) res.Body.Close() return &googleapi.Error{ Code: res.StatusCode, diff --git a/storage/integration_test.go b/storage/integration_test.go index 00fd5f32a867..f2aa5ba69d06 100644 --- a/storage/integration_test.go +++ b/storage/integration_test.go @@ -934,7 +934,7 @@ func TestIntegration_BucketCreateDelete(t *testing.T) { t.Fatalf("BucketHandle.Delete(%q): %v", newBucketName, err) } _, err = b.Attrs(ctx) - if err != ErrBucketNotExist { + if !errors.Is(err, ErrBucketNotExist) { t.Fatalf("expected ErrBucketNotExist, got %v", err) } }) @@ -2442,7 +2442,7 @@ func TestIntegration_Encoding(t *testing.T) { // Test NotFound. _, err = bkt.Object("obj-not-exists").NewReader(ctx) - if err != ErrObjectNotExist { + if !errors.Is(err, ErrObjectNotExist) { t.Errorf("Object should not exist, err found to be %v", err) } }) @@ -3325,11 +3325,11 @@ func TestIntegration_NonexistentBucket(t *testing.T) { ctx := skipExtraReadAPIs(context.Background(), "no reads in test") multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, _, prefix string, client *Client) { bkt := client.Bucket(prefix + uidSpace.New()) - if _, err := bkt.Attrs(ctx); err != ErrBucketNotExist { + if _, err := bkt.Attrs(ctx); !errors.Is(err, ErrBucketNotExist) { t.Errorf("Attrs: got %v, want ErrBucketNotExist", err) } it := bkt.Objects(ctx, nil) - if _, err := it.Next(); err != ErrBucketNotExist { + if _, err := it.Next(); !errors.Is(err, ErrBucketNotExist) { t.Errorf("Objects: got %v, want ErrBucketNotExist", err) } }) diff --git a/storage/storage.go b/storage/storage.go index df52af4f876e..83962f1ab6ef 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -53,8 +53,10 @@ import ( raw "google.golang.org/api/storage/v1" "google.golang.org/api/transport" htransport "google.golang.org/api/transport/http" + "google.golang.org/grpc/codes" "google.golang.org/grpc/experimental/stats" "google.golang.org/grpc/stats/opentelemetry" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/reflect/protoreflect" "google.golang.org/protobuf/types/known/fieldmaskpb" @@ -65,9 +67,11 @@ import ( var signedURLMethods = map[string]bool{"DELETE": true, "GET": true, "HEAD": true, "POST": true, "PUT": true} var ( - // ErrBucketNotExist indicates that the bucket does not exist. + // ErrBucketNotExist indicates that the bucket does not exist. It should be + // checked for using [errors.Is] instead of direct equality. ErrBucketNotExist = errors.New("storage: bucket doesn't exist") - // ErrObjectNotExist indicates that the object does not exist. + // ErrObjectNotExist indicates that the object does not exist. It should be + // checked for using [errors.Is] instead of direct equality. ErrObjectNotExist = errors.New("storage: object doesn't exist") // errMethodNotSupported indicates that the method called is not currently supported by the client. // TODO: Export this error when launching the transport-agnostic client. @@ -2619,3 +2623,25 @@ func applyCondsProto(method string, gen int64, conds *Conditions, msg proto.Mess } return nil } + +// formatObjectErr checks if the provided error is NotFound and if so, wraps +// it in an ErrObjectNotExist error. If not, formatObjectErr has no effect. +func formatObjectErr(err error) error { + var e *googleapi.Error + if s, ok := status.FromError(err); (ok && s.Code() == codes.NotFound) || + (errors.As(err, &e) && e.Code == http.StatusNotFound) { + return fmt.Errorf("%w: %w", ErrObjectNotExist, err) + } + return err +} + +// formatBucketError checks if the provided error is NotFound and if so, wraps +// it in an ErrBucketNotExist error. If not, formatBucketError has no effect. +func formatBucketError(err error) error { + var e *googleapi.Error + if s, ok := status.FromError(err); (ok && s.Code() == codes.NotFound) || + (errors.As(err, &e) && e.Code == http.StatusNotFound) { + return fmt.Errorf("%w: %w", ErrBucketNotExist, err) + } + return err +} diff --git a/storage/transfermanager/integration_test.go b/storage/transfermanager/integration_test.go index a1ba80746475..d3af64d21741 100644 --- a/storage/transfermanager/integration_test.go +++ b/storage/transfermanager/integration_test.go @@ -560,7 +560,7 @@ func TestIntegration_DownloaderErrorSync(t *testing.T) { // Check that the nonexistent object returned an error. if got.Object == nonexistentObject { - if got.Err != storage.ErrObjectNotExist { + if !errors.Is(got.Err, storage.ErrObjectNotExist) { t.Errorf("Object(%q) should not exist, err found to be %v", got.Object, got.Err) } continue @@ -718,7 +718,7 @@ func TestIntegration_DownloaderErrorAsync(t *testing.T) { callbackMu.Unlock() // Check that the nonexistent object returned an error. - if got.Err != storage.ErrObjectNotExist { + if !errors.Is(got.Err, storage.ErrObjectNotExist) { t.Errorf("Object(%q) should not exist, err found to be %v", got.Object, got.Err) } },