From 4db2480f472fc6958786ffc95eea96b39fee2d20 Mon Sep 17 00:00:00 2001 From: Robert Raynor <35671663+mooselumph@users.noreply.github.com> Date: Fri, 29 Mar 2024 18:06:13 -0700 Subject: [PATCH] don't count metadata not found as internal error (#418) --- disperser/apiserver/server.go | 9 +++++++-- disperser/apiserver/server_test.go | 2 +- disperser/common/blobstore/blob_metadata_store.go | 7 ++++++- disperser/errors.go | 3 ++- 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/disperser/apiserver/server.go b/disperser/apiserver/server.go index 7d38040622..f09da8d3d9 100644 --- a/disperser/apiserver/server.go +++ b/disperser/apiserver/server.go @@ -520,7 +520,9 @@ func (s *DispersalServer) GetBlobStatus(ctx context.Context, req *pb.BlobStatusR s.logger.Debug("metadataKey", "metadataKey", metadataKey.String()) metadata, err := s.blobStore.GetBlobMetadata(ctx, metadataKey) if err != nil { - // TODO: we need to distinguish NOT_FOUND from actual internal error. + if errors.Is(err, disperser.ErrMetadataNotFound) { + return nil, api.NewInvalidArgError("no metadata found for the requestID") + } return nil, api.NewInternalError(fmt.Sprintf("failed to get blob metadata, blobkey: %s", metadataKey.String())) } @@ -650,7 +652,10 @@ func (s *DispersalServer) RetrieveBlob(ctx context.Context, req *pb.RetrieveBlob blobMetadata, err := s.blobStore.GetMetadataInBatch(ctx, batchHeaderHash32, blobIndex) if err != nil { s.logger.Error("Failed to retrieve blob metadata", "err", err) - // TODO: we need to distinguish NOT_FOUND from actual internal error. + if errors.Is(err, disperser.ErrMetadataNotFound) { + s.metrics.IncrementFailedBlobRequestNum(codes.NotFound.String(), "", "RetrieveBlob") + return nil, api.NewInvalidArgError("no metadata found for the given batch header hash and blob index") + } s.metrics.IncrementFailedBlobRequestNum(codes.Internal.String(), "", "RetrieveBlob") return nil, api.NewInternalError("failed to get blob metadata, please retry") } diff --git a/disperser/apiserver/server_test.go b/disperser/apiserver/server_test.go index e0cf81e049..819e8203a5 100644 --- a/disperser/apiserver/server_test.go +++ b/disperser/apiserver/server_test.go @@ -350,7 +350,7 @@ func TestRetrieveBlobFailsWhenBlobNotConfirmed(t *testing.T) { // Try to retrieve the blob before it is confirmed _, err = retrieveBlob(t, dispersalServer, requestID, 2) assert.NotNil(t, err) - assert.Equal(t, err.Error(), "rpc error: code = Internal desc = failed to get blob metadata, please retry") + assert.Equal(t, "rpc error: code = InvalidArgument desc = no metadata found for the given batch header hash and blob index", err.Error()) } diff --git a/disperser/common/blobstore/blob_metadata_store.go b/disperser/common/blobstore/blob_metadata_store.go index 4a284895ef..5ed56cb38a 100644 --- a/disperser/common/blobstore/blob_metadata_store.go +++ b/disperser/common/blobstore/blob_metadata_store.go @@ -61,6 +61,11 @@ func (s *BlobMetadataStore) GetBlobMetadata(ctx context.Context, metadataKey dis Value: metadataKey.MetadataHash, }, }) + + if item == nil { + return nil, fmt.Errorf("%w: metadata not found for key %s", disperser.ErrMetadataNotFound, metadataKey) + } + if err != nil { return nil, err } @@ -199,7 +204,7 @@ func (s *BlobMetadataStore) GetBlobMetadataInBatch(ctx context.Context, batchHea } if len(items) == 0 { - return nil, fmt.Errorf("there is no metadata for batch %s and blob index %d", batchHeaderHash, blobIndex) + return nil, fmt.Errorf("%w: there is no metadata for batch %s and blob index %d", disperser.ErrMetadataNotFound, batchHeaderHash, blobIndex) } if len(items) > 1 { diff --git a/disperser/errors.go b/disperser/errors.go index 8681436942..2872dbd100 100644 --- a/disperser/errors.go +++ b/disperser/errors.go @@ -3,5 +3,6 @@ package disperser import "errors" var ( - ErrBlobNotFound = errors.New("blob not found") + ErrBlobNotFound = errors.New("blob not found") + ErrMetadataNotFound = errors.New("metadata not found") )