Skip to content

Commit 0dd7d3d

Browse files
BrennaEppBrennaEpp
authored
feat(storage): wrap NotFound errors for buckets and objects (#11519)
Code that checks directly against ErrBucketNotExist and ErrObjectNotExist will break; use `errors.Is()` instead.
1 parent 8fc5a87 commit 0dd7d3d

7 files changed

Lines changed: 63 additions & 86 deletions

File tree

‎storage/example_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package storage_test
1717
import (
1818
"bytes"
1919
"context"
20+
"errors"
2021
"fmt"
2122
"hash/crc32"
2223
"io"
@@ -878,7 +879,7 @@ func ExampleBucketHandle_exists() {
878879
}
879880

880881
attrs, err := client.Bucket("my-bucket").Attrs(ctx)
881-
if err == storage.ErrBucketNotExist {
882+
if errors.Is(err, storage.ErrBucketNotExist) {
882883
fmt.Println("The bucket does not exist")
883884
return
884885
}
@@ -896,7 +897,7 @@ func ExampleObjectHandle_exists() {
896897
}
897898

898899
attrs, err := client.Bucket("my-bucket").Object("my-object").Attrs(ctx)
899-
if err == storage.ErrObjectNotExist {
900+
if errors.Is(err, storage.ErrObjectNotExist) {
900901
fmt.Println("The object does not exist")
901902
return
902903
}

‎storage/grpc_client.go‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -305,17 +305,11 @@ func (c *grpcStorageClient) GetBucket(ctx context.Context, bucket string, conds
305305
var battrs *BucketAttrs
306306
err := run(ctx, func(ctx context.Context) error {
307307
res, err := c.raw.GetBucket(ctx, req, s.gax...)
308-
309308
battrs = newBucketFromProto(res)
310-
311309
return err
312310
}, s.retry, s.idempotent)
313311

314-
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
315-
return nil, ErrBucketNotExist
316-
}
317-
318-
return battrs, err
312+
return battrs, formatBucketError(err)
319313
}
320314
func (c *grpcStorageClient) UpdateBucket(ctx context.Context, bucket string, uattrs *BucketAttrsToUpdate, conds *BucketConditions, opts ...storageOption) (*BucketAttrs, error) {
321315
s := callSettings(c.settings, opts...)
@@ -474,10 +468,7 @@ func (c *grpcStorageClient) ListObjects(ctx context.Context, bucket string, q *Q
474468
return err
475469
}, s.retry, s.idempotent)
476470
if err != nil {
477-
if st, ok := status.FromError(err); ok && st.Code() == codes.NotFound {
478-
err = ErrBucketNotExist
479-
}
480-
return "", err
471+
return "", formatBucketError(err)
481472
}
482473

483474
for _, obj := range objects {
@@ -519,7 +510,7 @@ func (c *grpcStorageClient) DeleteObject(ctx context.Context, bucket, object str
519510
return c.raw.DeleteObject(ctx, req, s.gax...)
520511
}, s.retry, s.idempotent)
521512
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
522-
return ErrObjectNotExist
513+
return formatObjectErr(err)
523514
}
524515
return err
525516
}
@@ -554,7 +545,7 @@ func (c *grpcStorageClient) GetObject(ctx context.Context, params *getObjectPara
554545
}, s.retry, s.idempotent)
555546

556547
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
557-
return nil, ErrObjectNotExist
548+
return nil, formatObjectErr(err)
558549
}
559550

560551
return attrs, err
@@ -650,7 +641,7 @@ func (c *grpcStorageClient) UpdateObject(ctx context.Context, params *updateObje
650641
return err
651642
}, s.retry, s.idempotent)
652643
if e, ok := status.FromError(err); ok && e.Code() == codes.NotFound {
653-
return nil, ErrObjectNotExist
644+
return nil, formatObjectErr(err)
654645
}
655646

656647
return attrs, err
@@ -677,7 +668,7 @@ func (c *grpcStorageClient) RestoreObject(ctx context.Context, params *restoreOb
677668
return err
678669
}, s.retry, s.idempotent)
679670
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
680-
return nil, ErrObjectNotExist
671+
return nil, formatObjectErr(err)
681672
}
682673
return attrs, err
683674
}
@@ -707,7 +698,7 @@ func (c *grpcStorageClient) MoveObject(ctx context.Context, params *moveObjectPa
707698
return err
708699
}, s.retry, s.idempotent)
709700
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
710-
return nil, ErrObjectNotExist
701+
return nil, formatObjectErr(err)
711702
}
712703
return attrs, err
713704
}
@@ -950,7 +941,7 @@ func (c *grpcStorageClient) ComposeObject(ctx context.Context, req *composeObjec
950941
return err
951942
}, s.retry, s.idempotent); err != nil {
952943
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
953-
return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err)
944+
return nil, formatObjectErr(err)
954945
}
955946
return nil, err
956947
}
@@ -1002,7 +993,7 @@ func (c *grpcStorageClient) RewriteObject(ctx context.Context, req *rewriteObjec
1002993

1003994
if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil {
1004995
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
1005-
return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err)
996+
return nil, formatObjectErr(err)
1006997
}
1007998
return nil, err
1008999
}
@@ -1584,7 +1575,7 @@ func (c *grpcStorageClient) NewRangeReader(ctx context.Context, params *newRange
15841575
// These types of errors show up on the RecvMsg call, rather than the
15851576
// initialization of the stream via BidiReadObject above.
15861577
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
1587-
return ErrObjectNotExist
1578+
return formatObjectErr(err)
15881579
}
15891580
if err != nil {
15901581
return err

‎storage/grpc_reader.go‎

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,8 @@ import (
2626
"cloud.google.com/go/storage/internal/apiv2/storagepb"
2727
"github.com/googleapis/gax-go/v2"
2828
"google.golang.org/grpc"
29-
"google.golang.org/grpc/codes"
3029
"google.golang.org/grpc/encoding"
3130
"google.golang.org/grpc/mem"
32-
"google.golang.org/grpc/status"
3331
"google.golang.org/protobuf/encoding/protowire"
3432
"google.golang.org/protobuf/proto"
3533
)
@@ -146,13 +144,10 @@ func (c *grpcStorageClient) NewRangeReaderReadObject(ctx context.Context, params
146144
// use a custom decoder to avoid an extra copy at the protobuf layer.
147145
databufs := mem.BufferSlice{}
148146
err := stream.RecvMsg(&databufs)
149-
// These types of errors show up on the Recv call, rather than the
150-
// initialization of the stream via ReadObject above.
151-
if s, ok := status.FromError(err); ok && s.Code() == codes.NotFound {
152-
return ErrObjectNotExist
153-
}
154147
if err != nil {
155-
return err
148+
// NotFound types of errors show up on the Recv call, rather than the
149+
// initialization of the stream via ReadObject above.
150+
return formatObjectErr(err)
156151
}
157152
// Use a custom decoder that uses protobuf unmarshalling for all
158153
// fields except the object data. Object data is handled separately

‎storage/http_client.go‎

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -287,12 +287,8 @@ func (c *httpStorageClient) GetBucket(ctx context.Context, bucket string, conds
287287
return err
288288
}, s.retry, s.idempotent)
289289

290-
var e *googleapi.Error
291-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
292-
return nil, ErrBucketNotExist
293-
}
294290
if err != nil {
295-
return nil, err
291+
return nil, formatBucketError(err)
296292
}
297293
return newBucket(resp)
298294
}
@@ -382,11 +378,7 @@ func (c *httpStorageClient) ListObjects(ctx context.Context, bucket string, q *Q
382378
return err
383379
}, s.retry, s.idempotent)
384380
if err != nil {
385-
var e *googleapi.Error
386-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
387-
err = ErrBucketNotExist
388-
}
389-
return "", err
381+
return "", formatBucketError(err)
390382
}
391383
for _, item := range resp.Items {
392384
it.items = append(it.items, newObject(item))
@@ -416,11 +408,7 @@ func (c *httpStorageClient) DeleteObject(ctx context.Context, bucket, object str
416408
req.UserProject(s.userProject)
417409
}
418410
err := run(ctx, func(ctx context.Context) error { return req.Context(ctx).Do() }, s.retry, s.idempotent)
419-
var e *googleapi.Error
420-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
421-
return ErrObjectNotExist
422-
}
423-
return err
411+
return formatObjectErr(err)
424412
}
425413

426414
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
445433
obj, err = req.Context(ctx).Do()
446434
return err
447435
}, s.retry, s.idempotent)
448-
var e *googleapi.Error
449-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
450-
return nil, ErrObjectNotExist
451-
}
452436
if err != nil {
453-
return nil, err
437+
return nil, formatObjectErr(err)
454438
}
455439
return newObject(obj), nil
456440
}
@@ -555,12 +539,8 @@ func (c *httpStorageClient) UpdateObject(ctx context.Context, params *updateObje
555539
var obj *raw.Object
556540
var err error
557541
err = run(ctx, func(ctx context.Context) error { obj, err = call.Context(ctx).Do(); return err }, s.retry, s.idempotent)
558-
var e *googleapi.Error
559-
if errors.As(err, &e) && e.Code == http.StatusNotFound {
560-
return nil, ErrObjectNotExist
561-
}
562542
if err != nil {
563-
return nil, err
543+
return nil, formatObjectErr(err)
564544
}
565545
return newObject(obj), nil
566546
}
@@ -585,9 +565,8 @@ func (c *httpStorageClient) RestoreObject(ctx context.Context, params *restoreOb
585565
var obj *raw.Object
586566
var err error
587567
err = run(ctx, func(ctx context.Context) error { obj, err = req.Context(ctx).Do(); return err }, s.retry, s.idempotent)
588-
var e *googleapi.Error
589-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
590-
return nil, ErrObjectNotExist
568+
if err != nil {
569+
return nil, formatObjectErr(err)
591570
}
592571
return newObject(obj), err
593572
}
@@ -610,9 +589,8 @@ func (c *httpStorageClient) MoveObject(ctx context.Context, params *moveObjectPa
610589
var obj *raw.Object
611590
var err error
612591
err = run(ctx, func(ctx context.Context) error { obj, err = req.Context(ctx).Do(); return err }, s.retry, s.idempotent)
613-
var e *googleapi.Error
614-
if ok := errors.As(err, &e); ok && e.Code == http.StatusNotFound {
615-
return nil, ErrObjectNotExist
592+
if err != nil {
593+
return nil, formatObjectErr(err)
616594
}
617595
return newObject(obj), err
618596
}
@@ -800,11 +778,7 @@ func (c *httpStorageClient) ComposeObject(ctx context.Context, req *composeObjec
800778
retryCall := func(ctx context.Context) error { obj, err = call.Context(ctx).Do(); return err }
801779

802780
if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil {
803-
var e *googleapi.Error
804-
if errors.As(err, &e) && e.Code == http.StatusNotFound {
805-
return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err)
806-
}
807-
return nil, err
781+
return nil, formatObjectErr(err)
808782
}
809783
return newObject(obj), nil
810784
}
@@ -851,11 +825,7 @@ func (c *httpStorageClient) RewriteObject(ctx context.Context, req *rewriteObjec
851825
retryCall := func(ctx context.Context) error { res, err = call.Context(ctx).Do(); return err }
852826

853827
if err := run(ctx, retryCall, s.retry, s.idempotent); err != nil {
854-
var e *googleapi.Error
855-
if errors.As(err, &e) && e.Code == http.StatusNotFound {
856-
return nil, fmt.Errorf("%w: %w", ErrObjectNotExist, err)
857-
}
858-
return nil, err
828+
return nil, formatObjectErr(err)
859829
}
860830

861831
r := &rewriteObjectResponse{
@@ -1420,13 +1390,7 @@ func readerReopen(ctx context.Context, header http.Header, params *newRangeReade
14201390
err = run(ctx, func(ctx context.Context) error {
14211391
res, err = doDownload(ctx)
14221392
if err != nil {
1423-
var e *googleapi.Error
1424-
if errors.As(err, &e) {
1425-
if e.Code == http.StatusNotFound {
1426-
return ErrObjectNotExist
1427-
}
1428-
}
1429-
return err
1393+
return formatObjectErr(err)
14301394
}
14311395

14321396
if res.StatusCode == http.StatusNotFound {
@@ -1435,7 +1399,7 @@ func readerReopen(ctx context.Context, header http.Header, params *newRangeReade
14351399
return ErrObjectNotExist
14361400
}
14371401
if res.StatusCode < 200 || res.StatusCode > 299 {
1438-
body, _ := ioutil.ReadAll(res.Body)
1402+
body, _ := io.ReadAll(res.Body)
14391403
res.Body.Close()
14401404
return &googleapi.Error{
14411405
Code: res.StatusCode,

‎storage/integration_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@ func TestIntegration_BucketCreateDelete(t *testing.T) {
934934
t.Fatalf("BucketHandle.Delete(%q): %v", newBucketName, err)
935935
}
936936
_, err = b.Attrs(ctx)
937-
if err != ErrBucketNotExist {
937+
if !errors.Is(err, ErrBucketNotExist) {
938938
t.Fatalf("expected ErrBucketNotExist, got %v", err)
939939
}
940940
})
@@ -2442,7 +2442,7 @@ func TestIntegration_Encoding(t *testing.T) {
24422442

24432443
// Test NotFound.
24442444
_, err = bkt.Object("obj-not-exists").NewReader(ctx)
2445-
if err != ErrObjectNotExist {
2445+
if !errors.Is(err, ErrObjectNotExist) {
24462446
t.Errorf("Object should not exist, err found to be %v", err)
24472447
}
24482448
})
@@ -3325,11 +3325,11 @@ func TestIntegration_NonexistentBucket(t *testing.T) {
33253325
ctx := skipExtraReadAPIs(context.Background(), "no reads in test")
33263326
multiTransportTest(ctx, t, func(t *testing.T, ctx context.Context, _, prefix string, client *Client) {
33273327
bkt := client.Bucket(prefix + uidSpace.New())
3328-
if _, err := bkt.Attrs(ctx); err != ErrBucketNotExist {
3328+
if _, err := bkt.Attrs(ctx); !errors.Is(err, ErrBucketNotExist) {
33293329
t.Errorf("Attrs: got %v, want ErrBucketNotExist", err)
33303330
}
33313331
it := bkt.Objects(ctx, nil)
3332-
if _, err := it.Next(); err != ErrBucketNotExist {
3332+
if _, err := it.Next(); !errors.Is(err, ErrBucketNotExist) {
33333333
t.Errorf("Objects: got %v, want ErrBucketNotExist", err)
33343334
}
33353335
})

‎storage/storage.go‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,10 @@ import (
5353
raw "google.golang.org/api/storage/v1"
5454
"google.golang.org/api/transport"
5555
htransport "google.golang.org/api/transport/http"
56+
"google.golang.org/grpc/codes"
5657
"google.golang.org/grpc/experimental/stats"
5758
"google.golang.org/grpc/stats/opentelemetry"
59+
"google.golang.org/grpc/status"
5860
"google.golang.org/protobuf/proto"
5961
"google.golang.org/protobuf/reflect/protoreflect"
6062
"google.golang.org/protobuf/types/known/fieldmaskpb"
@@ -65,9 +67,11 @@ import (
6567
var signedURLMethods = map[string]bool{"DELETE": true, "GET": true, "HEAD": true, "POST": true, "PUT": true}
6668

6769
var (
68-
// ErrBucketNotExist indicates that the bucket does not exist.
70+
// ErrBucketNotExist indicates that the bucket does not exist. It should be
71+
// checked for using [errors.Is] instead of direct equality.
6972
ErrBucketNotExist = errors.New("storage: bucket doesn't exist")
70-
// ErrObjectNotExist indicates that the object does not exist.
73+
// ErrObjectNotExist indicates that the object does not exist. It should be
74+
// checked for using [errors.Is] instead of direct equality.
7175
ErrObjectNotExist = errors.New("storage: object doesn't exist")
7276
// errMethodNotSupported indicates that the method called is not currently supported by the client.
7377
// 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
26192623
}
26202624
return nil
26212625
}
2626+
2627+
// formatObjectErr checks if the provided error is NotFound and if so, wraps
2628+
// it in an ErrObjectNotExist error. If not, formatObjectErr has no effect.
2629+
func formatObjectErr(err error) error {
2630+
var e *googleapi.Error
2631+
if s, ok := status.FromError(err); (ok && s.Code() == codes.NotFound) ||
2632+
(errors.As(err, &e) && e.Code == http.StatusNotFound) {
2633+
return fmt.Errorf("%w: %w", ErrObjectNotExist, err)
2634+
}
2635+
return err
2636+
}
2637+
2638+
// formatBucketError checks if the provided error is NotFound and if so, wraps
2639+
// it in an ErrBucketNotExist error. If not, formatBucketError has no effect.
2640+
func formatBucketError(err error) error {
2641+
var e *googleapi.Error
2642+
if s, ok := status.FromError(err); (ok && s.Code() == codes.NotFound) ||
2643+
(errors.As(err, &e) && e.Code == http.StatusNotFound) {
2644+
return fmt.Errorf("%w: %w", ErrBucketNotExist, err)
2645+
}
2646+
return err
2647+
}

‎storage/transfermanager/integration_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -560,7 +560,7 @@ func TestIntegration_DownloaderErrorSync(t *testing.T) {
560560

561561
// Check that the nonexistent object returned an error.
562562
if got.Object == nonexistentObject {
563-
if got.Err != storage.ErrObjectNotExist {
563+
if !errors.Is(got.Err, storage.ErrObjectNotExist) {
564564
t.Errorf("Object(%q) should not exist, err found to be %v", got.Object, got.Err)
565565
}
566566
continue
@@ -718,7 +718,7 @@ func TestIntegration_DownloaderErrorAsync(t *testing.T) {
718718
callbackMu.Unlock()
719719

720720
// Check that the nonexistent object returned an error.
721-
if got.Err != storage.ErrObjectNotExist {
721+
if !errors.Is(got.Err, storage.ErrObjectNotExist) {
722722
t.Errorf("Object(%q) should not exist, err found to be %v", got.Object, got.Err)
723723
}
724724
},

0 commit comments

Comments
 (0)