Skip to content

Commit

Permalink
Code hygiene: return error instead of bool (#660)
Browse files Browse the repository at this point in the history
  • Loading branch information
jianoaix authored Jul 24, 2024
1 parent 890df75 commit 85bd03f
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 13 deletions.
7 changes: 4 additions & 3 deletions node/grpc/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package grpc

import (
"context"
"encoding/hex"
"errors"
"fmt"
"runtime"
Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions node/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 2 additions & 3 deletions node/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down

0 comments on commit 85bd03f

Please sign in to comment.