From 85bd03f5f37cdd2f782f9e219f8303f6313428f3 Mon Sep 17 00:00:00 2001 From: Jian Xiao <99709935+jianoaix@users.noreply.github.com> Date: Wed, 24 Jul 2024 15:54:06 -0700 Subject: [PATCH] Code hygiene: return error instead of bool (#660) --- node/grpc/server.go | 7 ++++--- node/store.go | 14 +++++++------- node/store_test.go | 5 ++--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/node/grpc/server.go b/node/grpc/server.go index 01f82cdea7..9823440a90 100644 --- a/node/grpc/server.go +++ b/node/grpc/server.go @@ -2,6 +2,7 @@ package grpc import ( "context" + "encoding/hex" "errors" "fmt" "runtime" @@ -302,10 +303,10 @@ func (s *Server) RetrieveChunks(ctx context.Context, in *pb.RetrieveChunksReques return nil, errors.New("request rate limited") } - chunks, format, ok := s.node.Store.GetChunks(ctx, batchHeaderHash, int(in.GetBlobIndex()), uint8(in.GetQuorumId())) - if !ok { + chunks, format, err := s.node.Store.GetChunks(ctx, batchHeaderHash, int(in.GetBlobIndex()), uint8(in.GetQuorumId())) + if err != nil { s.node.Metrics.RecordRPCRequest("RetrieveChunks", "failure", time.Since(start)) - return nil, fmt.Errorf("could not find chunks for batchHeaderHash %v, blob index: %v, quorumID: %v", batchHeaderHash, in.GetBlobIndex(), in.GetQuorumId()) + return nil, fmt.Errorf("could not find chunks for batchHeaderHash %v, blob index: %v, quorumID: %v", hex.EncodeToString(batchHeaderHash[:]), in.GetBlobIndex(), in.GetQuorumId()) } s.node.Metrics.RecordRPCRequest("RetrieveChunks", "success", time.Since(start)) return &pb.RetrieveChunksReply{Chunks: chunks, Encoding: format}, nil diff --git a/node/store.go b/node/store.go index 0e38be43d1..6b35e6270e 100644 --- a/node/store.go +++ b/node/store.go @@ -347,26 +347,26 @@ func (s *Store) GetBlobHeader(ctx context.Context, batchHeaderHash [32]byte, blo return data, nil } -// GetChunks returns the list of byte arrays stored for given blobKey along with a boolean -// indicating if the read was unsuccessful or the chunks were serialized correctly -func (s *Store) GetChunks(ctx context.Context, batchHeaderHash [32]byte, blobIndex int, quorumID core.QuorumID) ([][]byte, node.ChunkEncoding, bool) { +// GetChunks returns the list of byte arrays stored for given blobKey along with the encoding +// format of the bytes. +func (s *Store) GetChunks(ctx context.Context, batchHeaderHash [32]byte, blobIndex int, quorumID core.QuorumID) ([][]byte, node.ChunkEncoding, error) { log := s.logger blobKey, err := EncodeBlobKey(batchHeaderHash, blobIndex, quorumID) if err != nil { - return nil, node.ChunkEncoding_UNKNOWN, false + return nil, node.ChunkEncoding_UNKNOWN, err } data, err := s.db.Get(blobKey) if err != nil { - return nil, node.ChunkEncoding_UNKNOWN, false + return nil, node.ChunkEncoding_UNKNOWN, err } log.Debug("Retrieved chunk", "blobKey", hexutil.Encode(blobKey), "length", len(data)) chunks, format, err := DecodeChunks(data) if err != nil { - return nil, format, false + return nil, format, err } - return chunks, format, true + return chunks, format, nil } // HasKey returns if a given key has been stored. diff --git a/node/store_test.go b/node/store_test.go index 7c6faf45ce..c4c2a8d389 100644 --- a/node/store_test.go +++ b/node/store_test.go @@ -331,12 +331,11 @@ func TestStoringBlob(t *testing.T) { func decodeChunks(t *testing.T, s *node.Store, batchHeaderHash [32]byte, blobIdx int, chunkEncoding pb.ChunkEncoding) []*encoding.Frame { ctx := context.Background() - chunks, format, ok := s.GetChunks(ctx, batchHeaderHash, blobIdx, 0) - assert.True(t, ok) + chunks, format, err := s.GetChunks(ctx, batchHeaderHash, blobIdx, 0) + assert.Nil(t, err) assert.Equal(t, 1, len(chunks)) assert.Equal(t, chunkEncoding, format) var f *encoding.Frame - var err error switch chunkEncoding { case pb.ChunkEncoding_GOB: f, err = new(encoding.Frame).Deserialize(chunks[0])