[block-store] return ErrNotFound instead of default error type (#90)

#### Type of change

- Improvement (improvement to code, performance, etc)

#### Description

Currently, the committer sends NotFound grpc error code for block APIs
by parsing the error message. As this is fragile, this commit adds
ErrNotFound error so that the committer can use errors.Is().

#### Related issues

  - resolves #87

---------

Signed-off-by: Senthilnathan <cendhu@gmail.com>

Author: cendhu

common/ledger/blkstorage/blockindex.go

@@ -12,9 +12,9 @@ import (
 	"path/filepath"
 	"unicode/utf8"
 
+	"github.com/cockroachdb/errors"
 	"github.com/hyperledger/fabric-protos-go-apiv2/common"
 	"github.com/hyperledger/fabric-protos-go-apiv2/peer"
-	"github.com/pkg/errors"
 	"google.golang.org/protobuf/encoding/protowire"
 	"google.golang.org/protobuf/proto"
 
@@ -36,6 +36,9 @@ const (
 	snapshotMetadataFileName = "txids.metadata"
 )
 
+// ErrNotFound is returned when a requested item is not found in the block index.
+var ErrNotFound = errors.New("not found in index")
+
 var (
 	indexSavePointKey              = []byte(indexSavePointKeyStr)
 	errIndexSavePointKeyNotPresent = errors.New("NoBlockIndexed")
@@ -157,7 +160,7 @@ func (index *blockIndex) getBlockLocByHash(blockHash []byte) (*fileLocPointer, e
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such block hash [%x] in index", blockHash)
+		return nil, errors.Wrapf(ErrNotFound, "block hash [%x]", blockHash)
 	}
 	blkLoc := &fileLocPointer{}
 	if err := blkLoc.unmarshal(b); err != nil {
@@ -175,7 +178,7 @@ func (index *blockIndex) getBlockLocByBlockNum(blockNum uint64) (*fileLocPointer
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such block number [%d] in index", blockNum)
+		return nil, errors.Wrapf(ErrNotFound, "block number [%d]", blockNum)
 	}
 	blkLoc := &fileLocPointer{}
 	if err := blkLoc.unmarshal(b); err != nil {
@@ -250,7 +253,7 @@ func (index *blockIndex) getTxIDVal(txID string) (*TxIDIndexValue, uint64, error
 		return nil, 0, errors.Wrapf(err, "error while trying to retrieve transaction info by TXID [%s]", txID)
 	}
 	if !present {
-		return nil, 0, errors.Errorf("no such transaction ID [%s] in index", txID)
+		return nil, 0, errors.Wrapf(ErrNotFound, "transaction ID [%s]", txID)
 	}
 	valBytes := itr.Value()
 	if len(valBytes) == 0 {
@@ -276,7 +279,7 @@ func (index *blockIndex) getTXLocByBlockNumTranNum(blockNum uint64, tranNum uint
 		return nil, err
 	}
 	if b == nil {
-		return nil, errors.Errorf("no such blockNumber, transactionNumber <%d, %d> in index", blockNum, tranNum)
+		return nil, errors.Wrapf(ErrNotFound, "blockNumber, transactionNumber <%d, %d>", blockNum, tranNum)
 	}
 	txFLP := &fileLocPointer{}
 	if err := txFLP.unmarshal(b); err != nil {
@@ -358,7 +361,8 @@ func (index *blockIndex) exportUniqueTxIDs(dir string, newHashFunc snapshot.NewH
 func importTxIDsFromSnapshot(
 	snapshotDir string,
 	lastBlockNumInSnapshot uint64,
-	db *leveldbhelper.DBHandle) error {
+	db *leveldbhelper.DBHandle,
+) error {
 	txIDsMetadata, err := snapshot.OpenFile(filepath.Join(snapshotDir, snapshotMetadataFileName), snapshotFileFormat)
 	if err != nil {
 		return err

common/ledger/blkstorage/blockindex_test.go

@@ -67,7 +67,7 @@ func testBlockIndexSync(t *testing.T, numBlocks int, numBlocksToIndex int, syncB
 		// Before, we test for index sync-up, verify that the last set of blocks not indexed in the original index
 		for i := numBlocksToIndex + 1; i <= numBlocks; i++ {
 			_, err := blkfileMgr.retrieveBlockByNumber(uint64(i))
-			require.EqualError(t, err, fmt.Sprintf("no such block number [%d] in index", i))
+			requireNotFoundError(t, err, fmt.Sprintf("block number [%d]", i))
 		}
 
 		// perform index sync

common/ledger/blkstorage/blockstore_provider_test.go

@@ -143,24 +143,24 @@ func checkBlocks(t *testing.T, expectedBlocks []*common.Block, store *BlockStore
 func checkWithWrongInputs(t *testing.T, store *BlockStore, numBlocks int) {
 	block, err := store.RetrieveBlockByHash([]byte("non-existent-hash"))
 	require.Nil(t, block)
-	require.EqualError(t, err, fmt.Sprintf("no such block hash [%x] in index", []byte("non-existent-hash")))
+	requireNotFoundError(t, err, "block hash")
 
 	block, err = store.RetrieveBlockByTxID("non-existent-txid")
 	require.Nil(t, block)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 
 	tx, err := store.RetrieveTxByID("non-existent-txid")
 	require.Nil(t, tx)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 
 	tx, err = store.RetrieveTxByBlockNumTranNum(uint64(numBlocks+1), uint64(0))
 	require.Nil(t, tx)
-	require.EqualError(t, err, fmt.Sprintf("no such blockNumber, transactionNumber <%d, 0> in index", numBlocks+1))
+	requireNotFoundError(t, err, fmt.Sprintf("blockNumber, transactionNumber <%d, 0>", numBlocks+1))
 
 	txCode, blkNum, err := store.RetrieveTxValidationCodeByTxID("non-existent-txid")
 	require.Equal(t, peer.TxValidationCode(-1), txCode)
 	require.Equal(t, uint64(0), blkNum)
-	require.EqualError(t, err, "no such transaction ID [non-existent-txid] in index")
+	requireNotFoundError(t, err, "transaction ID [non-existent-txid]")
 }
 
 func TestBlockStoreProvider(t *testing.T) {

common/ledger/blkstorage/pkg_test.go

@@ -113,7 +113,7 @@ func (w *testBlockfileMgrWrapper) testGetBlockByHashNotIndexed(blocks []*common.
 	for _, block := range blocks {
 		hash := protoutil.BlockHeaderHash(block.Header)
 		_, err := w.blockfileMgr.retrieveBlockByHash(hash)
-		require.EqualError(w.t, err, fmt.Sprintf("no such block hash [%x] in index", hash))
+		requireNotFoundError(w.t, err, "block hash")
 	}
 }
 
@@ -123,7 +123,7 @@ func (w *testBlockfileMgrWrapper) testGetBlockByTxIDNotIndexed(blocks []*common.
 			txID, err := protoutil.GetOrComputeTxIDFromEnvelope(txEnv)
 			require.NoError(w.t, err)
 			_, err = w.blockfileMgr.retrieveBlockByTxID(txID)
-			require.EqualError(w.t, err, fmt.Sprintf("no such transaction ID [%s] in index", txID))
+			requireNotFoundError(w.t, err, fmt.Sprintf("transaction ID [%s]", txID))
 		}
 	}
 }

common/ledger/blkstorage/reset_test.go

@@ -236,7 +236,7 @@ func assertBlockStorePostReset(t *testing.T, store *BlockStore, originallyCommit
 	require.Equal(t, originallyCommittedBlocks[0], blk)
 
 	_, err = store.RetrieveBlockByNumber(1)
-	require.EqualError(t, err, "no such block number [1] in index")
+	requireNotFoundError(t, err, "block number [1]")
 
 	err = store.AddBlock(originallyCommittedBlocks[0])
 	require.EqualError(t, err, "block number should have been 1 but was 0")

common/ledger/blkstorage/snapshot_test.go

@@ -285,7 +285,7 @@ func TestImportFromSnapshot(t *testing.T) {
 
 			// before, we test for index sync-up, verify that the last set of blocks not indexed in the original index
 			_, err := blkfileMgr.retrieveBlockByNumber(block.Header.Number)
-			require.EqualError(t, err, fmt.Sprintf("no such block number [%d] in index", block.Header.Number))
+			requireNotFoundError(t, err, fmt.Sprintf("block number [%d]", block.Header.Number))
 
 			// close and open should be able to sync-up the index
 			closeBlockStore()
@@ -472,7 +472,7 @@ func verifyQueriesOnBlocksPriorToSnapshot(
 		require.EqualError(t, err, expectedErrStr)
 
 		_, err = bootstrappedBlockStore.RetrieveBlockByHash(blockHash)
-		require.EqualError(t, err, fmt.Sprintf("no such block hash [%x] in index", blockHash))
+		requireNotFoundError(t, err, "block hash")
 	}
 
 	bootstrappingSnapshotHeight := uint64(len(blocksDetailsBeforeSnapshot))

common/ledger/blkstorage/test_utils.go

@@ -0,0 +1,21 @@
+/*
+Copyright IBM Corp. All Rights Reserved.
+
+SPDX-License-Identifier: Apache-2.0
+*/
+
+package blkstorage
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+// requireNotFoundError is a test helper that verifies an error is ErrNotFound
+// and contains the expected context string.
+func requireNotFoundError(tb testing.TB, err error, expectedContext string) {
+	tb.Helper()
+	require.ErrorIs(tb, err, ErrNotFound)
+	require.ErrorContains(tb, err, expectedContext)
+}